[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Mon Apr 19 15:36:30 MDT 2010


The branch, master has been updated
       via  60d3692... Now SMB2 error messages are correctly being returned with the 1 byte data area, smbd_smb2_request_error_ex() must call smbd_smb2_request_done_ex() in order to do the padding correctly on compound replies.
       via  8bf7942... Ensure vectors are always allocated with consistent size. Removes one byte alloc on SMB2 error packet. Always use talloc_zero_array on out vectors - fixes valgrind errors in tevent writes.
       via  eacf5b2... Fix valgrind error where a strdup of name reads one byte beyond the end. Ensure buffer returned from inotify is null terminated.
      from  4fc5908... Removed more excess looping and fixed problem with incorrect IO flag handling.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 60d36929189eb8c5749431a4d90266b34c26b0c3
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Apr 19 14:32:08 2010 -0700

    Now SMB2 error messages are correctly being returned with the 1 byte data area, smbd_smb2_request_error_ex() must call smbd_smb2_request_done_ex() in order to do the padding correctly on compound replies.
    
    Jeremy.

commit 8bf7942fa4a5aceda3b01e9d5ad555a444b80faa
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Apr 19 13:43:42 2010 -0700

    Ensure vectors are always allocated with consistent size. Removes one byte alloc on SMB2 error packet. Always use talloc_zero_array on out vectors - fixes valgrind errors in tevent writes.
    
    Jeremy.

commit eacf5b235dd27ef844ebee80ded37ec7ecdf8ab2
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Apr 19 13:42:55 2010 -0700

    Fix valgrind error where a strdup of name reads one byte beyond the end. Ensure buffer returned from inotify is null terminated.
    
    Jeremy.

-----------------------------------------------------------------------

Summary of changes:
 source3/smbd/notify_inotify.c |    3 +-
 source3/smbd/smb2_server.c    |  166 +++++++++++++++++++++++++----------------
 2 files changed, 104 insertions(+), 65 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/notify_inotify.c b/source3/smbd/notify_inotify.c
index 6159945..b1d424d 100644
--- a/source3/smbd/notify_inotify.c
+++ b/source3/smbd/notify_inotify.c
@@ -246,8 +246,9 @@ static void inotify_handler(struct event_context *ev, struct fd_event *fde,
 		return;
 	}
 
-	e0 = e = (struct inotify_event *)TALLOC_SIZE(in, bufsize);
+	e0 = e = (struct inotify_event *)TALLOC_SIZE(in, bufsize + 1);
 	if (e == NULL) return;
+	((uint8_t *)e)[bufsize] = '\0';
 
 	status = read_data(in->fd, (char *)e0, bufsize);
 	if (!NT_STATUS_IS_OK(status)) {
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 72c2dbd..277a79f 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -23,6 +23,8 @@
 #include "../libcli/smb/smb_common.h"
 #include "../lib/tsocket/tsocket.h"
 
+#define OUTVEC_ALLOC_SIZE (SMB2_HDR_BODY + 9)
+
 static const char *smb2_names[] = {
 	"SMB2_NEGPROT",
 	"SMB2_SESSSETUP",
@@ -377,7 +379,7 @@ static NTSTATUS smbd_smb2_request_setup_out(struct smbd_smb2_request *req, uint1
 	int idx;
 
 	count = req->in.vector_count;
-	vector = talloc_array(req, struct iovec, count);
+	vector = talloc_zero_array(req, struct iovec, count);
 	if (vector == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -395,15 +397,16 @@ static NTSTATUS smbd_smb2_request_setup_out(struct smbd_smb2_request *req, uint1
 		struct iovec *current = &vector[idx];
 
 		if ((idx + 3) < count) {
-			/* we have a next command */
-			next_command_ofs = SMB2_HDR_BODY + 8;
+			/* we have a next command -
+			 * setup for the error case. */
+			next_command_ofs = SMB2_HDR_BODY + 9;
 		}
 
 		inhdr = (const uint8_t *)req->in.vector[idx].iov_base;
 		in_flags = IVAL(inhdr, SMB2_HDR_FLAGS);
 
-		outhdr = talloc_array(vector, uint8_t,
-				      SMB2_HDR_BODY + 8);
+		outhdr = talloc_zero_array(vector, uint8_t,
+				      OUTVEC_ALLOC_SIZE);
 		if (outhdr == NULL) {
 			return NT_STATUS_NO_MEMORY;
 		}
@@ -506,7 +509,7 @@ static struct smbd_smb2_request *dup_smb2_req(const struct smbd_smb2_request *re
 	newreq->async = false;
 	newreq->cancelled = false;
 
-	outvec = talloc_array(newreq, struct iovec, count);
+	outvec = talloc_zero_array(newreq, struct iovec, count);
 	if (!outvec) {
 		TALLOC_FREE(newreq);
 		return NULL;
@@ -518,16 +521,43 @@ static struct smbd_smb2_request *dup_smb2_req(const struct smbd_smb2_request *re
 	outvec[0].iov_base = newreq->out.nbt_hdr;
 	outvec[0].iov_len = 4;
 	memcpy(newreq->out.nbt_hdr, req->out.nbt_hdr, 4);
-		
-	for (i = 1; i < count; i++) {
-		if (!dup_smb2_vec(outvec,
+
+	for (i = 1; i < count; i += 3) {
+		/* i + 0 and i + 1 are always
+		 * boilerplate. */
+		outvec[i].iov_base = talloc_memdup(outvec,
+						req->out.vector[i].iov_base,
+						OUTVEC_ALLOC_SIZE);
+		if (!outvec[i].iov_base) {
+			break;
+		}
+		outvec[i].iov_len = SMB2_HDR_BODY;
+
+		outvec[i+1].iov_base = ((uint8_t *)outvec[i].iov_base) +
+						SMB2_HDR_BODY;
+		outvec[i+1].iov_len = 8;
+
+		if (req->out.vector[i+2].iov_base ==
+				((uint8_t *)req->out.vector[i].iov_base) +
+					(OUTVEC_ALLOC_SIZE - 1) &&
+				req->out.vector[i+2].iov_len == 1) {
+			/* Common SMB2 error packet case. */
+			outvec[i+2].iov_base = ((uint8_t *)outvec[i].iov_base) +
+				(OUTVEC_ALLOC_SIZE - 1);
+			outvec[i+2].iov_len = 1;
+		} else if (!dup_smb2_vec(outvec,
 				req->out.vector,
 				i)) {
-			TALLOC_FREE(newreq);
-			return NULL;
+			break;
 		}
 	}
 
+	if (i < count) {
+		/* Alloc failed. */
+		TALLOC_FREE(newreq);
+		return NULL;
+	}
+
 	smb2_setup_nbt_length(newreq->out.vector,
 		newreq->out.vector_count);
 
@@ -775,7 +805,7 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req,
 	smb2_setup_nbt_length(req->in.vector, 4);
 
 	/* Now recreate the out.vectors. */
-	outvec = talloc_array(req, struct iovec, 4);
+	outvec = talloc_zero_array(req, struct iovec, 4);
 	if (!outvec) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -785,7 +815,7 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req,
 
 	outvec[1].iov_base = talloc_memdup(outvec,
 				req->out.vector[i].iov_base,
-				SMB2_HDR_BODY + 8);
+				OUTVEC_ALLOC_SIZE);
 	if (!outvec[1].iov_base) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -797,11 +827,20 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req,
 
 	if (req->out.vector[i+2].iov_base &&
 			req->out.vector[i+2].iov_len) {
-		outvec[3].iov_base = talloc_memdup(outvec,
+		if (req->out.vector[i+2].iov_base ==
+				((uint8_t *)req->out.vector[i].iov_base) +
+					(OUTVEC_ALLOC_SIZE - 1) &&
+				req->out.vector[i].iov_len == 1) {
+			/* Common SMB2 error packet case. */
+			outvec[3].iov_base = ((uint8_t *)outvec[1].iov_base) +
+				(OUTVEC_ALLOC_SIZE - 1);
+		} else {
+			outvec[3].iov_base = talloc_memdup(outvec,
 					req->out.vector[i+2].iov_base,
 					req->out.vector[i+2].iov_len);
-		if (!outvec[3].iov_base) {
-			return NT_STATUS_NO_MEMORY;
+			if (!outvec[3].iov_base) {
+				return NT_STATUS_NO_MEMORY;
+			}
 		}
 		outvec[3].iov_len = req->out.vector[i+2].iov_len;
 	} else {
@@ -1232,51 +1271,6 @@ static void smbd_smb2_request_writev_done(struct tevent_req *subreq)
 	}
 }
 
-NTSTATUS smbd_smb2_request_error_ex(struct smbd_smb2_request *req,
-				    NTSTATUS status,
-				    DATA_BLOB *info,
-				    const char *location)
-{
-	uint8_t *outhdr;
-	uint8_t *outbody;
-	int i = req->current_idx;
-
-	DEBUG(10,("smbd_smb2_request_error_ex: idx[%d] status[%s] |%s| at %s\n",
-		  i, nt_errstr(status), info ? " +info" : "",
-		  location));
-
-	outhdr = (uint8_t *)req->out.vector[i].iov_base;
-
-	SIVAL(outhdr, SMB2_HDR_STATUS, NT_STATUS_V(status));
-
-	outbody = outhdr + SMB2_HDR_BODY;
-	SSVAL(outbody, 0, 9);
-
-	req->out.vector[i+1].iov_base = (void *)outbody;
-	req->out.vector[i+1].iov_len = 8;
-
-	if (info) {
-		SIVAL(outbody, 0x04, info->length);
-		req->out.vector[i+2].iov_base	= (void *)info->data;
-		req->out.vector[i+2].iov_len	= info->length;
-	} else {
-		req->out.vector[i+2].iov_base = talloc_array(req, uint8_t, 1);
-		if (!req->out.vector[i+2].iov_base) {
-			return NT_STATUS_NO_MEMORY;
-		}
-		SCVAL(req->out.vector[i+2].iov_base, 0, 0);
-		req->out.vector[i+2].iov_len = 1;
-	}
-
-	/*
-	 * if a request fails, all other remaining
-	 * compounded requests should fail too
-	 */
-	req->next_status = NT_STATUS_INVALID_PARAMETER;
-
-	return smbd_smb2_request_reply(req);
-}
-
 NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
 				   NTSTATUS status,
 				   DATA_BLOB body, DATA_BLOB *dyn,
@@ -1356,7 +1350,7 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
 			old_dyn = (uint8_t *)req->out.vector[i+2].iov_base;
 
 			new_size = old_size + pad_size;
-			new_dyn = talloc_array(req->out.vector,
+			new_dyn = talloc_zero_array(req->out.vector,
 					       uint8_t, new_size);
 			if (new_dyn == NULL) {
 				return smbd_smb2_request_error(req,
@@ -1368,8 +1362,6 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
 
 			req->out.vector[i+2].iov_base = (void *)new_dyn;
 			req->out.vector[i+2].iov_len = new_size;
-
-			TALLOC_FREE(old_dyn);
 		}
 		next_command_ofs += pad_size;
 	}
@@ -1379,6 +1371,52 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req,
 	return smbd_smb2_request_reply(req);
 }
 
+NTSTATUS smbd_smb2_request_error_ex(struct smbd_smb2_request *req,
+				    NTSTATUS status,
+				    DATA_BLOB *info,
+				    const char *location)
+{
+	DATA_BLOB body;
+	int i = req->current_idx;
+	uint8_t *outhdr = (uint8_t *)req->out.vector[i].iov_base;
+
+	DEBUG(10,("smbd_smb2_request_error_ex: idx[%d] status[%s] |%s| at %s\n",
+		  i, nt_errstr(status), info ? " +info" : "",
+		  location));
+
+	body.data = outhdr + SMB2_HDR_BODY;
+	body.length = 8;
+	SSVAL(body.data, 0, 9);
+
+	if (info) {
+		SIVAL(body.data, 0x04, info->length);
+	} else {
+		/* Allocated size of req->out.vector[i].iov_base
+		 * *MUST BE* OUTVEC_ALLOC_SIZE. So we have room for
+		 * 1 byte without having to do an alloc.
+		 */
+		info = talloc_zero_array(req->out.vector,
+					DATA_BLOB,
+					1);
+		if (!info) {
+			return NT_STATUS_NO_MEMORY;
+		}
+		info->data = ((uint8_t *)outhdr) +
+			OUTVEC_ALLOC_SIZE - 1;
+		info->length = 1;
+		SCVAL(info->data, 0, 0);
+	}
+
+	/*
+	 * if a request fails, all other remaining
+	 * compounded requests should fail too
+	 */
+	req->next_status = NT_STATUS_INVALID_PARAMETER;
+
+	return smbd_smb2_request_done_ex(req, status, body, info, __location__);
+}
+
+
 struct smbd_smb2_send_oplock_break_state {
 	struct smbd_server_connection *sconn;
 	uint8_t buf[4 + SMB2_HDR_BODY + 0x18];


-- 
Samba Shared Repository


More information about the samba-cvs mailing list