Bug 13796 - MacOS credit accounting breaks with async SESSION SETUP
Summary: MacOS credit accounting breaks with async SESSION SETUP
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 10344 12844 12845 13698
  Show dependency treegraph
 
Reported: 2019-02-19 18:09 UTC by David Disseldorp
Modified: 2020-08-19 00:17 UTC (History)
5 users (show)

See Also:


Attachments
WIP patch against master - DO NOT PUSH ! (5.74 KB, patch)
2019-02-22 20:08 UTC, Jeremy Allison
no flags Details
Patches for v4-10-test (6.83 KB, patch)
2019-04-15 13:55 UTC, Stefan Metzmacher
jra: review+
Details
Patches for v4-9-test (6.83 KB, patch)
2019-04-15 13:56 UTC, Stefan Metzmacher
jra: review+
Details
Patches for v4-8-test (6.83 KB, text/plain)
2019-04-15 13:56 UTC, Stefan Metzmacher
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2019-02-19 18:09:19 UTC
As reported by Richard Sharpe on samba-technical:

Apple has a bug in their handling of credits when Samba returns
STATUS_PENDING for a SESSION SETUP request.

Such responses only seem to occur when the Samba server is under heavy load.

Samba issues one credit in such cases, and, as per the spec issues
zero credits in the final successful response to the SESSION SETUP.

The Mac then issues a TREE CREATE, consuming one credit and getting one credit.

The Mac then issues a compound CREATE and CLOSE. This is a protocol
violation and Samba drops the connection.

I have communicated this issue to Apple, but I also created the
attached patch to mitigate the issue while Apple thinks about fixing
their code.


Metze and Jeremy followed up with a proposed workaround:

> >> I'd propose to skip the STATUS_PENDING using
> >> smb2_request_set_async_internal() and leave the credit handling as is.  
> > 
> > Trouble with that is the client may then timeout.  
> 
> I think that's what windows also does that, but we should retest that
> with a Samba dc and sleep(60) in SamLogon*.  

If that's the Windows behavior that makes sense
that the Mac would be expecting that rather
than the intermediate response with credits.

I bet Apple only tested sessionsetup timeout
against the Windows server, and so our response
makes them go down an untested codepath (credits
granted on intermediate sessionsetup reply) and
that's what triggered the Mac client bug.

Just my guess :-).
Comment 1 Jeremy Allison 2019-02-22 20:08:37 UTC
Created attachment 14862 [details]
WIP patch against master - DO NOT PUSH !

Just collecting this so we have a record.
Comment 2 Stefan Metzmacher 2019-04-15 13:55:37 UTC
Created attachment 15068 [details]
Patches for v4-10-test
Comment 3 Stefan Metzmacher 2019-04-15 13:56:01 UTC
Created attachment 15069 [details]
Patches for v4-9-test
Comment 4 Stefan Metzmacher 2019-04-15 13:56:31 UTC
Created attachment 15070 [details]
Patches for v4-8-test
Comment 5 Jeremy Allison 2019-04-15 18:14:44 UTC
Re-assigning to Karolin for inclusion in 4.10.next, 4.9.next (and 4.8.next if there is one ?).
Comment 6 Karolin Seeger 2019-04-16 07:29:20 UTC
(In reply to Jeremy Allison from comment #5)
Pushed to autobuild-v4-{10,9}-test.
4.8 is in the security fixes only mode...
Comment 7 Karolin Seeger 2019-04-25 07:02:13 UTC
(In reply to Karolin Seeger from comment #6)
Pushed to both branches.
Closing out bug report.

Thanks!