[PATCH] Fix bug #9571 - Unlink after open causes smbd to panic.

[Jeremy Allison]

Please review and push to master if you agree.

Regression smbtorture test is included to ensure
we don't break in future :-).

Jeremy.
-------------- next part --------------
>From a52305c3a083ff395978691af1d66b4cd085c182 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <piastry at etersoft.ru>
Date: Wed, 16 Jan 2013 15:02:26 +0400
Subject: [PATCH 1/2] Fix bug #9571 - Unlink after open causes smbd to panic.

s3:smbd: fix wrong lock order in posix unlink

Signed-off-by: Pavel Shilovsky <piastry at etersoft.ru>

Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 9c77f4d..92d047a 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -7663,8 +7663,8 @@ static NTSTATUS smb_posix_unlink(connection_struct *conn,
 				continue;
 			}
 			/* Fail with sharing violation. */
-			close_file(req, fsp, NORMAL_CLOSE);
 			TALLOC_FREE(lck);
+			close_file(req, fsp, NORMAL_CLOSE);
 			return NT_STATUS_SHARING_VIOLATION;
 		}
 	}
@@ -7678,12 +7678,12 @@ static NTSTATUS smb_posix_unlink(connection_struct *conn,
 						fsp,
 						smb_fname);
 
+	TALLOC_FREE(lck);
+
 	if (!NT_STATUS_IS_OK(status)) {
 		close_file(req, fsp, NORMAL_CLOSE);
-		TALLOC_FREE(lck);
 		return status;
 	}
-	TALLOC_FREE(lck);
 	return close_file(req, fsp, NORMAL_CLOSE);
 }
 
-- 
1.7.10.4


>From 38f45e04f405323dec1c5b53acf2c16bf9fa9e3e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 24 Jan 2013 16:20:14 -0800
Subject: [PATCH 2/2] Regression test for bug #9571 - Unlink after open causes
 smbd to panic

Replicates the protocol activity that triggers the crash.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/torture/torture.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 799c911..b2a0277 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -5368,6 +5368,8 @@ static bool run_simple_posix_open_test(int dummy)
 	bool correct = false;
 	NTSTATUS status;
 	size_t nread;
+	const char *fname_windows = "windows_file";
+	uint16_t fnum2 = (uint16_t)-1;
 
 	printf("Starting simple POSIX open test\n");
 
@@ -5390,6 +5392,8 @@ static bool run_simple_posix_open_test(int dummy)
 	cli_posix_unlink(cli1, hname);
 	cli_setatr(cli1, sname, 0, 0);
 	cli_posix_unlink(cli1, sname);
+	cli_setatr(cli1, fname_windows, 0, 0);
+	cli_posix_unlink(cli1, fname_windows);
 
 	/* Create a directory. */
 	status = cli_posix_mkdir(cli1, dname, 0777);
@@ -5681,6 +5685,40 @@ static bool run_simple_posix_open_test(int dummy)
 		goto out;
 	}
 
+	/*
+	 * Now create a Windows file, and attempt a POSIX unlink.
+	 * This should fail with a sharing violation but due to:
+	 *
+	 * [Bug 9571] Unlink after open causes smbd to panic
+	 *
+	 * ensure we've fixed the lock ordering violation.
+	 */
+
+	status = cli_ntcreate(cli1, fname_windows, 0,
+			FILE_READ_DATA|FILE_WRITE_DATA, 0,
+			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
+			FILE_CREATE,
+			0x0, 0x0, &fnum2);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Windows create of %s failed (%s)\n", fname_windows,
+			nt_errstr(status));
+		goto out;
+	}
+
+	/* Now try posix_unlink. */
+	status = cli_posix_unlink(cli1, fname_windows);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION)) {
+		printf("POSIX unlink of %s should fail "
+			"with NT_STATUS_SHARING_VIOLATION "
+			"got %s instead !\n",
+			fname_windows,
+			nt_errstr(status));
+		goto out;
+	}
+
+	cli_close(cli1, fnum2);
+
 	printf("Simple POSIX open test passed\n");
 	correct = true;
 
@@ -5691,6 +5729,11 @@ static bool run_simple_posix_open_test(int dummy)
 		fnum1 = (uint16_t)-1;
 	}
 
+	if (fnum2 != (uint16_t)-1) {
+		cli_close(cli1, fnum2);
+		fnum2 = (uint16_t)-1;
+	}
+
 	cli_setatr(cli1, sname, 0, 0);
 	cli_posix_unlink(cli1, sname);
 	cli_setatr(cli1, hname, 0, 0);
@@ -5699,6 +5742,8 @@ static bool run_simple_posix_open_test(int dummy)
 	cli_posix_unlink(cli1, fname);
 	cli_setatr(cli1, dname, 0, 0);
 	cli_posix_rmdir(cli1, dname);
+	cli_setatr(cli1, fname_windows, 0, 0);
+	cli_posix_unlink(cli1, fname_windows);
 
 	if (!torture_close_connection(cli1)) {
 		correct = false;
-- 
1.7.10.4