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

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;
}