Attachment 15634 Details for Bug 14205 – git-am fix for master.

[patch] git-am fix for master.

bug-14205-master (text/plain), 15.60 KB, created by Jeremy Allison on 2019-11-26 21:31:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2019-11-26 21:31:56 UTC

Size: 15.60 KB

patch

obsolete

>From 37b2a73cd7499a78fc4adb1ffd86b53d829a385b Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 26 Nov 2019 12:14:29 -0800
>Subject: [PATCH 1/6] s3: smbd: Allow smbd_smb2_process_negprot() to return
> NTSTATUS as it can fail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/globals.h     |  2 +-
> source3/smbd/smb2_server.c | 15 ++++++++-------
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
>diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
>index c791eb0fa6f..a5e02ebc96a 100644
>--- a/source3/smbd/globals.h
>+++ b/source3/smbd/globals.h
>@@ -234,7 +234,7 @@ NTSTATUS smbd_add_connection(struct smbXsrv_client *client, int sock_fd,
> 
> void reply_smb2002(struct smb_request *req, uint16_t choice);
> void reply_smb20ff(struct smb_request *req, uint16_t choice);
>-void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
>+NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
> 			       uint64_t expected_seq_low,
> 			       const uint8_t *inpdu, size_t size);
> 
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index 7044ecb991a..9cf8841d827 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -3629,7 +3629,7 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbXsrv_connection *xconn
> 	return NT_STATUS_OK;
> }
> 
>-void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
>+NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
> 			       uint64_t expected_seq_low,
> 			       const uint8_t *inpdu, size_t size)
> {
>@@ -3643,25 +3643,25 @@ void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
> 	status = smbd_initialize_smb2(xconn, expected_seq_low);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		smbd_server_connection_terminate(xconn, nt_errstr(status));
>-		return;
>+		return status;
> 	}
> 
> 	status = smbd_smb2_request_create(xconn, inpdu, size, &req);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		smbd_server_connection_terminate(xconn, nt_errstr(status));
>-		return;
>+		return status;
> 	}
> 
> 	status = smbd_smb2_request_validate(req);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		smbd_server_connection_terminate(xconn, nt_errstr(status));
>-		return;
>+		return status;
> 	}
> 
> 	status = smbd_smb2_request_setup_out(req);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		smbd_server_connection_terminate(xconn, nt_errstr(status));
>-		return;
>+		return status;
> 	}
> 
> #ifdef WITH_PROFILE
>@@ -3676,16 +3676,17 @@ void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
> 	status = smbd_smb2_request_dispatch(req);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		smbd_server_connection_terminate(xconn, nt_errstr(status));
>-		return;
>+		return status;
> 	}
> 
> 	status = smbd_smb2_request_next_incoming(xconn);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		smbd_server_connection_terminate(xconn, nt_errstr(status));
>-		return;
>+		return status;
> 	}
> 
> 	sconn->num_requests++;
>+	return NT_STATUS_OK;
> }
> 
> static int socket_error_from_errno(int ret,
>-- 
>2.24.0.432.g9d3f5f5b63-goog
>
>
>From 05400c24099697959763ff584b5d54e7a66a2a15 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 26 Nov 2019 12:17:29 -0800
>Subject: [PATCH 2/6] s3: smbd: Ensure we exit on smbd_smb2_process_negprot()
> fail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/process.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/process.c b/source3/smbd/process.c
>index 0e0d45d2af1..fc43519a614 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -1968,7 +1968,14 @@ static void process_smb(struct smbXsrv_connection *xconn,
> 		if (smbd_is_smb2_header(inbuf, nread)) {
> 			const uint8_t *inpdu = inbuf + NBT_HDR_SIZE;
> 			size_t pdulen = nread - NBT_HDR_SIZE;
>-			smbd_smb2_process_negprot(xconn, 0, inpdu, pdulen);
>+			NTSTATUS status = smbd_smb2_process_negprot(
>+						xconn,
>+						0,
>+						inpdu,
>+						pdulen);
>+			if (!NT_STATUS_IS_OK(status)) {
>+				exit_server_cleanly("SMB2 negprot fail");
>+			}
> 			return;
> 		}
> 		if (nread >= smb_size && valid_smb_header(inbuf)
>-- 
>2.24.0.432.g9d3f5f5b63-goog
>
>
>From 265399109ac0f0a46e8f7da41b6cbd59e5467d48 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 26 Nov 2019 12:21:06 -0800
>Subject: [PATCH 3/6] s3: smbd: Change reply_smb20xx() to return NTSTATUS.
>
>Not yet used.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/smb2_negprot.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
>index 6e7201b1cd8..b07fc972cbb 100644
>--- a/source3/smbd/smb2_negprot.c
>+++ b/source3/smbd/smb2_negprot.c
>@@ -36,7 +36,7 @@ extern fstring remote_proto;
>  * this is the entry point if SMB2 is selected via
>  * the SMB negprot and the given dialect.
>  */
>-static void reply_smb20xx(struct smb_request *req, uint16_t dialect)
>+static NTSTATUS reply_smb20xx(struct smb_request *req, uint16_t dialect)
> {
> 	uint8_t *smb2_inpdu;
> 	uint8_t *smb2_hdr;
>@@ -48,7 +48,7 @@ static void reply_smb20xx(struct smb_request *req, uint16_t dialect)
> 	if (smb2_inpdu == NULL) {
> 		DEBUG(0, ("Could not push spnego blob\n"));
> 		reply_nterror(req, NT_STATUS_NO_MEMORY);
>-		return;
>+		return NT_STATUS_NO_MEMORY;
> 	}
> 	smb2_hdr = smb2_inpdu;
> 	smb2_body = smb2_hdr + SMB2_HDR_BODY;
>@@ -64,8 +64,7 @@ static void reply_smb20xx(struct smb_request *req, uint16_t dialect)
> 
> 	req->outbuf = NULL;
> 
>-	smbd_smb2_process_negprot(req->xconn, 0, smb2_inpdu, len);
>-	return;
>+	return smbd_smb2_process_negprot(req->xconn, 0, smb2_inpdu, len);
> }
> 
> /*
>-- 
>2.24.0.432.g9d3f5f5b63-goog
>
>
>From b9be8393a9ee134d3f08770b92a712e677664a9e Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 26 Nov 2019 12:43:25 -0800
>Subject: [PATCH 4/6] s3: smbd: Change (*proto_reply_fn()) to return an
> NTSTATUS.
>
>That way the caller can know if the negprot really
>succeeded or not.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/globals.h      |  4 ++--
> source3/smbd/negprot.c      | 29 +++++++++++++++--------------
> source3/smbd/smb2_negprot.c |  8 ++++----
> 3 files changed, 21 insertions(+), 20 deletions(-)
>
>diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
>index a5e02ebc96a..e6641837668 100644
>--- a/source3/smbd/globals.h
>+++ b/source3/smbd/globals.h
>@@ -232,8 +232,8 @@ bool smbd_smb2_is_compound(const struct smbd_smb2_request *req);
> NTSTATUS smbd_add_connection(struct smbXsrv_client *client, int sock_fd,
> 			     struct smbXsrv_connection **_xconn);
> 
>-void reply_smb2002(struct smb_request *req, uint16_t choice);
>-void reply_smb20ff(struct smb_request *req, uint16_t choice);
>+NTSTATUS reply_smb2002(struct smb_request *req, uint16_t choice);
>+NTSTATUS reply_smb20ff(struct smb_request *req, uint16_t choice);
> NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
> 			       uint64_t expected_seq_low,
> 			       const uint8_t *inpdu, size_t size);
>diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c
>index 2d5edf1282c..3b2555e3d21 100644
>--- a/source3/smbd/negprot.c
>+++ b/source3/smbd/negprot.c
>@@ -66,7 +66,7 @@ static void get_challenge(struct smbXsrv_connection *xconn, uint8_t buff[8])
>  Reply for the lanman 1.0 protocol.
> ****************************************************************************/
> 
>-static void reply_lanman1(struct smb_request *req, uint16_t choice)
>+static NTSTATUS reply_lanman1(struct smb_request *req, uint16_t choice)
> {
> 	int secword=0;
> 	time_t t = time(NULL);
>@@ -100,7 +100,7 @@ static void reply_lanman1(struct smb_request *req, uint16_t choice)
> 	status = smbXsrv_connection_init_tables(xconn, PROTOCOL_LANMAN1);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		reply_nterror(req, status);
>-		return;
>+		return status;
> 	}
> 
> 	/* Reply, SMBlockread, SMBwritelock supported. */
>@@ -115,14 +115,14 @@ static void reply_lanman1(struct smb_request *req, uint16_t choice)
> 
> 	srv_put_dos_date((char *)req->outbuf,smb_vwv8,t);
> 
>-	return;
>+	return NT_STATUS_OK;
> }
> 
> /****************************************************************************
>  Reply for the lanman 2.0 protocol.
> ****************************************************************************/
> 
>-static void reply_lanman2(struct smb_request *req, uint16_t choice)
>+static NTSTATUS reply_lanman2(struct smb_request *req, uint16_t choice)
> {
> 	int secword=0;
> 	time_t t = time(NULL);
>@@ -158,7 +158,7 @@ static void reply_lanman2(struct smb_request *req, uint16_t choice)
> 	status = smbXsrv_connection_init_tables(xconn, PROTOCOL_LANMAN2);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		reply_nterror(req, status);
>-		return;
>+		return status;
> 	}
> 
> 	/* Reply, SMBlockread, SMBwritelock supported. */
>@@ -169,6 +169,7 @@ static void reply_lanman2(struct smb_request *req, uint16_t choice)
> 	SSVAL(req->outbuf,smb_vwv5,raw); /* readbraw and/or writebraw */
> 	SSVAL(req->outbuf,smb_vwv10, set_server_zone_offset(t)/60);
> 	srv_put_dos_date((char *)req->outbuf,smb_vwv8,t);
>+	return NT_STATUS_OK;
> }
> 
> /****************************************************************************
>@@ -266,7 +267,7 @@ DATA_BLOB negprot_spnego(TALLOC_CTX *ctx, struct smbXsrv_connection *xconn)
>  Reply for the nt protocol.
> ****************************************************************************/
> 
>-static void reply_nt1(struct smb_request *req, uint16_t choice)
>+static NTSTATUS reply_nt1(struct smb_request *req, uint16_t choice)
> {
> 	/* dual names + lock_and_read + nt SMBs + remote API calls */
> 	int capabilities = CAP_NT_FIND|CAP_LOCK_AND_READ|
>@@ -359,7 +360,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
> 	status = smbXsrv_connection_init_tables(xconn, PROTOCOL_NT1);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		reply_nterror(req, status);
>-		return;
>+		return status;
> 	}
> 
> 	SSVAL(req->outbuf,smb_vwv1+1, lp_max_mux()); /* maxmpx */
>@@ -385,7 +386,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
> 			if (ret == -1) {
> 				DEBUG(0, ("Could not push challenge\n"));
> 				reply_nterror(req, NT_STATUS_NO_MEMORY);
>-				return;
>+				return NT_STATUS_NO_MEMORY;
> 			}
> 			SCVAL(req->outbuf, smb_vwv16+1, ret);
> 		}
>@@ -395,7 +396,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
> 		if (ret == -1) {
> 			DEBUG(0, ("Could not push workgroup string\n"));
> 			reply_nterror(req, NT_STATUS_NO_MEMORY);
>-			return;
>+			return NT_STATUS_NO_MEMORY;
> 		}
> 		ret = message_push_string(&req->outbuf, lp_netbios_name(),
> 					  STR_UNICODE|STR_TERMINATE
>@@ -403,7 +404,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
> 		if (ret == -1) {
> 			DEBUG(0, ("Could not push netbios name string\n"));
> 			reply_nterror(req, NT_STATUS_NO_MEMORY);
>-			return;
>+			return NT_STATUS_NO_MEMORY;
> 		}
> 		DEBUG(3,("not using SPNEGO\n"));
> 	} else {
>@@ -411,14 +412,14 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
> 
> 		if (spnego_blob.data == NULL) {
> 			reply_nterror(req, NT_STATUS_NO_MEMORY);
>-			return;
>+			return NT_STATUS_NO_MEMORY;
> 		}
> 
> 		ret = message_push_blob(&req->outbuf, spnego_blob);
> 		if (ret == -1) {
> 			DEBUG(0, ("Could not push spnego blob\n"));
> 			reply_nterror(req, NT_STATUS_NO_MEMORY);
>-			return;
>+			return NT_STATUS_NO_MEMORY;
> 		}
> 		data_blob_free(&spnego_blob);
> 
>@@ -426,7 +427,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
> 		DEBUG(3,("using SPNEGO\n"));
> 	}
> 
>-	return;
>+	return NT_STATUS_OK;
> }
> 
> /* these are the protocol lists used for auto architecture detection:
>@@ -540,7 +541,7 @@ protocol [SMB 2.???]
> static const struct {
> 	const char *proto_name;
> 	const char *short_name;
>-	void (*proto_reply_fn)(struct smb_request *req, uint16_t choice);
>+	NTSTATUS (*proto_reply_fn)(struct smb_request *req, uint16_t choice);
> 	int protocol_level;
> } supported_protocols[] = {
> 	{"SMB 2.???",               "SMB2_FF",  reply_smb20ff,  PROTOCOL_SMB2_10},
>diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
>index b07fc972cbb..2c1d4185b28 100644
>--- a/source3/smbd/smb2_negprot.c
>+++ b/source3/smbd/smb2_negprot.c
>@@ -71,20 +71,20 @@ static NTSTATUS reply_smb20xx(struct smb_request *req, uint16_t dialect)
>  * this is the entry point if SMB2 is selected via
>  * the SMB negprot and the "SMB 2.002" dialect.
>  */
>-void reply_smb2002(struct smb_request *req, uint16_t choice)
>+NTSTATUS reply_smb2002(struct smb_request *req, uint16_t choice)
> {
>-	reply_smb20xx(req, SMB2_DIALECT_REVISION_202);
>+	return reply_smb20xx(req, SMB2_DIALECT_REVISION_202);
> }
> 
> /*
>  * this is the entry point if SMB2 is selected via
>  * the SMB negprot and the "SMB 2.???" dialect.
>  */
>-void reply_smb20ff(struct smb_request *req, uint16_t choice)
>+NTSTATUS reply_smb20ff(struct smb_request *req, uint16_t choice)
> {
> 	struct smbXsrv_connection *xconn = req->xconn;
> 	xconn->smb2.allow_2ff = true;
>-	reply_smb20xx(req, SMB2_DIALECT_REVISION_2FF);
>+	return reply_smb20xx(req, SMB2_DIALECT_REVISION_2FF);
> }
> 
> enum protocol_types smbd_smb2_protocol_dialect_match(const uint8_t *indyn,
>-- 
>2.24.0.432.g9d3f5f5b63-goog
>
>
>From f4a3443ab5e9f0a90931b7c67a8d5fc2956d4204 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 26 Nov 2019 12:46:16 -0800
>Subject: [PATCH 5/6] s3: smbd: Ensure we exit if
> supported_protocols[protocol].proto_reply_fn() fails.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/negprot.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c
>index 3b2555e3d21..8317dc49086 100644
>--- a/source3/smbd/negprot.c
>+++ b/source3/smbd/negprot.c
>@@ -580,6 +580,7 @@ void reply_negprot(struct smb_request *req)
> 	bool signing_required = true;
> 	int max_proto;
> 	int min_proto;
>+	NTSTATUS status;
> 
> 	START_PROFILE(SMBnegprot);
> 
>@@ -768,7 +769,11 @@ void reply_negprot(struct smb_request *req)
> 
> 	fstrcpy(remote_proto,supported_protocols[protocol].short_name);
> 	reload_services(sconn, conn_snum_used, true);
>-	supported_protocols[protocol].proto_reply_fn(req, choice);
>+	status = supported_protocols[protocol].proto_reply_fn(req, choice);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		exit_server_cleanly("negprot function failed\n");
>+	}
>+
> 	DEBUG(3,("Selected protocol %s\n",supported_protocols[protocol].proto_name));
> 
> 	DBG_INFO("negprot index=%zu\n", choice);
>-- 
>2.24.0.432.g9d3f5f5b63-goog
>
>
>From 1d2f9adaca98f71a81050a723dde3798959a4fc1 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 26 Nov 2019 12:53:09 -0800
>Subject: [PATCH 6/6] s3: smbd: Only set xconn->smb1.negprot.done = true after
> supported_protocols[protocol].proto_reply_fn() succeeds.
>
>Otherwise we can end up with negprot.done set, but
>without smbXsrv_connection_init_tables() being called.
>
>This can cause a client self-crash.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/negprot.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c
>index 8317dc49086..e77c8f52261 100644
>--- a/source3/smbd/negprot.c
>+++ b/source3/smbd/negprot.c
>@@ -588,7 +588,6 @@ void reply_negprot(struct smb_request *req)
> 		END_PROFILE(SMBnegprot);
> 		exit_server_cleanly("multiple negprot's are not permitted");
> 	}
>-	xconn->smb1.negprot.done = true;
> 
> 	if (req->buflen == 0) {
> 		DEBUG(0, ("negprot got no protocols\n"));
>@@ -778,6 +777,8 @@ void reply_negprot(struct smb_request *req)
> 
> 	DBG_INFO("negprot index=%zu\n", choice);
> 
>+	xconn->smb1.negprot.done = true;
>+
> 	/* We always have xconn->smb1.signing_state also for >= SMB2_02 */
> 	signing_required = smb_signing_is_mandatory(xconn->smb1.signing_state);
> 	if (signing_required && (chosen_level < PROTOCOL_NT1)) {
>-- 
>2.24.0.432.g9d3f5f5b63-goog
>

Actions: View

Attachments on bug 14205: 15634 | 15667 | 15668