Attachment 9690 Details for Bug 10422 – Patches for master

[patch] Patches for master

tmp.diff (text/plain), 11.74 KB, created by Stefan Metzmacher on 2014-02-18 13:37:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2014-02-18 13:37:48 UTC

Size: 11.74 KB

patch

obsolete

>From 6eb38bb55fc032ea2d0321ce1d6a5a30447a08a8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:28:35 +0100
>Subject: [PATCH 1/8] 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 a1206f9f09c6bc5b2115730e61c20df26d9fbb56 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:57:15 +0100
>Subject: [PATCH 2/8] 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 3384915929bb7c355e3e758a7242deab4ea36659 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:45:35 +0100
>Subject: [PATCH 3/8] 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 41b3611..938ee4c 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -3396,6 +3396,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) {
>@@ -3692,7 +3693,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 06c60c5e65ab03643d448114bb604f8fb6acc99b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:50:49 +0100
>Subject: [PATCH 4/8] 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 938ee4c..41ab9fb 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -3700,7 +3700,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 d81f3fae9c7543075fea3d7139620806052c86b2 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:52:09 +0100
>Subject: [PATCH 5/8] 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 047d84c554b263d05bfc7cfae2f9bb75052e98c8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:53:45 +0100
>Subject: [PATCH 6/8] 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 3ca15d3382c1138f519d4463663a77a3618007df Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:54:41 +0100
>Subject: [PATCH 7/8] 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 |   13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 6b56239..ab40433 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);
> 
>@@ -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.
> 	 */
>-	if (numtoread > sconn->smb1.negprot.max_recv) {
>+	if (numtoread > sconn->smb1.sessions.max_recv) {
> 		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_recv));
>+		numtoread = MIN(numtoread, sconn->smb1.sessions.max_recv);
> 	}
> 
> 	reply_outbuf(req, 5, numtoread+3);
>-- 
>1.7.9.5
>
>
>From c7f33a6b0a04e0ce37401b0a2286833a6c314781 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 6 Dec 2013 13:56:12 +0100
>Subject: [PATCH 8/8] 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 41ab9fb..9457000 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -245,7 +245,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
>

Flags:

jra: review-

Actions: View

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