Attachment 18054 Details for Bug 15432 – git-am fix for 4.19.next

[patch] git-am fix for 4.19.next

bug-15432-4.19.patch (text/plain), 7.00 KB, created by Jeremy Allison on 2023-08-15 17:34:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2023-08-15 17:34:23 UTC

Size: 7.00 KB

patch

obsolete

>From ee8654bdc85816420c201aa49c4a3f540d53b6de Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 11 Aug 2023 17:14:38 -0700
>Subject: [PATCH 1/3] s3: smbd: init_smb1_request() isn't being passed zero'ed
> memory from any codepath.
>
>If a client does a SMB1 NEGPROT followed by SMB1 TCON
>then req->session is left uninitialized.
>
>Show this causes a crash by deliberately initializing
>req->session to an invalid pointer. This will be removed
>once the test shows the crash, and the fix is added to
>cause init_smb1_request() to zero the memory passed in.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Noel Power <npower@samba.org>
>(cherry picked from commit f02f74e931f5821c7b7c1be2b8f0fb60c9a69b19)
>---
> source3/smbd/smb2_process.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c
>index 923810eeff6..99e30481491 100644
>--- a/source3/smbd/smb2_process.c
>+++ b/source3/smbd/smb2_process.c
>@@ -760,6 +760,7 @@ bool init_smb1_request(struct smb_request *req,
> 	req->smb2req = NULL;
> 	req->chain = NULL;
> 	req->posix_pathnames = lp_posix_pathnames();
>+	req->session = (void *)0xDEADBEEF;
> 
> 	/* Ensure we have at least wct words and 2 bytes of bcc. */
> 	if (smb_size + req->wct*2 > req_size) {
>-- 
>2.34.1
>
>
>From 72995d90e542983d28a988f5de38e305c834726c Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 11 Aug 2023 17:18:26 -0700
>Subject: [PATCH 2/3] s3: torture: Add SMB1-NEGOTIATE-TCON that shows the SMB1
> server crashes on the uninitialized req->session.
>
>Found by Robert Morris <rtm@lcs.mit.edu>.
>
>Adds knownfail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Noel Power <nopower@samba.org>
>(cherry picked from commit c32df3bb31ce6275cfb91107e34e2d6b3c2fba1b)
>---
> selftest/knownfail.d/smb1_negprot_tcon |  1 +
> source3/selftest/tests.py              | 11 +++++++
> source3/torture/torture.c              | 40 ++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+)
> create mode 100644 selftest/knownfail.d/smb1_negprot_tcon
>
>diff --git a/selftest/knownfail.d/smb1_negprot_tcon b/selftest/knownfail.d/smb1_negprot_tcon
>new file mode 100644
>index 00000000000..4f620948c31
>--- /dev/null
>+++ b/selftest/knownfail.d/smb1_negprot_tcon
>@@ -0,0 +1 @@
>+^samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-TCON.smbtorture$fileserver_smb1$
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index e4c535c5fe5..979422734da 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -241,6 +241,17 @@ plantestsuite("samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-EXIT",
>                 smbtorture3,
>                 "-mNT1"])
> 
>+plantestsuite("samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-TCON",
>+                "fileserver_smb1",
>+                [os.path.join(samba3srcdir,
>+                              "script/tests/test_smbtorture_s3.sh"),
>+                'SMB1-NEGOTIATE-TCON',
>+                '//$SERVER_IP/tmp',
>+                '$USERNAME',
>+                '$PASSWORD',
>+                smbtorture3,
>+                "-mNT1"])
>+
> #
> # MSDFS attribute tests.
> #
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index 1e496e74456..2a8b8dec22f 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -14999,6 +14999,42 @@ static bool run_smb1_negotiate_exit(int dummy)
> 	return true;
> }
> 
>+static bool run_smb1_negotiate_tcon(int dummy)
>+{
>+	struct cli_state *cli = NULL;
>+	uint16_t cnum = 0;
>+	uint16_t max_xmit = 0;
>+	NTSTATUS status;
>+
>+	printf("Starting send SMB1 negotiate+tcon.\n");
>+	cli = open_nbt_connection();
>+	if (cli == NULL) {
>+		d_fprintf(stderr, "open_nbt_connection failed!\n");
>+		return false;
>+	}
>+	smbXcli_conn_set_sockopt(cli->conn, sockops);
>+
>+	status = smbXcli_negprot(cli->conn, 0, PROTOCOL_NT1, PROTOCOL_NT1);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		d_fprintf(stderr, "smbXcli_negprot failed %s!\n",
>+			nt_errstr(status));
>+		return false;
>+	}
>+        status = cli_raw_tcon(cli,
>+			      share,
>+			      "",
>+			      "?????",
>+			      &max_xmit,
>+			      &cnum);
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>+		d_fprintf(stderr, "cli_raw_tcon failed - got %s "
>+			"(should get NT_STATUS_ACCESS_DENIED)!\n",
>+			nt_errstr(status));
>+		return false;
>+	}
>+	return true;
>+}
>+
> static bool run_ign_bad_negprot(int dummy)
> {
> 	struct tevent_context *ev;
>@@ -15735,6 +15771,10 @@ static struct {
> 		.name  = "SMB1-NEGOTIATE-EXIT",
> 		.fn    = run_smb1_negotiate_exit,
> 	},
>+	{
>+		.name  = "SMB1-NEGOTIATE-TCON",
>+		.fn    = run_smb1_negotiate_tcon,
>+	},
> 	{
> 		.name  = "SMB1-DFS-PATHS",
> 		.fn    = run_smb1_dfs_paths,
>-- 
>2.34.1
>
>
>From 250fe3f1aa4aa555198c3959223a0eed26a38e2b Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 11 Aug 2023 17:28:53 -0700
>Subject: [PATCH 3/3] s3: smbd: Ensure init_smb1_request() zeros out what the
> incoming pointer points to.
>
>Remove the now unneeded req->xxx = NULL assignments (and the
>deliberately bogus req->session = (void *)0xDEADBEEF one
>used to demonstrate the bug).
>
>Remove knownfail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Noel Power <npower@samba.org>
>
>Autobuild-User(master): Noel Power <npower@samba.org>
>Autobuild-Date(master): Tue Aug 15 12:06:36 UTC 2023 on atb-devel-224
>
>(cherry picked from commit 4145bfb1b5a3639caf26a310d612aec29fc00117)
>---
> selftest/knownfail.d/smb1_negprot_tcon | 1 -
> source3/smbd/smb2_process.c            | 8 ++------
> 2 files changed, 2 insertions(+), 7 deletions(-)
> delete mode 100644 selftest/knownfail.d/smb1_negprot_tcon
>
>diff --git a/selftest/knownfail.d/smb1_negprot_tcon b/selftest/knownfail.d/smb1_negprot_tcon
>deleted file mode 100644
>index 4f620948c31..00000000000
>--- a/selftest/knownfail.d/smb1_negprot_tcon
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-TCON.smbtorture$fileserver_smb1$
>diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c
>index 99e30481491..d55f80646ac 100644
>--- a/source3/smbd/smb2_process.c
>+++ b/source3/smbd/smb2_process.c
>@@ -731,6 +731,8 @@ bool init_smb1_request(struct smb_request *req,
> 		return false;
> 	}
> 
>+	*req = (struct smb_request) { .cmd = 0};
>+
> 	req->request_time = timeval_current();
> 	now = timeval_to_nttime(&req->request_time);
> 
>@@ -749,18 +751,13 @@ bool init_smb1_request(struct smb_request *req,
> 	req->encrypted = encrypted;
> 	req->sconn = sconn;
> 	req->xconn = xconn;
>-	req->conn = NULL;
> 	if (xconn != NULL) {
> 		status = smb1srv_tcon_lookup(xconn, req->tid, now, &tcon);
> 		if (NT_STATUS_IS_OK(status)) {
> 			req->conn = tcon->compat;
> 		}
> 	}
>-	req->chain_fsp = NULL;
>-	req->smb2req = NULL;
>-	req->chain = NULL;
> 	req->posix_pathnames = lp_posix_pathnames();
>-	req->session = (void *)0xDEADBEEF;
> 
> 	/* Ensure we have at least wct words and 2 bytes of bcc. */
> 	if (smb_size + req->wct*2 > req_size) {
>@@ -779,7 +776,6 @@ bool init_smb1_request(struct smb_request *req,
> 		return false;
> 	}
> 
>-	req->outbuf = NULL;
> 	return true;
> }
> 
>-- 
>2.34.1
>

Flags:

npower: review+

Actions: View

Attachments on bug 15432: 17997 | 18045 | 18054 | 18055 | 18056