Attachment 9698 Details for Bug 10422 – Current patchset for master (fails samba3.raw.read(s3dc))

[patch] Current patchset for master (fails samba3.raw.read(s3dc))

tmp.diff (text/plain), 16.50 KB, created by Stefan Metzmacher on 2014-02-19 15:03:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2014-02-19 15:03:35 UTC

Size: 16.50 KB

patch

obsolete

>From 34738ead4d94d65cb0ca5d9e19113e402fa64c25 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 19 Feb 2014 13:54:44 +0100
>Subject: [PATCH 01/12] s3:utils/smbfilter: use a local variable for the
> packet buffer
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/utils/smbfilter.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/utils/smbfilter.c b/source3/utils/smbfilter.c
>index 33f1a90..734971b 100644
>--- a/source3/utils/smbfilter.c
>+++ b/source3/utils/smbfilter.c
>@@ -35,7 +35,6 @@
> #define CLI_CAPABILITY_SET  0
> 
> static char *netbiosname;
>-static char packet[BUFFER_SIZE];
> 
> static void save_file(const char *fname, void *ppacket, size_t length)
> {
>@@ -178,6 +177,7 @@ static void filter_child(int c, struct sockaddr_storage *dest_ss)
> {
> 	NTSTATUS status;
> 	int s = -1;
>+	uint8_t packet[128*1024];
> 
> 	/* we have a connection from a new client, now connect to the server */
> 	status = open_socket_out(dest_ss, TCP_SMB_PORT, LONG_CONNECT_TIMEOUT, &s);
>-- 
>1.7.9.5
>
>
>From bae5bedc461a44be6fdcf972655fb253e59eb9ef Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 19 Feb 2014 13:56:06 +0100
>Subject: [PATCH 02/12] s3:torture: use CLI_BUFFER_SIZE instead of BUFFER_SIZE
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/torture/torture.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index 1f29a70..340f754 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -3087,7 +3087,7 @@ static bool run_randomipc(int dummy)
> 
> 		cli_api(cli, 
> 			param, param_len, 8,  
>-			NULL, 0, BUFFER_SIZE, 
>+			NULL, 0, CLI_BUFFER_SIZE,
> 			&rparam, &rprcnt,     
> 			&rdata, &rdrcnt);
> 		if (i % 100 == 0) {
>-- 
>1.7.9.5
>
>
>From 76eef78fb2d574a2430940e64a0ab6dd0e2b916a Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 19 Feb 2014 13:57:28 +0100
>Subject: [PATCH 03/12] s3:client: only limit the buffer by the given length
> 'n'
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/client/client.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/client/client.c b/source3/client/client.c
>index 16cbfe8..7ecf097 100644
>--- a/source3/client/client.c
>+++ b/source3/client/client.c
>@@ -232,7 +232,7 @@ static int readfile(uint8_t *b, int n, XFILE *f)
> 		return x_fread(b,1,n,f);
> 
> 	i = 0;
>-	while (i < (n - 1) && (i < BUFFER_SIZE)) {
>+	while (i < (n - 1)) {
> 		if ((c = x_getc(f)) == EOF) {
> 			break;
> 		}
>-- 
>1.7.9.5
>
>
>From fa1550e3ff7477c02f36e925fdccd1555d490774 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 19 Feb 2014 13:59:07 +0100
>Subject: [PATCH 04/12] s3:param: avoid using BUFFER_SIZE to limit the
> lp_min_receive_file_size()
>
>The current value of BUFFER_SIZE 128*1024 means receive_file is disabled
>for SMB1 clients. There's really no reason to add such limit.
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/param/loadparm.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
>index f2d1e6b..28aa35a 100644
>--- a/source3/param/loadparm.c
>+++ b/source3/param/loadparm.c
>@@ -5205,7 +5205,7 @@ int lp_min_receive_file_size(void)
> 	if (Globals.iminreceivefile < 0) {
> 		return 0;
> 	}
>-	return MIN(Globals.iminreceivefile, BUFFER_SIZE);
>+	return Globals.iminreceivefile;
> }
> 
> /*******************************************************************
>-- 
>1.7.9.5
>
>
>From b45752e15076f686a2d5985713e0680b5609f472 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:28:35 +0100
>Subject: [PATCH 05/12] libcli/smb: add SMB_BUFFER_SIZE_MIN/MAX defines
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> libcli/smb/smb_constants.h |    8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/libcli/smb/smb_constants.h b/libcli/smb/smb_constants.h
>index 4b24be0..4190e64 100644
>--- a/libcli/smb/smb_constants.h
>+++ b/libcli/smb/smb_constants.h
>@@ -209,6 +209,14 @@ enum smb_signing_setting {
> #define NEGOTIATE_SECURITY_SIGNATURES_ENABLED	0x04
> #define NEGOTIATE_SECURITY_SIGNATURES_REQUIRED	0x08
> 
>+/*
>+ * The negotiated buffer size for non LARGE_READX/WRITEX
>+ * should be limited to uint16_t and has to be at least
>+ * 500, which is the default for MinClientBufferSize on Windows.
>+ */
>+#define SMB_BUFFER_SIZE_MIN 500
>+#define SMB_BUFFER_SIZE_MAX 65535
>+
> /* Capabilities.  see ftp.microsoft.com/developr/drg/cifs/cifs/cifs4.txt */
> 
> #define CAP_RAW_MODE		0x00000001
>-- 
>1.7.9.5
>
>
>From 75c4b39ae94edefd72ff6c0f37630f957ec03a48 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:57:15 +0100
>Subject: [PATCH 06/12] s3:include: let CLI_BUFFER_SIZE be an alias of
> SMB_BUFFER_SIZE_MAX
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/include/client.h |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/include/client.h b/source3/include/client.h
>index 35fa2f1..59fb104 100644
>--- a/source3/include/client.h
>+++ b/source3/include/client.h
>@@ -22,7 +22,7 @@
> #ifndef _CLIENT_H
> #define _CLIENT_H
> 
>-#define CLI_BUFFER_SIZE (0xFFFF)
>+#define CLI_BUFFER_SIZE SMB_BUFFER_SIZE_MAX
> 
> /* default client timeout to 20 seconds on most commands */
> #define CLIENT_TIMEOUT (20 * 1000)
>-- 
>1.7.9.5
>
>
>From c47614e37c6bd41057b87d1c8e2e7951ccdf4c79 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:45:35 +0100
>Subject: [PATCH 07/12] s3:smbd: use SMB_BUFFER_SIZE_MIN/MAX to limit
> lp_max_xmit()
>
>The current limit of 128*1024 causes problems as the value has to be
><= UINT16_MAX otherwise some clients get confused, as they want to
>use the MaxBufferSize value from the negprot response (uint32_t)
>for the MaxBufferSize value in thet session setup request (uint16_t).
>E.g. Windows 7 (as client) sends MaxBufferSize = 0 if the server value
>is > UINT16_MAX.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/process.c |    7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index f5ca2f9..df9d30c 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -3332,6 +3332,7 @@ void smbd_process(struct tevent_context *ev_ctx,
> 	const char *remaddr = NULL;
> 	char *rhost;
> 	int ret;
>+	int tmp;
> 
> 	conn = talloc_zero(ev_ctx, struct smbXsrv_connection);
> 	if (conn == NULL) {
>@@ -3628,7 +3629,11 @@ void smbd_process(struct tevent_context *ev_ctx,
> 
> 	sconn->nbt.got_session = false;
> 
>-	sconn->smb1.negprot.max_recv = MIN(lp_max_xmit(),BUFFER_SIZE);
>+	tmp = lp_max_xmit();
>+	tmp = MAX(tmp, SMB_BUFFER_SIZE_MIN);
>+	tmp = MIN(tmp, SMB_BUFFER_SIZE_MAX);
>+
>+	sconn->smb1.negprot.max_recv = tmp;
> 
> 	sconn->smb1.sessions.done_sesssetup = false;
> 	sconn->smb1.sessions.max_send = BUFFER_SIZE;
>-- 
>1.7.9.5
>
>
>From 2884e151ff7d2a0f7e46ecd88a3045df50b7e84e Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:50:49 +0100
>Subject: [PATCH 08/12] s3:smbd: use sconn->smb1.sessions.max_send =
> SMB_BUFFER_SIZE_MAX
>
>SMB_BUFFER_SIZE_MAX is UINT16_MAX and the largest value a client
>can possibly specify in the session setup request.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/process.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index df9d30c..390b3ad 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -3636,7 +3636,7 @@ void smbd_process(struct tevent_context *ev_ctx,
> 	sconn->smb1.negprot.max_recv = tmp;
> 
> 	sconn->smb1.sessions.done_sesssetup = false;
>-	sconn->smb1.sessions.max_send = BUFFER_SIZE;
>+	sconn->smb1.sessions.max_send = SMB_BUFFER_SIZE_MAX;
> 
> 	if (!init_dptrs(sconn)) {
> 		exit_server("init_dptrs() failed");
>-- 
>1.7.9.5
>
>
>From f5d77b97ace22e17cded653958081122f8360ac3 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:52:09 +0100
>Subject: [PATCH 09/12] s3:smbd: reject a MaxBufferSize < SMB_BUFFER_SIZE_MIN
> (500) in a session setup request
>
>This makes sure sconn->smb1.sessions.max_send is always >= SMB_BUFFER_SIZE_MIN.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/sesssetup.c |   19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
>diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c
>index 02cb445..4b86a99 100644
>--- a/source3/smbd/sesssetup.c
>+++ b/source3/smbd/sesssetup.c
>@@ -383,10 +383,13 @@ static void reply_sesssetup_and_X_spnego(struct smb_request *req)
> 		}
> 
> 		if (!sconn->smb1.sessions.done_sesssetup) {
>-			sconn->smb1.sessions.max_send =
>-				MIN(sconn->smb1.sessions.max_send,smb_bufsize);
>+			if (smb_bufsize < SMB_BUFFER_SIZE_MIN) {
>+				reply_force_doserror(req, ERRSRV, ERRerror);
>+				return;
>+			}
>+			sconn->smb1.sessions.max_send = smb_bufsize;
>+			sconn->smb1.sessions.done_sesssetup = true;
> 		}
>-		sconn->smb1.sessions.done_sesssetup = true;
> 
> 		/* current_user_info is changed on new vuid */
> 		reload_services(sconn, conn_snum_used, true);
>@@ -1088,10 +1091,14 @@ void reply_sesssetup_and_X(struct smb_request *req)
> 	req->vuid = sess_vuid;
> 
> 	if (!sconn->smb1.sessions.done_sesssetup) {
>-		sconn->smb1.sessions.max_send =
>-			MIN(sconn->smb1.sessions.max_send,smb_bufsize);
>+		if (smb_bufsize < SMB_BUFFER_SIZE_MIN) {
>+			reply_force_doserror(req, ERRSRV, ERRerror);
>+			END_PROFILE(SMBsesssetupX);
>+			return;
>+		}
>+		sconn->smb1.sessions.max_send = smb_bufsize;
>+		sconn->smb1.sessions.done_sesssetup = true;
> 	}
>-	sconn->smb1.sessions.done_sesssetup = true;
> 
> 	END_PROFILE(SMBsesssetupX);
> }
>-- 
>1.7.9.5
>
>
>From d81612cb455b15360e3afabb2614e6a2238a8a95 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:53:45 +0100
>Subject: [PATCH 10/12] s3:smbd: take less than SMB_BUFFER_SIZE_MIN ('500') as
> header overhead in ipc.c
>
>We're now sure that sconn->smb1.sessions.max_send is >= SMB_BUFFER_SIZE_MIN.
>in order to garantee some progress we need to make sure our assumed
>header overhead is less than SMB_BUFFER_SIZE_MIN.
>
>Assuming 372 bytes for the SMBtrans headers should still be more than
>enough.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/ipc.c |   10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/ipc.c b/source3/smbd/ipc.c
>index 91d5047..dbb259c 100644
>--- a/source3/smbd/ipc.c
>+++ b/source3/smbd/ipc.c
>@@ -109,12 +109,14 @@ void send_trans_reply(connection_struct *conn,
> 	int lparam = rparam ? rparam_len : 0;
> 	struct smbd_server_connection *sconn = req->sconn;
> 	int max_send = sconn->smb1.sessions.max_send;
>+	/* HACK: make sure we send at least 128 byte in one go */
>+	int hdr_overhead = SMB_BUFFER_SIZE_MIN - 128;
> 
> 	if (buffer_too_large)
> 		DEBUG(5,("send_trans_reply: buffer %d too large\n", ldata ));
> 
>-	this_lparam = MIN(lparam,max_send - 500); /* hack */
>-	this_ldata  = MIN(ldata,max_send - (500+this_lparam));
>+	this_lparam = MIN(lparam,max_send - hdr_overhead);
>+	this_ldata  = MIN(ldata,max_send - (hdr_overhead+this_lparam));
> 
> 	align = ((this_lparam)%4);
> 
>@@ -163,9 +165,9 @@ void send_trans_reply(connection_struct *conn,
> 	while (tot_data_sent < ldata || tot_param_sent < lparam)
> 	{
> 		this_lparam = MIN(lparam-tot_param_sent,
>-				  max_send - 500); /* hack */
>+				  max_send - hdr_overhead);
> 		this_ldata  = MIN(ldata -tot_data_sent,
>-				  max_send - (500+this_lparam));
>+				  max_send - (hdr_overhead+this_lparam));
> 
> 		if(this_lparam < 0)
> 			this_lparam = 0;
>-- 
>1.7.9.5
>
>
>From 97c5cfea677c0eab93e02625a9422309e8725609 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:54:41 +0100
>Subject: [PATCH 11/12] s3:smbd: use sconn->smb1.sessions.max_recv instead of
> BUFFER_SIZE
>
>The calculation for the possible buffer size should use the client
>provided limits.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/reply.c |   25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 6b56239..4c697b6 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -1700,7 +1700,7 @@ void reply_search(struct smb_request *req)
> 		unsigned int i;
> 		maxentries = MIN(
> 			maxentries,
>-			((BUFFER_SIZE -
>+			((req->sconn->smb1.sessions.max_send -
> 			  ((uint8 *)smb_buf(req->outbuf) + 3 - req->outbuf))
> 			 /DIR_STRUCT_SIZE));
> 
>@@ -3490,7 +3490,8 @@ void reply_lockread(struct smb_request *req)
> 	numtoread = SVAL(req->vwv+1, 0);
> 	startpos = IVAL_TO_SMB_OFF_T(req->vwv+2, 0);
> 
>-	numtoread = MIN(BUFFER_SIZE - (smb_size + 3*2 + 3), numtoread);
>+	numtoread = MIN(sconn->smb1.sessions.max_send - (smb_size + 3*2 + 3),
>+			numtoread);
> 
> 	reply_outbuf(req, 5, numtoread + 3);
> 
>@@ -3501,7 +3502,7 @@ void reply_lockread(struct smb_request *req)
> 	 * protocol request that predates the read/write lock concept. 
> 	 * Thus instead of asking for a read lock here we need to ask
> 	 * for a write lock. JRA.
>-	 * Note that the requested lock size is unaffected by max_recv.
>+	 * Note that the requested lock size is unaffected by max_send.
> 	 */
> 
> 	br_lck = do_lock(req->sconn->msg_ctx,
>@@ -3524,15 +3525,15 @@ void reply_lockread(struct smb_request *req)
> 	}
> 
> 	/*
>-	 * However the requested READ size IS affected by max_recv. Insanity.... JRA.
>+	 * However the requested READ size IS affected by max_send. Insanity.... JRA.
> 	 */
> 
>-	if (numtoread > sconn->smb1.negprot.max_recv) {
>+	if (numtoread > sconn->smb1.sessions.max_send) {
> 		DEBUG(0,("reply_lockread: requested read size (%u) is greater than maximum allowed (%u). \
> Returning short read of maximum allowed for compatibility with Windows 2000.\n",
> 			(unsigned int)numtoread,
>-			(unsigned int)sconn->smb1.negprot.max_recv));
>-		numtoread = MIN(numtoread, sconn->smb1.negprot.max_recv);
>+			(unsigned int)sconn->smb1.sessions.max_send));
>+		numtoread = MIN(numtoread, sconn->smb1.sessions.max_send);
> 	}
> 	nread = read_file(fsp,data,startpos,numtoread);
> 
>@@ -3600,17 +3601,15 @@ void reply_read(struct smb_request *req)
> 	numtoread = SVAL(req->vwv+1, 0);
> 	startpos = IVAL_TO_SMB_OFF_T(req->vwv+2, 0);
> 
>-	numtoread = MIN(BUFFER_SIZE-outsize,numtoread);
>-
> 	/*
>-	 * The requested read size cannot be greater than max_recv. JRA.
>+	 * The requested read size cannot be greater than max_send. JRA.
> 	 */
>-	if (numtoread > sconn->smb1.negprot.max_recv) {
>+	if (numtoread > sconn->smb1.sessions.max_send) {
> 		DEBUG(0,("reply_read: requested read size (%u) is greater than maximum allowed (%u). \
> Returning short read of maximum allowed for compatibility with Windows 2000.\n",
> 			(unsigned int)numtoread,
>-			(unsigned int)sconn->smb1.negprot.max_recv));
>-		numtoread = MIN(numtoread, sconn->smb1.negprot.max_recv);
>+			(unsigned int)sconn->smb1.sessions.max_send));
>+		numtoread = sconn->smb1.sessions.max_send;
> 	}
> 
> 	reply_outbuf(req, 5, numtoread+3);
>-- 
>1.7.9.5
>
>
>From 5bc644ac5906bb6d3fcf9eb36f864a4f5fcfd5b6 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:56:12 +0100
>Subject: [PATCH 12/12] s3:smbd: s/BUFFER_SIZE/LARGE_WRITEX_BUFFER_SIZE
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=10422
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/include/smb.h  |    4 +---
> source3/smbd/process.c |    2 +-
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/source3/include/smb.h b/source3/include/smb.h
>index 1f6813e..aab4ff5 100644
>--- a/source3/include/smb.h
>+++ b/source3/include/smb.h
>@@ -32,11 +32,9 @@
> /* logged when starting the various Samba daemons */
> #define COPYRIGHT_STARTUP_MESSAGE	"Copyright Andrew Tridgell and the Samba Team 1992-2014"
> 
>-
>-#define BUFFER_SIZE (128*1024)
>-
> #define SAFETY_MARGIN 1024
> #define LARGE_WRITEX_HDR_SIZE 65
>+#define LARGE_WRITEX_BUFFER_SIZE (128*1024)
> 
> #define NMB_PORT 137
> #define DGRAM_PORT 138
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index 390b3ad..930f2ae 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -238,7 +238,7 @@ static bool valid_packet_size(size_t len)
> 	 * of header. Don't print the error if this fits.... JRA.
> 	 */
> 
>-	if (len > (BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) {
>+	if (len > (LARGE_WRITEX_BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) {
> 		DEBUG(0,("Invalid packet length! (%lu bytes).\n",
> 					(unsigned long)len));
> 		return false;
>-- 
>1.7.9.5
>

Actions: View

Attachments on bug 10422: 9673 | 9690 | 9691 | 9698 | 9792 | 9793