The OS X client kernel panics when doing compound reads from Samba, because it expects the compound read response to be padded to an 8 byte boundary. Windows client behaviour: - doesn't use compound reads Windows SMB server behaviour: - compound read responses are always padded to an 8 byte boundary OS X client behaviour: - compound read requests are not padded against OS X SMB server (AAPL?) - compound read requests are padded against Windows and Samba - if compound read request was padded and response is NOT padded, the client kernel panics OS X SMB server behaviour with smbtorture (ie no AAPL): - if compound read request is padded, response is padded - if compound read request is not padded, response is also not padded For Samba imo this boils down to: employ Windows behaviour of always padding compound read responses, patch to follow, including torture test. Once I have the bug id, I'll query dochelp for clarification on [MS-SMB2] 3.3.4.1.3 <https://msdn.microsoft.com/en-us/library/cc246720.aspx> where compound responses are discussed.
Created attachment 11057 [details] Patch and torture test
Created attachment 11058 [details] Patch and torture test
Created attachment 11063 [details] Compound read from Win2012 Compound read response is padded to an 8 byte boundary.
dochelp confirmed that "all the compound responses sent by Windows servers are 8 byte aligned". MS-SMB2 will be updated with an additional Windows behavior note.
Ralph, I want to push this but I think + uint32_t next_command_body_ofs, tmp_offset, next_command_ofs = 0; should move tmp_offset inside the body of: + /* + * See if we need to recalculate the offset to the next + * response, or add padding for the last request in a compound + * chain. + */ + + if ((next_command_body_ofs > 0) || + ((next_command_body_ofs == 0) && (flags & SMB2_HDR_FLAG_CHAINED))) { + tmp_offset = SMB2_HDR_BODY; + tmp_offset += SMBD_SMB2_OUT_BODY_LEN(req); + tmp_offset += SMBD_SMB2_OUT_DYN_LEN(req); + + if (next_command_body_ofs > 0) { + /* + * We have a next compound, use calculated + * offset and add padding. + */ + next_command_ofs = tmp_offset; + pad_size = 8 - (tmp_offset % 8); + } else if (flags & SMB2_HDR_FLAG_CHAINED) { + /* + * Last command (next_command_body_ofs == 0) + * in a compound request, add padding to 8 + * byte boundary if necessary. According to + * [MS-SMB2] 3.3.4.1.3, Sending Compounded + * Responses this, this shouldn't be + * necessary, but OS X client crash hard if we + * don't and this is also what Windows 2012 + * server does. + */ + pad_size = 8 - (tmp_offset % 8); + } } as this is the only place that uses it. Can you do that tidyup and re-post the patch ? Thanks ! Jeremy.
Hi Jeremy! metze reviewed the patch and gave some more recommmendations for a proper fix. Will attach an updated patch later on today. Thanks!
Created attachment 11099 [details] Patch for master
Created attachment 11103 [details] Patch for 4.2 cherry-picked from master
Created attachment 11104 [details] Patch for 4.1 cherry-picked from master
Pushed to autobuild-v4-[1|2]-test.
(In reply to Karolin Seeger from comment #10) Pushed to both branches. Closing out bug report. Thanks!