Bug 10422 - max xmit > 64kb leads in segmentation fault
Summary: max xmit > 64kb leads in segmentation fault
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:
: 10546 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-02-06 08:55 UTC by Björn Baumbach
Modified: 2015-01-08 03:15 UTC (History)
3 users (show)

See Also:

Attachments
Completely untested patches (11.51 KB, patch)
2014-02-10 20:55 UTC, Stefan Metzmacher 		no flags Details
Patches for master (11.74 KB, patch)
2014-02-18 13:37 UTC, Stefan Metzmacher 		jra: review-
Details
Hacks to reproduce the session setup error (4.77 KB, patch)
2014-02-18 13:38 UTC, Stefan Metzmacher 		no flags Details
Current patchset for master (fails samba3.raw.read(s3dc)) (16.50 KB, patch)
2014-02-19 15:03 UTC, Stefan Metzmacher 		no flags Details
Patches for v4-1-test (23.35 KB, patch)
2014-03-20 15:25 UTC, Stefan Metzmacher 		jra: review+
Details
Patch for v4-0-test (23.30 KB, patch)
2014-03-20 15:25 UTC, Stefan Metzmacher 		jra: review+
Details
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Baumbach 2014-02-06 08:55:05 UTC 
The smbd crashes in source3/smbd/ipc.c:void send_trans_reply() when the max xmit is greater than 64kb.
Comment 1 Jeremy Allison 2014-02-10 19:09:31 UTC 
Isn't this a duplicate of bug:

https://bugzilla.samba.org/show_bug.cgi?id=10415

which Volker has already fixed ? Or am I confused :-).

Jeremy.
Comment 2 Volker Lendecke 2014-02-10 19:40:18 UTC 
(In reply to comment #1)
> Isn't this a duplicate of bug:
> 
> https://bugzilla.samba.org/show_bug.cgi?id=10415
> 
> which Volker has already fixed ? Or am I confused :-).
> 
> Jeremy.

The crash Björn talks about is an explicit panic, not a memory corruption.
Comment 3 Stefan Metzmacher 2014-02-10 20:55:35 UTC 
Created attachment 9673 [details]
Completely untested patches

Jeremy, here's my work in progress, but we need to test the behavior
of Windows when it gets a 0 MaxBuffer in the first SessionSetup.
Comment 4 Stefan Metzmacher 2014-02-18 13:37:48 UTC 
Created attachment 9690 [details]
Patches for master
Comment 5 Stefan Metzmacher 2014-02-18 13:38:35 UTC 
Created attachment 9691 [details]
Hacks to reproduce the session setup error
Comment 6 Jeremy Allison 2014-02-18 22:52:17 UTC 
Let me grub through the CIFSFS code first to see if this is going to break large transfers with the Linux client...

Jeremy.
Comment 7 Jeremy Allison 2014-02-19 01:32:09 UTC 
Comment on attachment 9690 [details]
Patches for master

Still needs work. The BUFFER_SIZE macro is still used in :

source3/client/client.c:	while (i < (n - 1) && (i < BUFFER_SIZE)) {
source3/param/loadparm.c:	return MIN(Globals.iminreceivefile, BUFFER_SIZE);
source3/torture/torture.c:			NULL, 0, BUFFER_SIZE, 
source3/utils/smbfilter.c:static char packet[BUFFER_SIZE];

plus I think you're missing the addition of smb1.sessions.max_recv in the globals.h smb1 struct.

Do you want me to fix these and re-submit, or will you do it ?

Jeremy.
Comment 8 Stefan Metzmacher 2014-02-19 15:03:35 UTC 
Created attachment 9698 [details]
Current patchset for master (fails samba3.raw.read(s3dc))

Hi Jeremy, here's my current work..., if you want go from there...
Comment 9 Stefan Metzmacher 2014-03-20 15:25:26 UTC 
Created attachment 9792 [details]
Patches for v4-1-test
Comment 10 Stefan Metzmacher 2014-03-20 15:25:50 UTC 
Created attachment 9793 [details]
Patch for v4-0-test
Comment 11 Jeremy Allison 2014-03-21 17:38:59 UTC 
Re-assigning to Karolin for inclusion in 4.0.next, 4.1.next.
Jeremy.
Comment 12 Karolin Seeger 2014-03-25 09:51:18 UTC 
(In reply to comment #11)
> Re-assigning to Karolin for inclusion in 4.0.next, 4.1.next.
> Jeremy.

Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Comment 13 Karolin Seeger 2014-04-04 18:51:31 UTC 
Pushed to v4-1-test and v4-0-test.
Closing out bug report.

Thanks!
Comment 14 Jeremy Allison 2014-04-11 21:25:08 UTC 
*** Bug 10546 has been marked as a duplicate of this bug. ***
Comment 15 Warren 2015-01-08 03:15:16 UTC 
Would it better to update the document which still says 64K worth of data, while LARGE_WRITEX_BUFFER_SIZE is (128*1024)?

/*
 * A WRITEX with CAP_LARGE_WRITEX can be 64k worth of data plus 65 bytes
 * of header. Don't print the error if this fits.... JRA.
 */

if (len > (LARGE_WRITEX_BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) {
    DEBUG(0,("Invalid packet length! (%lu bytes).\n",
             (unsigned long)len));
    return false;
}