From 38ea109d3b41360fbc27be829b5fdd4670a718fa Mon Sep 17 00:00:00 2001 From: Volker Lendecke 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 Reviewed-by: Jeremy Allison --- selftest/knownfail | 1 + source3/selftest/tests.py | 13 +++ source3/torture/torture.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 286 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index 1817db384e0..7c43ff6901e 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 91b870430b9..a41516d8911 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -109,6 +109,19 @@ for t in tests: t = "TLDAP" plantestsuite("samba3.smbtorture_s3.plain.%s" % 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 3507e4f198b..aaff1593000 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -46,6 +46,7 @@ #include "lib/util/time.h" #include "lib/crypto/md5.h" #include "lib/gencache.h" +#include "lib/util/sys_rw.h" extern char *optarg; extern int optind; @@ -3994,6 +3995,274 @@ 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, /* Impersonation */ + 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 \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. */ @@ -12020,6 +12289,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 dd722a820318ddcb24580eaee895b462c74e6e21 Mon Sep 17 00:00:00 2001 From: Volker Lendecke 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 Reviewed-by: Jeremy Allison (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 de557f53a20..f425be37913 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 d4dc6ea9a6a9cd7b94c77a616ee1f086082fa4bf Mon Sep 17 00:00:00 2001 From: Volker Lendecke 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 Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison 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 7c43ff6901e..1817db384e0 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 f425be37913..e36ca88325b 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->sconn->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