Attachment 15369 Details for Bug 14060 – Patch for 4.9

[patch] Patch for 4.9

14060-4.9.patch (text/plain), 14.39 KB, created by Volker Lendecke on 2019-08-02 08:09:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Volker Lendecke

Created: 2019-08-02 08:09:23 UTC

Size: 14.39 KB

patch

obsolete

>From 8e3b186d277a21ca520de8e2d96398e7c1987b3f Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Mon, 29 Jul 2019 17:14:00 +0200
>Subject: [PATCH 1/3] torture3: Add oplock5 kernel-oplock test
>
>Show that the current smb1 server does not properly retry a nonblocking
>open of a kernel-oplocked file
>
>Based on 57695ad44bf
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14060
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> selftest/knownfail        |   1 +
> source3/selftest/tests.py |  13 +++
> source3/torture/torture.c | 271 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 285 insertions(+)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 4139bfd02bc..6e79daaccb5 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -12,6 +12,7 @@
> ^samba3.smb2.session enc.reconnect # expected to give CONNECTION_DISCONNECTED, we need to fix the test
> ^samba3.raw.session enc # expected to give ACCESS_DENIED as SMB1 encryption isn't used
> ^samba3.smbtorture_s3.crypt_server # expected to give ACCESS_DENIED as SMB1 encryption isn't used
>+^samba3.smbtorture_s3.*.OPLOCK5.*$fileserver$
> ^samba3.nbt.dgram.*netlogon2$nt4_dc$
> ^samba3.*rap.sam.*.useradd # Not provided by Samba 3
> ^samba3.*rap.sam.*.userdelete # Not provided by Samba 3
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 64546900d83..ec3aa2bebbb 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -95,6 +95,19 @@ for t in tests:
> t = "TLDAP"
> plantestsuite("samba3.smbtorture_s3.plain.%s(ad_dc)" % t, "ad_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER/tmp', '$DC_USERNAME', '$DC_PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
> 
>+t = "OPLOCK5"
>+plantestsuite("samba3.smbtorture_s3.plain.%s" % t,
>+              "fileserver",
>+              [os.path.join(samba3srcdir,
>+                            "script/tests/test_smbtorture_s3.sh"),
>+               t,
>+               '//$SERVER/tmp',
>+               '$USERNAME',
>+               '$PASSWORD',
>+               smbtorture3,
>+               "",
>+               "-l $LOCAL_PATH",
>+               "-mNT1"])
> #
> # RENAME-ACCESS needs to run against a special share - acl_xattr_ign_sysacl_windows
> #
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index 2b020ca0f9c..1ba343b3188 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -44,6 +44,7 @@
> #include "lib/util/sys_rw_data.h"
> #include "lib/util/base64.h"
> #include "lib/util/time.h"
>+#include "lib/util/sys_rw.h"
> 
> extern char *optarg;
> extern int optind;
>@@ -3992,6 +3993,273 @@ static void oplock4_got_open(struct tevent_req *req)
> 	}
> }
> 
>+#ifdef HAVE_KERNEL_OPLOCKS_LINUX
>+
>+struct oplock5_state {
>+	int pipe_down_fd;
>+};
>+
>+/*
>+ * Async open the file that has a kernel oplock, do an echo to get
>+ * that 100% across, close the file to signal to the child fd that the
>+ * oplock can be dropped, wait for the open reply.
>+ */
>+
>+static void oplock5_opened(struct tevent_req *subreq);
>+static void oplock5_pong(struct tevent_req *subreq);
>+static void oplock5_timedout(struct tevent_req *subreq);
>+
>+static struct tevent_req *oplock5_send(
>+	TALLOC_CTX *mem_ctx,
>+	struct tevent_context *ev,
>+	struct cli_state *cli,
>+	const char *fname,
>+	int pipe_down_fd)
>+{
>+	struct tevent_req *req = NULL, *subreq = NULL;
>+	struct oplock5_state *state = NULL;
>+	static uint8_t data = 0;
>+
>+	req = tevent_req_create(mem_ctx, &state, struct oplock5_state);
>+	if (req == NULL) {
>+		return NULL;
>+	}
>+	state->pipe_down_fd = pipe_down_fd;
>+
>+	subreq = cli_ntcreate_send(
>+		state,
>+		ev,
>+		cli,
>+		fname,
>+		0,			/* CreatFlags */
>+		SEC_FILE_READ_DATA,    /* DesiredAccess */
>+		FILE_ATTRIBUTE_NORMAL,  /* FileAttributes */
>+		FILE_SHARE_WRITE|FILE_SHARE_READ, /* ShareAccess */
>+		FILE_OPEN,		 /* CreateDisposition */
>+		FILE_NON_DIRECTORY_FILE, /* CreateOptions */
>+		0);			 /* SecurityFlags */
>+	if (tevent_req_nomem(subreq, req)) {
>+		return tevent_req_post(req, ev);
>+	}
>+	tevent_req_set_callback(subreq, oplock5_opened, req);
>+
>+	subreq = cli_echo_send(
>+		state,
>+		ev,
>+		cli,
>+		1,
>+		(DATA_BLOB) { .data = &data, .length = sizeof(data) });
>+	if (tevent_req_nomem(subreq, req)) {
>+		return tevent_req_post(req, ev);
>+	}
>+	tevent_req_set_callback(subreq, oplock5_pong, req);
>+
>+	subreq = tevent_wakeup_send(state, ev, timeval_current_ofs(20, 0));
>+	if (tevent_req_nomem(subreq, req)) {
>+		return tevent_req_post(req, ev);
>+	}
>+	tevent_req_set_callback(subreq, oplock5_timedout, req);
>+
>+	return req;
>+}
>+
>+static void oplock5_opened(struct tevent_req *subreq)
>+{
>+	struct tevent_req *req = tevent_req_callback_data(
>+		subreq, struct tevent_req);
>+	NTSTATUS status;
>+	uint16_t fnum;
>+
>+	status = cli_ntcreate_recv(subreq, &fnum, NULL);
>+	TALLOC_FREE(subreq);
>+	if (tevent_req_nterror(req, status)) {
>+		return;
>+	}
>+	tevent_req_done(req);
>+}
>+
>+static void oplock5_pong(struct tevent_req *subreq)
>+{
>+	struct tevent_req *req = tevent_req_callback_data(
>+		subreq, struct tevent_req);
>+	struct oplock5_state *state = tevent_req_data(
>+		req, struct oplock5_state);
>+	NTSTATUS status;
>+
>+	status = cli_echo_recv(subreq);
>+	TALLOC_FREE(subreq);
>+	if (tevent_req_nterror(req, status)) {
>+		return;
>+	}
>+
>+	close(state->pipe_down_fd);
>+}
>+
>+static void oplock5_timedout(struct tevent_req *subreq)
>+{
>+	struct tevent_req *req = tevent_req_callback_data(
>+		subreq, struct tevent_req);
>+	bool ok;
>+
>+	ok = tevent_wakeup_recv(subreq);
>+	TALLOC_FREE(subreq);
>+	if (!ok) {
>+		tevent_req_oom(req);
>+		return;
>+	}
>+	tevent_req_nterror(req, NT_STATUS_TIMEOUT);
>+}
>+
>+static NTSTATUS oplock5_recv(struct tevent_req *req)
>+{
>+	return tevent_req_simple_recv_ntstatus(req);
>+}
>+
>+static bool run_oplock5(int dummy)
>+{
>+	struct tevent_context *ev = NULL;
>+	struct tevent_req *req = NULL;
>+	struct cli_state *cli = NULL;
>+	const char *fname = "oplock5.txt";
>+	int pipe_down[2], pipe_up[2];
>+	pid_t child_pid;
>+	uint8_t c = '\0';
>+	NTSTATUS status;
>+	int ret;
>+	bool ok;
>+
>+	printf("starting oplock5\n");
>+
>+	if (local_path == NULL) {
>+		d_fprintf(stderr, "oplock5 must be given a local path via "
>+			  "-l <localpath>\n");
>+		return false;
>+	}
>+
>+	ret = pipe(pipe_down);
>+	if (ret == -1) {
>+		d_fprintf(stderr, "pipe() failed: %s\n", strerror(errno));
>+		return false;
>+	}
>+	ret = pipe(pipe_up);
>+	if (ret == -1) {
>+		d_fprintf(stderr, "pipe() failed: %s\n", strerror(errno));
>+		return false;
>+	}
>+
>+	child_pid = fork();
>+	if (child_pid == -1) {
>+		d_fprintf(stderr, "fork() failed: %s\n", strerror(errno));
>+		return false;
>+	}
>+
>+	if (child_pid == 0) {
>+		char *local_file = NULL;
>+		int fd;
>+
>+		close(pipe_down[1]);
>+		close(pipe_up[0]);
>+
>+		local_file = talloc_asprintf(
>+			talloc_tos(), "%s/%s", local_path, fname);
>+		if (local_file == 0) {
>+			c = 1;
>+			goto do_write;
>+		}
>+		fd = open(local_file, O_RDWR|O_CREAT, 0644);
>+		if (fd == -1) {
>+			d_fprintf(stderr,
>+				  "open(%s) in child failed: %s\n",
>+				  local_file,
>+				  strerror(errno));
>+			c = 2;
>+			goto do_write;
>+		}
>+
>+		signal(SIGIO, SIG_IGN);
>+
>+		ret = fcntl(fd, F_SETLEASE, F_WRLCK);
>+		if (ret == -1) {
>+			d_fprintf(stderr,
>+				  "SETLEASE in child failed: %s\n",
>+				  strerror(errno));
>+			c = 3;
>+			goto do_write;
>+		}
>+
>+	do_write:
>+		ret = sys_write(pipe_up[1], &c, sizeof(c));
>+		if (ret == -1) {
>+			d_fprintf(stderr,
>+				  "sys_write failed: %s\n",
>+				  strerror(errno));
>+			exit(4);
>+		}
>+		ret = sys_read(pipe_down[0], &c, sizeof(c));
>+		if (ret == -1) {
>+			d_fprintf(stderr,
>+				  "sys_read failed: %s\n",
>+				  strerror(errno));
>+			exit(5);
>+		}
>+		exit(0);
>+	}
>+
>+	close(pipe_up[1]);
>+	close(pipe_down[0]);
>+
>+	ret = sys_read(pipe_up[0], &c, sizeof(c));
>+	if (ret != 1) {
>+		d_fprintf(stderr,
>+			  "sys_read failed: %s\n",
>+			  strerror(errno));
>+		return false;
>+	}
>+	if (c != 0) {
>+		d_fprintf(stderr, "got error code %"PRIu8"\n", c);
>+		return false;
>+	}
>+
>+	ok = torture_open_connection(&cli, 0);
>+	if (!ok) {
>+		d_fprintf(stderr, "torture_open_connection failed\n");
>+		return false;
>+	}
>+
>+	ev = samba_tevent_context_init(talloc_tos());
>+	if (ev == NULL) {
>+		d_fprintf(stderr, "samba_tevent_context_init failed\n");
>+		return false;
>+	}
>+
>+	req = oplock5_send(ev, ev, cli, fname, pipe_down[1]);
>+	if (req == NULL) {
>+		d_fprintf(stderr, "oplock5_send failed\n");
>+		return false;
>+	}
>+
>+	ok = tevent_req_poll_ntstatus(req, ev, &status);
>+	if (!ok) {
>+		d_fprintf(stderr,
>+			  "tevent_req_poll_ntstatus failed: %s\n",
>+			  nt_errstr(status));
>+		return false;
>+	}
>+
>+	status = oplock5_recv(req);
>+	TALLOC_FREE(req);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		d_fprintf(stderr,
>+			  "oplock5 failed: %s\n",
>+			  nt_errstr(status));
>+		return false;
>+	}
>+
>+	return true;
>+}
>+
>+#endif /* HAVE_KERNEL_OPLOCKS_LINUX */
>+
> /*
>   Test delete on close semantics.
>  */
>@@ -11860,6 +12128,9 @@ static struct {
> 	{"OPLOCK1",  run_oplock1, 0},
> 	{"OPLOCK2",  run_oplock2, 0},
> 	{"OPLOCK4",  run_oplock4, 0},
>+#ifdef HAVE_KERNEL_OPLOCKS_LINUX
>+	{"OPLOCK5",  run_oplock5, 0},
>+#endif
> 	{"DIR",  run_dirtest, 0},
> 	{"DIR1",  run_dirtest1, 0},
> 	{"DIR-CREATETIME",  run_dir_createtime, 0},
>-- 
>2.11.0
>
>
>From 74054ce8cb5b267fb9eb852f3a77ed0038905b1c Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Mon, 29 Jul 2019 20:45:51 +0200
>Subject: [PATCH 2/3] smbd: Always open files with O_NONBLOCK
>
>It's perfectly possible that someone else takes a kernel oplock and
>makes us block, independent of our own kernel oplock setting.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14060
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit ef582ffcf3a220b73f678d9bce0fd37800f76c54)
>---
> source3/smbd/open.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index af6a36f0bd0..4481dbfd739 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -3231,21 +3231,19 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 		flags2 &= ~(O_CREAT|O_TRUNC);
> 	}
> 
>-	if (lp_kernel_oplocks(SNUM(conn))) {
>-		/*
>-		 * With kernel oplocks the open breaking an oplock
>-		 * blocks until the oplock holder has given up the
>-		 * oplock or closed the file. We prevent this by always
>-		 * trying to open the file with O_NONBLOCK (see "man
>-		 * fcntl" on Linux).
>-		 *
>-		 * If a process that doesn't use the smbd open files
>-		 * database or communication methods holds a kernel
>-		 * oplock we must periodically poll for available open
>-		 * using O_NONBLOCK.
>-		 */
>-		flags2 |= O_NONBLOCK;
>-	}
>+	/*
>+	 * With kernel oplocks the open breaking an oplock
>+	 * blocks until the oplock holder has given up the
>+	 * oplock or closed the file. We prevent this by always
>+	 * trying to open the file with O_NONBLOCK (see "man
>+	 * fcntl" on Linux).
>+	 *
>+	 * If a process that doesn't use the smbd open files
>+	 * database or communication methods holds a kernel
>+	 * oplock we must periodically poll for available open
>+	 * using O_NONBLOCK.
>+	 */
>+	flags2 |= O_NONBLOCK;
> 
> 	/*
> 	 * Ensure we can't write on a read-only share or file.
>-- 
>2.11.0
>
>
>From 8e26759fad4b33d7ee882de1e4b369ad6d366aa2 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Tue, 30 Jul 2019 12:24:53 +0200
>Subject: [PATCH 3/3] smbd: Fix retry for kernel-oplocked files
>
>This now removed comment describes the bug correctly:
>/*
> * As this timer event is owned by req, it will
> * disappear if req it talloc_freed.
> */
>
>In smb1, "req" disappears once the reply_whatever routine is done. Thus
>the timer goes away and we never look at "req" again.
>
>This change moves the valid data (xconn and mid) to
>deferred_open_record, and changes the talloc hierarchy such that the
>timer is now a child of open_rec, which is a child of the deferred
>message.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14060
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Wed Jul 31 00:12:34 UTC 2019 on sn-devel-184
>
>(cherry picked from commit 2b590e16bcb4f4e1f1f0bf049a863db38e634beb)
>---
> selftest/knownfail  |  1 -
> source3/smbd/open.c | 19 +++++++++++--------
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 6e79daaccb5..4139bfd02bc 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -12,7 +12,6 @@
> ^samba3.smb2.session enc.reconnect # expected to give CONNECTION_DISCONNECTED, we need to fix the test
> ^samba3.raw.session enc # expected to give ACCESS_DENIED as SMB1 encryption isn't used
> ^samba3.smbtorture_s3.crypt_server # expected to give ACCESS_DENIED as SMB1 encryption isn't used
>-^samba3.smbtorture_s3.*.OPLOCK5.*$fileserver$
> ^samba3.nbt.dgram.*netlogon2$nt4_dc$
> ^samba3.*rap.sam.*.useradd # Not provided by Samba 3
> ^samba3.*rap.sam.*.userdelete # Not provided by Samba 3
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 4481dbfd739..6b5f88976b8 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -43,6 +43,9 @@
> extern const struct generic_mapping file_generic_mapping;
> 
> struct deferred_open_record {
>+	struct smbXsrv_connection *xconn;
>+	uint64_t mid;
>+
>         bool delayed_for_oplocks;
> 	bool async_open;
>         struct file_id id;
>@@ -2458,10 +2461,12 @@ static void kernel_oplock_poll_open_timer(struct tevent_context *ev,
> 				      struct timeval current_time,
> 				      void *private_data)
> {
>+	struct deferred_open_record *open_rec = talloc_get_type_abort(
>+		private_data, struct deferred_open_record);
> 	bool ok;
>-	struct smb_request *req = (struct smb_request *)private_data;
> 
>-	ok = schedule_deferred_open_message_smb(req->xconn, req->mid);
>+	ok = schedule_deferred_open_message_smb(
>+		open_rec->xconn, open_rec->mid);
> 	if (!ok) {
> 		exit_server("schedule_deferred_open_message_smb failed");
> 	}
>@@ -2489,6 +2494,8 @@ static void setup_kernel_oplock_poll_open(struct timeval request_time,
> 	if (open_rec == NULL) {
> 		exit_server("talloc failed");
> 	}
>+	open_rec->xconn = req->xconn;
>+	open_rec->mid = req->mid;
> 
> 	ok = push_deferred_open_message_smb(req,
> 					    request_time,
>@@ -2499,15 +2506,11 @@ static void setup_kernel_oplock_poll_open(struct timeval request_time,
> 		exit_server("push_deferred_open_message_smb failed");
> 	}
> 
>-	/*
>-	 * As this timer event is owned by req, it will
>-	 * disappear if req it talloc_freed.
>-	 */
> 	open_rec->te = tevent_add_timer(req->ev_ctx,
>-					req,
>+					open_rec,
> 					timeval_current_ofs(1, 0),
> 					kernel_oplock_poll_open_timer,
>-					req);
>+					open_rec);
> 	if (open_rec->te == NULL) {
> 		exit_server("tevent_add_timer failed");
> 	}
>-- 
>2.11.0
>

Flags:

jra: review+

Actions: View

Attachments on bug 14060: 15367 | 15368 | 15369 | 15436