There is a bug in source3/smbd/trans2.c:marshall_stream_info where it is overflowing the buffer provided and returning without all the data. Attached is a capture of Windows to Windows (via SMB2) of a dir/r request. It shows W2K08 responding to STATUS_BUFFER_OVERFLOW twice until the client gives it a large enough buffer. Attached also is a capture of Samba 3.6.6 (but it applies to master, I believe) of Samba returning incomplete data and an overflowed result but the wrong status.
Created attachment 8275 [details] Win7 to W2K08 dir/r results
Created attachment 8276 [details] Win 7 to Samba 3.6.6 dir/r result
Here is a potential fix for this problem. Basically, correctly detect that we will overflow the return buffer and return STATUS_BUFFER_OVERFLOW before doing so. I will attach a patch as well after I have tested it. index 61d755c..9bd1f53 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -4159,7 +4159,7 @@ static NTSTATUS marshall_stream_info(unsigned int num_stre unsigned int i; unsigned int ofs = 0; - for (i = 0; i < num_streams && ofs <= max_data_bytes; i++) { + for (i = 0; i < num_streams; i++) { unsigned int next_offset; size_t namelen; smb_ucs2_t *namebuf; @@ -4178,6 +4178,15 @@ static NTSTATUS marshall_stream_info(unsigned int num_str namelen -= 2; + /* + * We cannot overflow ... + */ + if ((ofs + 24 + namelen) > max_data_bytes) { + DEBUG(10, ("overflowed the data buffer at stream %u\n", + i)); + return STATUS_BUFFER_OVERFLOW; + } + SIVAL(data, ofs+4, namelen); SOFF_T(data, ofs+8, streams[i].size); SOFF_T(data, ofs+16, streams[i].alloc_size); @@ -4801,6 +4810,7 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn, if (!NT_STATUS_IS_OK(status)) { DEBUG(10, ("marshall_stream_info failed: %s\n", nt_errstr(status))); + TALLOC_FREE(streams); return status; }
Created attachment 8278 [details] Patch that gets Samba 3.6.6 behaving as W2K08 does. With this patch we now seem to behave as W2K08 does.
This needs to be a blocker for 3.6.x and 4.0.x. Jeremy.
Ok, the same check needs to be done when we're adding the alignment padding just a little later on. Updated patches (git-am format) for master, 4.0.0 and 3.6.x to follow. Jeremy.
Created attachment 8281 [details] git-am fix for master, 4.0.0, and 3.6.x. Richard can you review please ? If it passes your tests I'll push to master and assign to Karolin for inclusion in 3.6.next and 4.0.0. Same fix cherry-picks cleanly across all branches. Jeremy.
Comment on attachment 8281 [details] git-am fix for master, 4.0.0, and 3.6.x. Looks good to me. The last debug could be dropped. It is not really needed.
Pushed to master (943797c232f96a5dd411a803ad90b6980b2785b0). Re-assigning to Karolin for inclusion in 3.6.x and 4.0.0. Jeremy.
Pushed to autobuild-v4-0-test and v3-6-test (will be included in 3.6.10).
Pushed to v4-0-test. Closing out bug report. Thanks!