From fe247b54c4e0f395de4cdacdced61560f3aeae54 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 4 Sep 2019 12:50:06 +0200 Subject: [PATCH 01/39] s3:blocking: call change_to_user_by_fsp() when dbwrap_watched_watch* finishes This is not strictly required as fd-based calls are used, but it's more consistent to call SMB_VFS_BRL_LOCK_WINDOWS() in the same environment on retry. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 28ac2cbaf92a8619f0380f024c5a220d9fdc4622) --- source3/smbd/blocking.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index ffc3142b74c6..ca8a625ba9dd 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -334,6 +334,15 @@ static void smbd_smb1_do_locks_retry(struct tevent_req *subreq) NTSTATUS status; bool ok; + /* + * Make sure we run as the user again + */ + ok = change_to_user_by_fsp(state->fsp); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return; + } + status = dbwrap_watched_watch_recv(subreq, NULL, NULL); TALLOC_FREE(subreq); -- 2.17.1 From 64d576801039f6a1322b8d8c0839daa0d6832f23 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 4 Sep 2019 12:47:07 +0200 Subject: [PATCH 02/39] s3:smb2_lock: call change_to_user_by_fsp() when dbwrap_watched_watch* finishes This is not strictly required as fd-based calls are used, but it's more consistent to call SMB_VFS_BRL_LOCK_WINDOWS() in the same environment on retry. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 8b565de1acb0fda121cb0bd4cff42d66ee027529) --- source3/smbd/smb2_lock.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index 36ec36301b11..e9c8d7f890e6 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -421,6 +421,16 @@ static void smbd_smb2_lock_retry(struct tevent_req *subreq) struct server_id blocking_pid = { 0 }; uint64_t blocking_smblctx; NTSTATUS status; + bool ok; + + /* + * Make sure we run as the user again + */ + ok = change_to_user_by_fsp(state->fsp); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return; + } status = dbwrap_watched_watch_recv(subreq, NULL, NULL); TALLOC_FREE(subreq); -- 2.17.1 From cf39809ed822a4afa1eb764bc6aa26fbe2765a66 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 5 Sep 2019 08:43:32 +0200 Subject: [PATCH 03/39] s3:locking: add/split out byte_range_{valid,overlap}() helper functions They implement the logic from [MS-FSA]. The following commits will use these functions in other locations. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 0e5613e39d6c6bb892fed939c63b4f14b878803b) --- source3/locking/brlock.c | 109 +++++++++++++++++++++++++++++++++------ source3/locking/proto.h | 6 +++ 2 files changed, 99 insertions(+), 16 deletions(-) diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c index cdfd09ceff11..628c2574357d 100644 --- a/source3/locking/brlock.c +++ b/source3/locking/brlock.c @@ -96,6 +96,92 @@ static bool brl_same_context(const struct lock_context *ctx1, (ctx1->tid == ctx2->tid)); } +bool byte_range_valid(uint64_t ofs, uint64_t len) +{ + uint64_t max_len = UINT64_MAX - ofs; + uint64_t effective_len; + + /* + * [MS-FSA] specifies this: + * + * If (((FileOffset + Length - 1) < FileOffset) && Length != 0) { + * return STATUS_INVALID_LOCK_RANGE + * } + * + * We avoid integer wrapping and calculate + * max and effective len instead. + */ + + if (len == 0) { + return true; + } + + effective_len = len - 1; + if (effective_len <= max_len) { + return true; + } + + return false; +} + +bool byte_range_overlap(uint64_t ofs1, + uint64_t len1, + uint64_t ofs2, + uint64_t len2) +{ + uint64_t last1; + uint64_t last2; + bool valid; + + /* + * This is based on [MS-FSA] 2.1.4.10 + * Algorithm for Determining If a Range Access + * Conflicts with Byte-Range Locks + */ + + /* + * The {0, 0} range doesn't conflict with any byte-range lock + */ + if (ofs1 == 0 && len1 == 0) { + return false; + } + if (ofs2 == 0 && len2 == 0) { + return false; + } + + /* + * The caller should have checked that the ranges are + * valid. But currently we gracefully handle + * the overflow of a read/write check. + */ + valid = byte_range_valid(ofs1, len1); + if (valid) { + last1 = ofs1 + len1 - 1; + } else { + last1 = UINT64_MAX; + } + valid = byte_range_valid(ofs2, len2); + if (valid) { + last2 = ofs2 + len2 - 1; + } else { + last2 = UINT64_MAX; + } + + /* + * If one range starts after the last + * byte of the other range there's + * no conflict. + */ + if (ofs1 > last2) { + return false; + } + if (ofs2 > last1) { + return false; + } + + return true; +} + /**************************************************************************** See if lck1 and lck2 overlap. ****************************************************************************/ @@ -103,20 +189,10 @@ static bool brl_same_context(const struct lock_context *ctx1, static bool brl_overlap(const struct lock_struct *lck1, const struct lock_struct *lck2) { - /* XXX Remove for Win7 compatibility. */ - /* this extra check is not redundant - it copes with locks - that go beyond the end of 64 bit file space */ - if (lck1->size != 0 && - lck1->start == lck2->start && - lck1->size == lck2->size) { - return True; - } - - if (lck1->start >= (lck2->start+lck2->size) || - lck2->start >= (lck1->start+lck1->size)) { - return False; - } - return True; + return byte_range_overlap(lck1->start, + lck1->size, + lck2->start, + lck2->size); } /**************************************************************************** @@ -336,11 +412,12 @@ NTSTATUS brl_lock_windows_default(struct byte_range_lock *br_lck, files_struct *fsp = br_lck->fsp; struct lock_struct *locks = br_lck->lock_data; NTSTATUS status; + bool valid; SMB_ASSERT(plock->lock_type != UNLOCK_LOCK); - if ((plock->start + plock->size - 1 < plock->start) && - plock->size != 0) { + valid = byte_range_valid(plock->start, plock->size); + if (!valid) { return NT_STATUS_INVALID_LOCK_RANGE; } diff --git a/source3/locking/proto.h b/source3/locking/proto.h index 3a086fa0516d..2487fa5d14d7 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -31,6 +31,12 @@ void brl_shutdown(void); unsigned int brl_num_locks(const struct byte_range_lock *brl); struct files_struct *brl_fsp(struct byte_range_lock *brl); +bool byte_range_valid(uint64_t ofs, uint64_t len); +bool byte_range_overlap(uint64_t ofs1, + uint64_t len1, + uint64_t ofs2, + uint64_t len2); + NTSTATUS brl_lock_windows_default(struct byte_range_lock *br_lck, struct lock_struct *plock); -- 2.17.1 From be4c57615c77df85374d4491718eaf9275600d8c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 9 Aug 2019 00:47:39 +0200 Subject: [PATCH 04/39] s3:locking: add share_mode_wakeup_waiters() helper function BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit bd8884e5722cbbb7783fb4ae53e4f35b31031b01) --- source3/locking/proto.h | 1 + source3/locking/share_mode_lock.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/source3/locking/proto.h b/source3/locking/proto.h index 2487fa5d14d7..7cb8bf3e3c99 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -138,6 +138,7 @@ NTSTATUS share_mode_do_locked( bool *modified_dependent, void *private_data), void *private_data); +NTSTATUS share_mode_wakeup_waiters(struct file_id id); struct share_mode_lock *fetch_share_mode_unlocked(TALLOC_CTX *mem_ctx, struct file_id id); diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c index 430d14fab4a1..372e188c1c0c 100644 --- a/source3/locking/share_mode_lock.c +++ b/source3/locking/share_mode_lock.c @@ -771,6 +771,18 @@ NTSTATUS share_mode_do_locked( return NT_STATUS_OK; } +static void share_mode_wakeup_waiters_fn(struct db_record *rec, + bool *modified_dependent, + void *private_data) +{ + *modified_dependent = true; +} + +NTSTATUS share_mode_wakeup_waiters(struct file_id id) +{ + return share_mode_do_locked(id, share_mode_wakeup_waiters_fn, NULL); +} + struct fetch_share_mode_unlocked_state { TALLOC_CTX *mem_ctx; struct share_mode_lock *lck; -- 2.17.1 From 6a1339fb5fe8238715a3391de44e7f0a3271bf41 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 19:19:07 +0200 Subject: [PATCH 05/39] s3:blocking: remove unused timeval_brl_min() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 3b788d97f9995e24e4005567f90a925957fb1e00) --- source3/smbd/blocking.c | 16 ---------------- source3/smbd/proto.h | 2 -- 2 files changed, 18 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index ca8a625ba9dd..cdc4613270e2 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -28,22 +28,6 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_LOCKING -/**************************************************************************** - We need a version of timeval_min that treats zero timval as infinite. -****************************************************************************/ - -struct timeval timeval_brl_min(const struct timeval *tv1, - const struct timeval *tv2) -{ - if (timeval_is_zero(tv1)) { - return *tv2; - } - if (timeval_is_zero(tv2)) { - return *tv1; - } - return timeval_min(tv1, tv2); -} - NTSTATUS smbd_do_locks_try( struct files_struct *fsp, enum brl_flavour lock_flav, diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 37eeb9f31ca4..b59f1f4123da 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -95,8 +95,6 @@ bool aio_add_req_to_fsp(files_struct *fsp, struct tevent_req *req); /* The following definitions come from smbd/blocking.c */ -struct timeval timeval_brl_min(const struct timeval *tv1, - const struct timeval *tv2); NTSTATUS smbd_do_locks_try( struct files_struct *fsp, enum brl_flavour lock_flav, -- 2.17.1 From 5ab2d9a9dea13bc72cb16cb737e9898193fb700d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 16:10:58 +0200 Subject: [PATCH 06/39] s3:torture: fix the timeout alarm handling on LOCK9 smbXcli_conn_disconnect(alarm_cli->conn, NT_STATUS_OK) means existing requests are not finished with an error, but instead just keep dangling arround. Pass NT_STATUS_LOCAL_DISCONNECT in order to fail the cli_lock32() call after getting SIGALARM. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit e18c8ced8e7a872deb118191595425ef6b826bfa) --- source3/torture/torture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index f26c634b7a76..5a75796928a1 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -2543,7 +2543,7 @@ static void alarm_handler(int dummy) static void alarm_handler_parent(int dummy) { - smbXcli_conn_disconnect(alarm_cli->conn, NT_STATUS_OK); + smbXcli_conn_disconnect(alarm_cli->conn, NT_STATUS_LOCAL_DISCONNECT); } static void do_local_lock(int read_fd, int write_fd) -- 2.17.1 From 0c5f1b6a9a6552e65c359cc7e8e5c28f95d14138 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 14:31:54 +0200 Subject: [PATCH 07/39] s3:torture: convert LOCK9 into LOCK9A and LOCK9B LOCK9A is the original test (with a timeout of -1) and LOCK9B is the same but with timeout of 10 seconds. LOCK9B is needed to demonstrate a server bug in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit ac28eec3e4af710feab3be3d4b25bfbe38294431) --- selftest/skip | 2 +- source3/selftest/tests.py | 3 ++- source3/torture/torture.c | 38 ++++++++++++++++++++++++++------------ 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/selftest/skip b/selftest/skip index 1e7448acb9f7..c471072e88f6 100644 --- a/selftest/skip +++ b/selftest/skip @@ -34,7 +34,7 @@ ^samba3.smbtorture_s3.*.pipe_number ^samba3.smbtorture_s3.LOCAL-DBTRANS #hangs for some reason ^samba3.smbtorture_s3.*.DIR1 #loops on 64 bit linux with ext4 -^samba3.smbtorture_s3.plain.LOCK9\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server +^samba3.smbtorture_s3.plain.LOCK9.*\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.OPLOCK2\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.STREAMERROR\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server ^samba3.smbtorture_s3.plain.DIR1\(ad_dc_ntvfs\) # Fails against the s4 ntvfs server diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 9569aa9ae008..a3daeacae6bf 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -78,7 +78,8 @@ plantestsuite("samba3.local_s3", "nt4_dc:local", [os.path.join(samba3srcdir, "sc plantestsuite("samba3.blackbox.registry.upgrade", "nt4_dc:local", [os.path.join(samba3srcdir, "script/tests/test_registry_upgrade.sh"), net, dbwrap_tool]) -tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", "LOCK5", "LOCK6", "LOCK7", "LOCK9", +tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", "LOCK5", "LOCK6", "LOCK7", + "LOCK9A", "LOCK9B", "LOCK10", "LOCK11", "LOCK12", diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 5a75796928a1..66dc0cf4d1cf 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -2546,7 +2546,7 @@ static void alarm_handler_parent(int dummy) smbXcli_conn_disconnect(alarm_cli->conn, NT_STATUS_LOCAL_DISCONNECT); } -static void do_local_lock(int read_fd, int write_fd) +static void do_local_lock(const char *fname, int read_fd, int write_fd) { int fd; char c = '\0'; @@ -2555,7 +2555,7 @@ static void do_local_lock(int read_fd, int write_fd) int ret; local_pathname = talloc_asprintf(talloc_tos(), - "%s/lockt9.lck", local_path); + "%s/%s", local_path, fname); if (!local_pathname) { printf("child: alloc fail\n"); exit(1); @@ -2614,10 +2614,10 @@ static void do_local_lock(int read_fd, int write_fd) exit(0); } -static bool run_locktest9(int dummy) +static bool _run_locktest9X(const char *fname, int timeout) { struct cli_state *cli1; - const char *fname = "\\lockt9.lck"; + char *fpath = talloc_asprintf(talloc_tos(), "\\%s", fname); uint16_t fnum; bool correct = False; int pipe_in[2], pipe_out[2]; @@ -2628,10 +2628,10 @@ static bool run_locktest9(int dummy) double seconds; NTSTATUS status; - printf("starting locktest9\n"); + printf("starting locktest9X: %s\n", fname); if (local_path == NULL) { - d_fprintf(stderr, "locktest9 must be given a local path via -l \n"); + d_fprintf(stderr, "locktest9X must be given a local path via -l \n"); return false; } @@ -2646,7 +2646,7 @@ static bool run_locktest9(int dummy) if (child_pid == 0) { /* Child. */ - do_local_lock(pipe_out[0], pipe_in[1]); + do_local_lock(fname, pipe_out[0], pipe_in[1]); exit(0); } @@ -2669,7 +2669,7 @@ static bool run_locktest9(int dummy) smbXcli_conn_set_sockopt(cli1->conn, sockops); - status = cli_openx(cli1, fname, O_RDWR, DENY_NONE, + status = cli_openx(cli1, fpath, O_RDWR, DENY_NONE, &fnum); if (!NT_STATUS_IS_OK(status)) { d_fprintf(stderr, "cli_openx returned %s\n", nt_errstr(status)); @@ -2700,7 +2700,7 @@ static bool run_locktest9(int dummy) start = timeval_current(); - status = cli_lock32(cli1, fnum, 0, 4, -1, WRITE_LOCK); + status = cli_lock32(cli1, fnum, 0, 4, timeout, WRITE_LOCK); if (!NT_STATUS_IS_OK(status)) { d_fprintf(stderr, "Unable to apply write lock on range 0:4, error was " "%s\n", nt_errstr(status)); @@ -2727,10 +2727,20 @@ fail: fail_nofd: - printf("finished locktest9\n"); + printf("finished locktest9X: %s\n", fname); return correct; } +static bool run_locktest9a(int dummy) +{ + return _run_locktest9X("lock9a.dat", -1); +} + +static bool run_locktest9b(int dummy) +{ + return _run_locktest9X("lock9b.dat", 10000); +} + struct locktest10_state { bool ok; bool done; @@ -13651,8 +13661,12 @@ static struct { .fn = run_locktest8, }, { - .name = "LOCK9", - .fn = run_locktest9, + .name = "LOCK9A", + .fn = run_locktest9a, + }, + { + .name = "LOCK9B", + .fn = run_locktest9b, }, { .name = "LOCK10", -- 2.17.1 From 29affeda9cb87ef3a9d9e29eb71a11b2606acb37 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 11:09:47 +0200 Subject: [PATCH 08/39] s3:blocking: demonstrate the posix lock retry fails This is just a temporary commit that shows the bug and its fix. It will be reverted once the problem is fixed. The posix lock retry fails if the client specified timeout is smaller than the hardcoded 1 second retry. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 2ec9e93a7aac2706b4a5931495d56a7b64f8d894) --- selftest/knownfail.d/lock9 | 1 + source3/smbd/blocking.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 selftest/knownfail.d/lock9 diff --git a/selftest/knownfail.d/lock9 b/selftest/knownfail.d/lock9 new file mode 100644 index 000000000000..044622586eb4 --- /dev/null +++ b/selftest/knownfail.d/lock9 @@ -0,0 +1 @@ +^samba3.smbtorture_s3.*.LOCK9B diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index cdc4613270e2..91438fe44860 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -236,7 +236,7 @@ struct tevent_req *smbd_smb1_do_locks_send( DBG_DEBUG("Blocked on a posix lock. Retry in one second\n"); - tmp = timeval_current_ofs(1, 0); + tmp = timeval_current_ofs(15, 0); endtime = timeval_min(&endtime, &tmp); } @@ -381,7 +381,7 @@ static void smbd_smb1_do_locks_retry(struct tevent_req *subreq) DBG_DEBUG("Blocked on a posix lock. Retry in one second\n"); - tmp = timeval_current_ofs(1, 0); + tmp = timeval_current_ofs(15, 0); endtime = timeval_min(&endtime, &tmp); } -- 2.17.1 From d3d85331c3b32f498d545ade48dc10cbfff0df8d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 18:34:36 +0200 Subject: [PATCH 09/39] s3:blocking: split smbd_smb1_do_locks_retry() into _try() and _retry() This will make it possible to have just one caller to smbd_do_locks_try() later and use smbd_smb1_do_locks_try() from within smbd_smb1_do_locks_send(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Reviewed-by: Volker Lendecke Signed-off-by: Stefan Metzmacher (cherry picked from commit e79fcfaaf2ecfca6c3747f6fe4be51f332ebf10d) --- source3/smbd/blocking.c | 72 ++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 91438fe44860..0fa39ae58ab9 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -109,6 +109,7 @@ struct smbd_smb1_do_locks_state { uint16_t blocker; }; +static void smbd_smb1_do_locks_try(struct tevent_req *req); static void smbd_smb1_do_locks_retry(struct tevent_req *subreq); static void smbd_smb1_blocked_locks_cleanup( struct tevent_req *req, enum tevent_req_state req_state); @@ -300,10 +301,8 @@ static void smbd_smb1_blocked_locks_cleanup( fsp, blocked, struct tevent_req *, num_blocked-1); } -static void smbd_smb1_do_locks_retry(struct tevent_req *subreq) +static void smbd_smb1_do_locks_try(struct tevent_req *req) { - struct tevent_req *req = tevent_req_callback_data( - subreq, struct tevent_req); struct smbd_smb1_do_locks_state *state = tevent_req_data( req, struct smbd_smb1_do_locks_state); struct files_struct *fsp = state->fsp; @@ -315,36 +314,10 @@ static void smbd_smb1_do_locks_retry(struct tevent_req *subreq) struct timeval endtime; struct server_id blocking_pid = { 0 }; uint64_t blocking_smblctx = 0; + struct tevent_req *subreq = NULL; NTSTATUS status; bool ok; - /* - * Make sure we run as the user again - */ - ok = change_to_user_by_fsp(state->fsp); - if (!ok) { - tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); - return; - } - - status = dbwrap_watched_watch_recv(subreq, NULL, NULL); - TALLOC_FREE(subreq); - - DBG_DEBUG("dbwrap_watched_watch_recv returned %s\n", - nt_errstr(status)); - - if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) { - double elapsed = timeval_elapsed(&state->endtime); - if (elapsed > 0) { - smbd_smb1_brl_finish_by_req( - req, NT_STATUS_FILE_LOCK_CONFLICT); - return; - } - /* - * This is a posix lock retry. Just retry. - */ - } - lck = get_existing_share_mode_lock(state, fsp->file_id); if (tevent_req_nomem(lck, req)) { DBG_DEBUG("Could not get share mode lock\n"); @@ -396,6 +369,45 @@ done: smbd_smb1_brl_finish_by_req(req, status); } +static void smbd_smb1_do_locks_retry(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct smbd_smb1_do_locks_state *state = tevent_req_data( + req, struct smbd_smb1_do_locks_state); + NTSTATUS status; + bool ok; + + /* + * Make sure we run as the user again + */ + ok = change_to_user_by_fsp(state->fsp); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return; + } + + status = dbwrap_watched_watch_recv(subreq, NULL, NULL); + TALLOC_FREE(subreq); + + DBG_DEBUG("dbwrap_watched_watch_recv returned %s\n", + nt_errstr(status)); + + if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) { + double elapsed = timeval_elapsed(&state->endtime); + if (elapsed > 0) { + smbd_smb1_brl_finish_by_req( + req, NT_STATUS_FILE_LOCK_CONFLICT); + return; + } + /* + * This is a posix lock retry. Just retry. + */ + } + + smbd_smb1_do_locks_try(req); +} + NTSTATUS smbd_smb1_do_locks_recv(struct tevent_req *req) { struct smbd_smb1_do_locks_state *state = tevent_req_data( -- 2.17.1 From 1dad680f6b689a069e81e32e2a61ad62d23cc714 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 18:34:36 +0200 Subject: [PATCH 10/39] s3:blocking: move from 'timeout' to 'smbd_smb1_do_locks_state->timeout' This will make it possible to just use smbd_smb1_do_locks_try() in a later commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 8fe708acb43ea36d0cbf398713b125daba180a2d) --- source3/smbd/blocking.c | 20 +++++++++++--------- source3/smbd/proto.h | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 0fa39ae58ab9..81facc43154d 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -101,6 +101,7 @@ struct smbd_smb1_do_locks_state { struct tevent_context *ev; struct smb_request *smbreq; struct files_struct *fsp; + uint32_t timeout; struct timeval endtime; bool large_offset; /* required for correct cancel */ enum brl_flavour lock_flav; @@ -119,7 +120,7 @@ struct tevent_req *smbd_smb1_do_locks_send( struct tevent_context *ev, struct smb_request **smbreq, /* talloc_move()d into our state */ struct files_struct *fsp, - uint32_t timeout, + uint32_t lock_timeout, bool large_offset, enum brl_flavour lock_flav, uint16_t num_locks, @@ -142,6 +143,7 @@ struct tevent_req *smbd_smb1_do_locks_send( state->ev = ev; state->smbreq = talloc_move(state, smbreq); state->fsp = fsp; + state->timeout = lock_timeout; state->large_offset = large_offset; state->lock_flav = lock_flav; state->num_locks = num_locks; @@ -155,13 +157,13 @@ struct tevent_req *smbd_smb1_do_locks_send( return tevent_req_post(req, ev); } - if ((timeout != 0) && (timeout != UINT32_MAX)) { + if ((state->timeout != 0) && (state->timeout != UINT32_MAX)) { /* * Windows internal resolution for blocking locks * seems to be about 200ms... Don't wait for less than * that. JRA. */ - timeout = MAX(timeout, lp_lock_spin_time()); + state->timeout = MAX(state->timeout, lp_lock_spin_time()); } lck = get_existing_share_mode_lock(state, state->fsp->file_id); @@ -187,7 +189,7 @@ struct tevent_req *smbd_smb1_do_locks_send( goto done; } - if (timeout == 0) { + if (state->timeout == 0) { struct smbd_lock_element *blocker = &locks[state->blocker]; if ((blocker->offset >= 0xEF000000) && @@ -196,7 +198,7 @@ struct tevent_req *smbd_smb1_do_locks_send( * This must be an optimization of an ancient * application bug... */ - timeout = lp_lock_spin_time(); + state->timeout = lp_lock_spin_time(); } if ((fsp->lock_failure_seen) && @@ -208,15 +210,15 @@ struct tevent_req *smbd_smb1_do_locks_send( */ DBG_DEBUG("Delaying lock request due to previous " "failure\n"); - timeout = lp_lock_spin_time(); + state->timeout = lp_lock_spin_time(); } } DBG_DEBUG("timeout=%"PRIu32", blocking_smblctx=%"PRIu64"\n", - timeout, + state->timeout, blocking_smblctx); - if (timeout == 0) { + if (state->timeout == 0) { tevent_req_nterror(req, status); goto done; } @@ -229,7 +231,7 @@ struct tevent_req *smbd_smb1_do_locks_send( TALLOC_FREE(lck); tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); - state->endtime = timeval_current_ofs_msec(timeout); + state->endtime = timeval_current_ofs_msec(state->timeout); endtime = state->endtime; if (blocking_smblctx == UINT64_MAX) { diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index b59f1f4123da..cd1ec9a1f9e8 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -108,7 +108,7 @@ struct tevent_req *smbd_smb1_do_locks_send( struct tevent_context *ev, struct smb_request **smbreq, /* talloc_move()d into our state */ struct files_struct *fsp, - uint32_t timeout, + uint32_t lock_timeout, bool large_offset, enum brl_flavour lock_flav, uint16_t num_locks, -- 2.17.1 From decdf07cc3eee875cfd427435e84cd80f486c17a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 14:21:38 +0200 Subject: [PATCH 11/39] s3:blocking: fix posix lock retry We should evaluate the timeout condition after the very last retry and not before. Otherwise we'd fail to retry when waiting for posix locks. The problem happens if the client provided timeout is smaller than the 1 sec (for testing temporary 15 secs) retry. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit e8d719d31f885d7b6d5b317165f90ec40df169c9) --- selftest/knownfail.d/lock9 | 1 - source3/smbd/blocking.c | 36 +++++++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 12 deletions(-) delete mode 100644 selftest/knownfail.d/lock9 diff --git a/selftest/knownfail.d/lock9 b/selftest/knownfail.d/lock9 deleted file mode 100644 index 044622586eb4..000000000000 --- a/selftest/knownfail.d/lock9 +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.*.LOCK9B diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 81facc43154d..587923aa5ec1 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -319,6 +319,7 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) struct tevent_req *subreq = NULL; NTSTATUS status; bool ok; + double elapsed; lck = get_existing_share_mode_lock(state, fsp->file_id); if (tevent_req_nomem(lck, req)) { @@ -341,6 +342,24 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) goto done; } + /* + * The client specified timeout elapsed + * avoid further retries. + * + * Otherwise keep waiting either waiting + * for changes in locking.tdb or the polling + * mode timers waiting for posix locks. + */ + elapsed = timeval_elapsed(&state->endtime); + if (elapsed > 0) { + /* + * On timeout we always return + * NT_STATUS_FILE_LOCK_CONFLICT + */ + status = NT_STATUS_FILE_LOCK_CONFLICT; + goto done; + } + subreq = dbwrap_watched_watch_send( state, state->ev, lck->data->record, blocking_pid); if (tevent_req_nomem(subreq, req)) { @@ -395,17 +414,12 @@ static void smbd_smb1_do_locks_retry(struct tevent_req *subreq) DBG_DEBUG("dbwrap_watched_watch_recv returned %s\n", nt_errstr(status)); - if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) { - double elapsed = timeval_elapsed(&state->endtime); - if (elapsed > 0) { - smbd_smb1_brl_finish_by_req( - req, NT_STATUS_FILE_LOCK_CONFLICT); - return; - } - /* - * This is a posix lock retry. Just retry. - */ - } + /* + * We ignore any errors here, it's most likely + * we just get NT_STATUS_OK or NT_STATUS_IO_TIMEOUT. + * + * In any case we can just give it a retry. + */ smbd_smb1_do_locks_try(req); } -- 2.17.1 From cc45f119042051e0e3a3f89efc0f6ad639435c51 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 16:44:11 +0200 Subject: [PATCH 12/39] s3:blocking: Remove bug reproducer from a few commits ago The problem is fixed, now we can revert the change that made it easier to trigger. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 62ec58b06c38ee82bb3147c4d325413fd3a76499) --- source3/smbd/blocking.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 587923aa5ec1..af889a10d621 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -239,7 +239,7 @@ struct tevent_req *smbd_smb1_do_locks_send( DBG_DEBUG("Blocked on a posix lock. Retry in one second\n"); - tmp = timeval_current_ofs(15, 0); + tmp = timeval_current_ofs(1, 0); endtime = timeval_min(&endtime, &tmp); } @@ -375,7 +375,7 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) DBG_DEBUG("Blocked on a posix lock. Retry in one second\n"); - tmp = timeval_current_ofs(15, 0); + tmp = timeval_current_ofs(1, 0); endtime = timeval_min(&endtime, &tmp); } -- 2.17.1 From bae2c2390eb39eabd445321a4cac28e0ce28f7ae Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 17:26:43 +0200 Subject: [PATCH 13/39] s3:blocking: use dynamic posix lock wait intervals We want to start with a short timeout (200ms) and slow down to larger timeouts up to 2s for the default value of "lock spin time". BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 15765644d2590d6549f8fcc01c39c56387eed654) --- source3/smbd/blocking.c | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index af889a10d621..50e1d436eb75 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -102,6 +102,7 @@ struct smbd_smb1_do_locks_state { struct smb_request *smbreq; struct files_struct *fsp; uint32_t timeout; + uint32_t polling_msecs; struct timeval endtime; bool large_offset; /* required for correct cancel */ enum brl_flavour lock_flav; @@ -115,6 +116,32 @@ static void smbd_smb1_do_locks_retry(struct tevent_req *subreq); static void smbd_smb1_blocked_locks_cleanup( struct tevent_req *req, enum tevent_req_state req_state); +static void smbd_smb1_do_locks_update_polling_msecs( + struct smbd_smb1_do_locks_state *state) +{ + /* + * The default lp_lock_spin_time() is 200ms. + * + * v_min is in the range of 0.002 to 20 secs + * (0.2 secs by default) + * + * v_max is in the range of 0.02 to 200 secs + * (2.0 secs by default) + * + * The typical steps are: + * 0.2, 0.4, 0.6, 0.8, ... 2.0 + */ + uint32_t v_min = MAX(2, MIN(20000, lp_lock_spin_time())); + uint32_t v_max = 10 * v_min; + + if (state->polling_msecs >= v_max) { + state->polling_msecs = v_max; + return; + } + + state->polling_msecs += v_min; +} + struct tevent_req *smbd_smb1_do_locks_send( TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -237,9 +264,12 @@ struct tevent_req *smbd_smb1_do_locks_send( if (blocking_smblctx == UINT64_MAX) { struct timeval tmp; - DBG_DEBUG("Blocked on a posix lock. Retry in one second\n"); + smbd_smb1_do_locks_update_polling_msecs(state); + + DBG_DEBUG("Blocked on a posix lock. Retry in %"PRIu32" msecs\n", + state->polling_msecs); - tmp = timeval_current_ofs(1, 0); + tmp = timeval_current_ofs_msec(state->polling_msecs); endtime = timeval_min(&endtime, &tmp); } @@ -373,9 +403,12 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) if (blocking_smblctx == UINT64_MAX) { struct timeval tmp; - DBG_DEBUG("Blocked on a posix lock. Retry in one second\n"); + smbd_smb1_do_locks_update_polling_msecs(state); + + DBG_DEBUG("Blocked on a posix lock. Retry in %"PRIu32" msecs\n", + state->polling_msecs); - tmp = timeval_current_ofs(1, 0); + tmp = timeval_current_ofs_msec(state->polling_msecs); endtime = timeval_min(&endtime, &tmp); } -- 2.17.1 From e7232c0e906f1bede3071586e8583fe8e8817fdc Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 16:30:16 +0200 Subject: [PATCH 14/39] s4:torture/raw: assert to get LOCK_NOT_GRANTED in torture_samba3_posixtimedlock() There should not be a different if the blocker is a posix process instead of another smbd. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 2a77025a1e16d897281e5840192c93fa03328681) --- selftest/knownfail.d/samba3posixtimedlock | 1 + source4/torture/raw/samba3misc.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 selftest/knownfail.d/samba3posixtimedlock diff --git a/selftest/knownfail.d/samba3posixtimedlock b/selftest/knownfail.d/samba3posixtimedlock new file mode 100644 index 000000000000..56d2d349e1ea --- /dev/null +++ b/selftest/knownfail.d/samba3posixtimedlock @@ -0,0 +1 @@ +^samba3.raw.samba3posixtimedlock.samba3posixtimedlock diff --git a/source4/torture/raw/samba3misc.c b/source4/torture/raw/samba3misc.c index dc460b9cd8b9..2f484023bea8 100644 --- a/source4/torture/raw/samba3misc.c +++ b/source4/torture/raw/samba3misc.c @@ -775,8 +775,8 @@ static void receive_lock_result(struct smbcli_request *req) } /* - * Check that Samba3 correctly deals with conflicting posix byte range locks - * on an underlying file + * Check that Samba3 correctly deals with conflicting local posix byte range + * locks on an underlying file via "normal" SMB1 (without unix extentions). * * Note: This test depends on "posix locking = yes". * Note: To run this test, use "--option=torture:localdir=" @@ -873,7 +873,7 @@ bool torture_samba3_posixtimedlock(struct torture_context *tctx, struct smbcli_s status = smb_raw_lock(cli->tree, &io); ret = true; - CHECK_STATUS(tctx, status, NT_STATUS_FILE_LOCK_CONFLICT); + CHECK_STATUS(tctx, status, NT_STATUS_LOCK_NOT_GRANTED); if (!ret) { goto done; -- 2.17.1 From dec0c40f6147ba49af7f74db9d9b90190cf7087c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 12:04:43 +0200 Subject: [PATCH 15/39] s3:blocking: maintain state->deny_status For Windows locks we start with LOCK_NOT_GRANTED and use FILE_LOCK_CONFLICT if we retried after a timeout. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit aba0ee46258f3dd910421facb742fce3318a6946) --- selftest/knownfail.d/samba3posixtimedlock | 1 - source3/smbd/blocking.c | 33 +++++++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) delete mode 100644 selftest/knownfail.d/samba3posixtimedlock diff --git a/selftest/knownfail.d/samba3posixtimedlock b/selftest/knownfail.d/samba3posixtimedlock deleted file mode 100644 index 56d2d349e1ea..000000000000 --- a/selftest/knownfail.d/samba3posixtimedlock +++ /dev/null @@ -1 +0,0 @@ -^samba3.raw.samba3posixtimedlock.samba3posixtimedlock diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 50e1d436eb75..a87d62d910aa 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -109,6 +109,7 @@ struct smbd_smb1_do_locks_state { uint16_t num_locks; struct smbd_lock_element *locks; uint16_t blocker; + NTSTATUS deny_status; }; static void smbd_smb1_do_locks_try(struct tevent_req *req); @@ -176,6 +177,16 @@ struct tevent_req *smbd_smb1_do_locks_send( state->num_locks = num_locks; state->locks = locks; + if (lock_flav == POSIX_LOCK) { + /* + * SMB1 posix locks always use + * NT_STATUS_FILE_LOCK_CONFLICT. + */ + state->deny_status = NT_STATUS_FILE_LOCK_CONFLICT; + } else { + state->deny_status = NT_STATUS_LOCK_NOT_GRANTED; + } + DBG_DEBUG("state=%p, state->smbreq=%p\n", state, state->smbreq); if (num_locks == 0) { @@ -245,10 +256,19 @@ struct tevent_req *smbd_smb1_do_locks_send( state->timeout, blocking_smblctx); + /* + * If the endtime is not elapsed yet, + * it means we'll retry after a timeout. + * In that case we'll have to return + * NT_STATUS_FILE_LOCK_CONFLICT + * instead of NT_STATUS_LOCK_NOT_GRANTED. + */ if (state->timeout == 0) { + status = state->deny_status; tevent_req_nterror(req, status); goto done; } + state->deny_status = NT_STATUS_FILE_LOCK_CONFLICT; subreq = dbwrap_watched_watch_send( state, state->ev, lck->data->record, blocking_pid); @@ -379,16 +399,19 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) * Otherwise keep waiting either waiting * for changes in locking.tdb or the polling * mode timers waiting for posix locks. + * + * If the endtime is not expired yet, + * it means we'll retry after a timeout. + * In that case we'll have to return + * NT_STATUS_FILE_LOCK_CONFLICT + * instead of NT_STATUS_LOCK_NOT_GRANTED. */ elapsed = timeval_elapsed(&state->endtime); if (elapsed > 0) { - /* - * On timeout we always return - * NT_STATUS_FILE_LOCK_CONFLICT - */ - status = NT_STATUS_FILE_LOCK_CONFLICT; + status = state->deny_status; goto done; } + state->deny_status = NT_STATUS_FILE_LOCK_CONFLICT; subreq = dbwrap_watched_watch_send( state, state->ev, lck->data->record, blocking_pid); -- 2.17.1 From ac2d02f2143e22accaf5b266158f5b010e0a378f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 12:33:28 +0200 Subject: [PATCH 16/39] s3:brlock: always return LOCK_NOT_GRANTED instead of FILE_LOCK_CONFLICT Returning NT_STATUS_FILE_LOCK_CONFLICT is a SMB1 only detail for delayed brlock requests, which is handled in smbd_smb1_do_locks*(). The brlock layer should be consistent even for posix locks. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Signed-off-by: Volker Lendecke (cherry picked from commit ad98eec6090430ba5296a5111dde2e53b9cd217a) --- source3/locking/brlock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c index 628c2574357d..0a85bd0b057b 100644 --- a/source3/locking/brlock.c +++ b/source3/locking/brlock.c @@ -462,7 +462,7 @@ NTSTATUS brl_lock_windows_default(struct byte_range_lock *br_lck, plock->context.smblctx = 0xFFFFFFFFFFFFFFFFLL; if (errno_ret == EACCES || errno_ret == EAGAIN) { - status = NT_STATUS_FILE_LOCK_CONFLICT; + status = NT_STATUS_LOCK_NOT_GRANTED; goto fail; } else { status = map_nt_error_from_unix(errno); @@ -829,7 +829,7 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, TALLOC_FREE(tp); /* Remember who blocked us. */ plock->context.smblctx = curr_lock->context.smblctx; - return NT_STATUS_FILE_LOCK_CONFLICT; + return NT_STATUS_LOCK_NOT_GRANTED; } /* Just copy the Windows lock into the new array. */ memcpy(&tp[count], curr_lock, sizeof(struct lock_struct)); @@ -849,7 +849,7 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, TALLOC_FREE(tp); /* Remember who blocked us. */ plock->context.smblctx = curr_lock->context.smblctx; - return NT_STATUS_FILE_LOCK_CONFLICT; + return NT_STATUS_LOCK_NOT_GRANTED; } /* Work out overlaps. */ @@ -912,7 +912,7 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, if (errno_ret == EACCES || errno_ret == EAGAIN) { TALLOC_FREE(tp); - status = NT_STATUS_FILE_LOCK_CONFLICT; + status = NT_STATUS_LOCK_NOT_GRANTED; goto fail; } else { TALLOC_FREE(tp); -- 2.17.1 From fbb16fb8fe6b998a028a31023944c9a8f69f1a6b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 16:14:23 +0200 Subject: [PATCH 17/39] s3:smb2_lock: move from 'blocking' to 'state->blocking' This will simplify the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit f13d13ae9da3072862a781bc926e7a06e8384337) --- source3/smbd/smb2_lock.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index e9c8d7f890e6..4cf735ff48d0 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -42,6 +42,7 @@ struct smbd_smb2_lock_state { struct smbd_smb2_request *smb2req; struct smb_request *smb1req; struct files_struct *fsp; + bool blocking; uint16_t lock_count; struct smbd_lock_element *locks; }; @@ -200,7 +201,6 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, { struct tevent_req *req; struct smbd_smb2_lock_state *state; - bool blocking = false; bool isunlock = false; uint16_t i; struct smbd_lock_element *locks; @@ -241,7 +241,7 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); return tevent_req_post(req, ev); } - blocking = true; + state->blocking = true; break; case SMB2_LOCK_FLAG_SHARED|SMB2_LOCK_FLAG_FAIL_IMMEDIATELY: @@ -383,7 +383,7 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - if (blocking && + if (state->blocking && (NT_STATUS_EQUAL(status, NT_STATUS_LOCK_NOT_GRANTED) || NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT))) { struct tevent_req *subreq; -- 2.17.1 From 3e16d8c8ba80c58086be0dfd15d557265b5152f3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 16:39:41 +0200 Subject: [PATCH 18/39] s3:smb2_lock: split smbd_smb2_lock_retry() into _try() and _retry() This makes it possible to reuse _try() in the _send() function in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit d096742da1a045357f52ccd5b28d499c30e96152) --- source3/smbd/smb2_lock.c | 49 ++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index 4cf735ff48d0..c2b4603f3e10 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -189,6 +189,7 @@ static void smbd_smb2_request_lock_done(struct tevent_req *subreq) } } +static void smbd_smb2_lock_try(struct tevent_req *req); static void smbd_smb2_lock_retry(struct tevent_req *subreq); static bool smbd_smb2_lock_cancel(struct tevent_req *req); @@ -410,10 +411,8 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } -static void smbd_smb2_lock_retry(struct tevent_req *subreq) +static void smbd_smb2_lock_try(struct tevent_req *req) { - struct tevent_req *req = tevent_req_callback_data( - subreq, struct tevent_req); struct smbd_smb2_lock_state *state = tevent_req_data( req, struct smbd_smb2_lock_state); struct share_mode_lock *lck = NULL; @@ -421,22 +420,7 @@ static void smbd_smb2_lock_retry(struct tevent_req *subreq) struct server_id blocking_pid = { 0 }; uint64_t blocking_smblctx; NTSTATUS status; - bool ok; - - /* - * Make sure we run as the user again - */ - ok = change_to_user_by_fsp(state->fsp); - if (!ok) { - tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); - return; - } - - status = dbwrap_watched_watch_recv(subreq, NULL, NULL); - TALLOC_FREE(subreq); - if (tevent_req_nterror(req, status)) { - return; - } + struct tevent_req *subreq = NULL; lck = get_existing_share_mode_lock( talloc_tos(), state->fsp->file_id); @@ -467,6 +451,33 @@ static void smbd_smb2_lock_retry(struct tevent_req *subreq) tevent_req_set_callback(subreq, smbd_smb2_lock_retry, req); } +static void smbd_smb2_lock_retry(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct smbd_smb2_lock_state *state = tevent_req_data( + req, struct smbd_smb2_lock_state); + NTSTATUS status; + bool ok; + + /* + * Make sure we run as the user again + */ + ok = change_to_user_by_fsp(state->fsp); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return; + } + + status = dbwrap_watched_watch_recv(subreq, NULL, NULL); + TALLOC_FREE(subreq); + if (tevent_req_nterror(req, status)) { + return; + } + + smbd_smb2_lock_try(req); +} + static NTSTATUS smbd_smb2_lock_recv(struct tevent_req *req) { return tevent_req_simple_recv_ntstatus(req); -- 2.17.1 From ccf662c4b13a190d2b7589570e88ce9c7cab3b4b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 16:39:41 +0200 Subject: [PATCH 19/39] s3:smb2_lock: error out early in smbd_smb2_lock_send() We no longer expect NT_STATUS_FILE_LOCK_CONFLICT from the VFS layer and assert that in a future version. This makes it easier to port the same logic to smbd_smb2_lock_try(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 39d514cdc358f175d0968f4a78f8f2f05a6c1707) --- source3/smbd/smb2_lock.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index c2b4603f3e10..c049c33ebbcb 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -383,10 +383,26 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, tevent_req_done(req); return tevent_req_post(req, ev); } + if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT)) { + /* + * This is a bug and will be changed into an assert + * in a future version. We should only + * ever get NT_STATUS_LOCK_NOT_GRANTED here! + */ + static uint64_t _bug_count; + int _level = (_bug_count++ == 0) ? DBGLVL_ERR: DBGLVL_DEBUG; + DBG_PREFIX(_level, ("BUG: Got %s mapping to " + "NT_STATUS_LOCK_NOT_GRANTED\n", + nt_errstr(status))); + status = NT_STATUS_LOCK_NOT_GRANTED; + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_LOCK_NOT_GRANTED)) { + TALLOC_FREE(lck); + tevent_req_nterror(req, status); + return tevent_req_post(req, ev); + } - if (state->blocking && - (NT_STATUS_EQUAL(status, NT_STATUS_LOCK_NOT_GRANTED) || - NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT))) { + if (state->blocking) { struct tevent_req *subreq; DBG_DEBUG("Watching share mode lock\n"); -- 2.17.1 From 3b7e085bcf603c454ef587285938af0e54a55338 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 16:39:41 +0200 Subject: [PATCH 20/39] s3:smb2_lock: let smbd_smb2_lock_try() explicitly check for the retry condition This makes it possible to reuse _try() in the _send() function in the next commit. We should not retry forever on a hard error. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 7f77e0b4e9878f1f3515206d052adc012e26aafb) --- source3/smbd/smb2_lock.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index c049c33ebbcb..153705b26a16 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -457,6 +457,32 @@ static void smbd_smb2_lock_try(struct tevent_req *req) tevent_req_done(req); return; } + if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT)) { + /* + * This is a bug and will be changed into an assert + * in future version. We should only + * ever get NT_STATUS_LOCK_NOT_GRANTED here! + */ + static uint64_t _bug_count; + int _level = (_bug_count++ == 0) ? DBGLVL_ERR: DBGLVL_DEBUG; + DBG_PREFIX(_level, ("BUG: Got %s mapping to " + "NT_STATUS_LOCK_NOT_GRANTED\n", + nt_errstr(status))); + status = NT_STATUS_LOCK_NOT_GRANTED; + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_LOCK_NOT_GRANTED)) { + TALLOC_FREE(lck); + tevent_req_nterror(req, status); + return; + } + + if (!state->blocking) { + TALLOC_FREE(lck); + tevent_req_nterror(req, status); + return; + } + + DBG_DEBUG("Watching share mode lock\n"); subreq = dbwrap_watched_watch_send( state, state->ev, lck->data->record, blocking_pid); -- 2.17.1 From 2eb696f10109305031b1a2d7b6d77759871c0c29 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Aug 2019 16:39:41 +0200 Subject: [PATCH 21/39] s3:smb2_lock: make use of smbd_smb2_lock_try() in smbd_smb2_lock_send() We only need the logic to call smbd_do_locks_try() and a possible retry once in smbd_smb2_lock_try(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher (cherry picked from commit 359e9992be713bbecfdb19998d69e1d3f020c5e9) --- source3/smbd/smb2_lock.c | 68 ++++------------------------------------ 1 file changed, 6 insertions(+), 62 deletions(-) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index 153705b26a16..a8ccf21cc200 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -205,10 +205,6 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, bool isunlock = false; uint16_t i; struct smbd_lock_element *locks; - struct share_mode_lock *lck = NULL; - uint16_t blocker_idx; - struct server_id blocking_pid = { 0 }; - uint64_t blocking_smblctx; NTSTATUS status; req = tevent_req_create(mem_ctx, &state, @@ -363,68 +359,16 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - lck = get_existing_share_mode_lock( - talloc_tos(), state->fsp->file_id); - if (tevent_req_nomem(lck, req)) { - return tevent_req_post(req, ev); - } - - status = smbd_do_locks_try( - state->fsp, - WINDOWS_LOCK, - state->lock_count, - state->locks, - &blocker_idx, - &blocking_pid, - &blocking_smblctx); - - if (NT_STATUS_IS_OK(status)) { - TALLOC_FREE(lck); - tevent_req_done(req); - return tevent_req_post(req, ev); - } - if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT)) { - /* - * This is a bug and will be changed into an assert - * in a future version. We should only - * ever get NT_STATUS_LOCK_NOT_GRANTED here! - */ - static uint64_t _bug_count; - int _level = (_bug_count++ == 0) ? DBGLVL_ERR: DBGLVL_DEBUG; - DBG_PREFIX(_level, ("BUG: Got %s mapping to " - "NT_STATUS_LOCK_NOT_GRANTED\n", - nt_errstr(status))); - status = NT_STATUS_LOCK_NOT_GRANTED; - } - if (!NT_STATUS_EQUAL(status, NT_STATUS_LOCK_NOT_GRANTED)) { - TALLOC_FREE(lck); - tevent_req_nterror(req, status); + smbd_smb2_lock_try(req); + if (!tevent_req_is_in_progress(req)) { return tevent_req_post(req, ev); } - if (state->blocking) { - struct tevent_req *subreq; - - DBG_DEBUG("Watching share mode lock\n"); - - subreq = dbwrap_watched_watch_send( - state, state->ev, lck->data->record, blocking_pid); - TALLOC_FREE(lck); - if (tevent_req_nomem(subreq, req)) { - return tevent_req_post(req, ev); - } - tevent_req_set_callback(subreq, smbd_smb2_lock_retry, req); - - tevent_req_defer_callback(req, smb2req->sconn->ev_ctx); - aio_add_req_to_fsp(state->fsp, req); - tevent_req_set_cancel_fn(req, smbd_smb2_lock_cancel); + tevent_req_defer_callback(req, smb2req->sconn->ev_ctx); + aio_add_req_to_fsp(state->fsp, req); + tevent_req_set_cancel_fn(req, smbd_smb2_lock_cancel); - return req; - } - - TALLOC_FREE(lck); - tevent_req_nterror(req, status); - return tevent_req_post(req, ev); + return req; } static void smbd_smb2_lock_try(struct tevent_req *req) -- 2.17.1 From c71d9e42a49a3b2723bbae7ca58748d1fd375de5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 17:38:30 +0200 Subject: [PATCH 22/39] s4:torture/smb2: add smb2.samba3misc.localposixlock1 This demonstrates that the SMB2 code path doesn't do any retry for local posix locks. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 7155d3a2c5d7237f00cccb1802c1341cf295864e) --- selftest/knownfail.d/smb2.localposixlock | 1 + selftest/skip | 1 + source3/selftest/tests.py | 2 +- source4/torture/smb2/samba3misc.c | 188 +++++++++++++++++++++++ source4/torture/smb2/smb2.c | 1 + source4/torture/smb2/wscript_build | 1 + 6 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 selftest/knownfail.d/smb2.localposixlock create mode 100644 source4/torture/smb2/samba3misc.c diff --git a/selftest/knownfail.d/smb2.localposixlock b/selftest/knownfail.d/smb2.localposixlock new file mode 100644 index 000000000000..1b84f074e0b5 --- /dev/null +++ b/selftest/knownfail.d/smb2.localposixlock @@ -0,0 +1 @@ +^samba3.smb2.samba3misc.localposixlock1 diff --git a/selftest/skip b/selftest/skip index c471072e88f6..11bf29599fab 100644 --- a/selftest/skip +++ b/selftest/skip @@ -93,6 +93,7 @@ ^samba4.rpc.samr.passwords.*ncacn_np\(ad_dc_slowtests\) # currently fails, possibly config issue ^samba4.rpc.samr.passwords.*s4member # currently fails, possibly config issue ^samba4.raw.scan.eamax +^samba4.smb2.samba3misc ^samba4.smb2.notify ^samba4.smb2.scan ^samba4.smb2.lease diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index a3daeacae6bf..ebc366de3ea2 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -529,7 +529,7 @@ for t in tests: plansmbtorture4testsuite(t, env, '//$SERVER/tmp -k yes -U$DC_USERNAME@$REALM%$DC_PASSWORD --option=torture:addc=$DC_SERVER', description='kerberos connection') plansmbtorture4testsuite(t, env, '//$SERVER/tmpguest -U% --option=torture:addc=$DC_SERVER', description='anonymous connection') plansmbtorture4testsuite(t, env, '//$SERVER/tmp -k no -U$DC_USERNAME@$REALM%$DC_PASSWORD', description='ntlm user@realm') - elif t == "raw.samba3posixtimedlock": + elif t == "raw.samba3posixtimedlock" or t == "smb2.samba3misc": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmpguest -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share') plansmbtorture4testsuite(t, "ad_dc", '//$SERVER_IP/tmpguest -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/ad_dc/share') elif t == "raw.chkpath": diff --git a/source4/torture/smb2/samba3misc.c b/source4/torture/smb2/samba3misc.c new file mode 100644 index 000000000000..a5fe6c1bbeab --- /dev/null +++ b/source4/torture/smb2/samba3misc.c @@ -0,0 +1,188 @@ +/* + Unix SMB/CIFS implementation. + + Test some misc Samba3 code paths + + Copyright (C) Volker Lendecke 2006 + Copyright (C) Stefan Metzmacher 2019 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "system/time.h" +#include "system/filesys.h" +#include "libcli/smb2/smb2.h" +#include "libcli/smb2/smb2_calls.h" +#include "../libcli/smb/smbXcli_base.h" +#include "torture/torture.h" +#include "torture/smb2/proto.h" +#include "torture/util.h" +#include "lib/events/events.h" +#include "param/param.h" + +#define CHECK_STATUS(status, correct) do { \ + const char *_cmt = "(" __location__ ")"; \ + torture_assert_ntstatus_equal_goto(tctx,status,correct, \ + ret,done,_cmt); \ + } while (0) + +#define BASEDIR "samba3misc.smb2" + +#define WAIT_FOR_ASYNC_RESPONSE(req) \ + while (!req->cancel.can_cancel && req->state <= SMB2_REQUEST_RECV) { \ + if (tevent_loop_once(tctx->ev) != 0) { \ + break; \ + } \ + } + +static void torture_smb2_tree_disconnect_timer(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval now, + void *private_data) +{ + struct smb2_tree *tree = + talloc_get_type_abort(private_data, + struct smb2_tree); + + smbXcli_conn_disconnect(tree->session->transport->conn, + NT_STATUS_CTX_CLIENT_QUERY_TIMEOUT); +} + +/* + * Check that Samba3 correctly deals with conflicting local posix byte range + * locks on an underlying file via "normal" SMB2 (without posix extentions). + * + * Note: This test depends on "posix locking = yes". + * Note: To run this test, use "--option=torture:localdir=" + */ +static bool torture_samba3_localposixlock1(struct torture_context *tctx, + struct smb2_tree *tree) +{ + NTSTATUS status; + bool ret = true; + int rc; + const char *fname = "posixtimedlock.dat"; + const char *fpath; + const char *localdir; + const char *localname; + struct smb2_handle h = {{0}}; + struct smb2_lock lck = {0}; + struct smb2_lock_element el[1] = {{0}}; + struct smb2_request *req = NULL; + int fd = -1; + struct flock posix_lock; + struct tevent_timer *te; + + status = torture_smb2_testdir(tree, BASEDIR, &h); + CHECK_STATUS(status, NT_STATUS_OK); + smb2_util_close(tree, h); + + status = torture_smb2_testfile(tree, fname, &h); + CHECK_STATUS(status, NT_STATUS_OK); + + fpath = talloc_asprintf(tctx, "%s\\%s", BASEDIR, fname); + torture_assert(tctx, fpath != NULL, "fpath\n"); + + status = torture_smb2_testfile(tree, fpath, &h); + CHECK_STATUS(status, NT_STATUS_OK); + + localdir = torture_setting_string(tctx, "localdir", NULL); + torture_assert(tctx, localdir != NULL, + "--option=torture:localdir= required\n"); + + localname = talloc_asprintf(tctx, "%s/%s/%s", + localdir, BASEDIR, fname); + torture_assert(tctx, localname != NULL, "localname\n"); + + /* + * Lock a byte range from posix + */ + + torture_comment(tctx, " local open(%s)\n", localname); + fd = open(localname, O_RDWR); + if (fd == -1) { + torture_warning(tctx, "open(%s) failed: %s\n", + localname, strerror(errno)); + torture_assert(tctx, fd != -1, "open localname\n"); + } + + posix_lock.l_type = F_WRLCK; + posix_lock.l_whence = SEEK_SET; + posix_lock.l_start = 0; + posix_lock.l_len = 1; + + torture_comment(tctx, " local fcntl\n"); + rc = fcntl(fd, F_SETLK, &posix_lock); + if (rc == -1) { + torture_warning(tctx, "fcntl failed: %s\n", strerror(errno)); + torture_assert(tctx, rc != -1, "fcntl lock\n"); + } + + el[0].offset = 0; + el[0].length = 1; + el[0].reserved = 0x00000000; + el[0].flags = SMB2_LOCK_FLAG_EXCLUSIVE | + SMB2_LOCK_FLAG_FAIL_IMMEDIATELY; + lck.in.locks = el; + lck.in.lock_count = 0x0001; + lck.in.lock_sequence = 0x00000000; + lck.in.file.handle = h; + + torture_comment(tctx, " remote non-blocking lock\n"); + status = smb2_lock(tree, &lck); + CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED); + + torture_comment(tctx, " remote async blocking lock\n"); + el[0].flags = SMB2_LOCK_FLAG_EXCLUSIVE; + req = smb2_lock_send(tree, &lck); + torture_assert(tctx, req != NULL, "smb2_lock_send()\n"); + + te = tevent_add_timer(tctx->ev, + tctx, timeval_current_ofs(5, 0), + torture_smb2_tree_disconnect_timer, + tree); + torture_assert(tctx, te != NULL, "tevent_add_timer\n"); + + torture_comment(tctx, " remote wait for STATUS_PENDING\n"); + WAIT_FOR_ASYNC_RESPONSE(req); + + torture_comment(tctx, " local close file\n"); + close(fd); + fd = -1; + + torture_comment(tctx, " remote lock should now succeed\n"); + status = smb2_lock_recv(req, &lck); + CHECK_STATUS(status, NT_STATUS_OK); + +done: + if (fd != -1) { + close(fd); + } + smb2_util_close(tree, h); + smb2_deltree(tree, BASEDIR); + return ret; +} + +struct torture_suite *torture_smb2_samba3misc_init(TALLOC_CTX *ctx) +{ + struct torture_suite *suite = torture_suite_create(ctx, "samba3misc"); + + torture_suite_add_1smb2_test(suite, "localposixlock1", + torture_samba3_localposixlock1); + + suite->description = talloc_strdup(suite, "SMB2 Samba3 MISC"); + + return suite; +} diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c index f495c19d251f..e57dba3c1d94 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -195,6 +195,7 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx) torture_suite_add_suite(suite, torture_smb2_doc_init(suite)); torture_suite_add_suite(suite, torture_smb2_multichannel_init(suite)); + torture_suite_add_suite(suite, torture_smb2_samba3misc_init(suite)); suite->description = talloc_strdup(suite, "SMB2-specific tests"); diff --git a/source4/torture/smb2/wscript_build b/source4/torture/smb2/wscript_build index e605a4589ac1..8b17cfb36d44 100644 --- a/source4/torture/smb2/wscript_build +++ b/source4/torture/smb2/wscript_build @@ -34,6 +34,7 @@ bld.SAMBA_MODULE('TORTURE_SMB2', sharemode.c smb2.c streams.c + samba3misc.c util.c ''', subsystem='smbtorture', -- 2.17.1 From 606c32559c85bd63efc4ac2aa3ac9c65ab07bcb7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 2 Aug 2019 14:50:27 +0200 Subject: [PATCH 23/39] s3:smb2_lock: add retry for POSIX locks BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 8decf41bbb8be2b4ac463eb6ace16a8628276ab5) --- selftest/knownfail.d/smb2.localposixlock | 1 - source3/smbd/smb2_lock.c | 55 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/smb2.localposixlock diff --git a/selftest/knownfail.d/smb2.localposixlock b/selftest/knownfail.d/smb2.localposixlock deleted file mode 100644 index 1b84f074e0b5..000000000000 --- a/selftest/knownfail.d/smb2.localposixlock +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.samba3misc.localposixlock1 diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index a8ccf21cc200..8ba54fe69953 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -43,6 +43,7 @@ struct smbd_smb2_lock_state { struct smb_request *smb1req; struct files_struct *fsp; bool blocking; + uint32_t polling_msecs; uint16_t lock_count; struct smbd_lock_element *locks; }; @@ -371,6 +372,32 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, return req; } +static void smbd_smb2_lock_update_polling_msecs( + struct smbd_smb2_lock_state *state) +{ + /* + * The default lp_lock_spin_time() is 200ms. + * + * v_min is in the range of 0.002 to 20 secs + * (0.2 secs by default) + * + * v_max is in the range of 0.02 to 200 secs + * (2.0 secs by default) + * + * The typical steps are: + * 0.2, 0.4, 0.6, 0.8, ... 2.0 + */ + uint32_t v_min = MAX(2, MIN(20000, lp_lock_spin_time())); + uint32_t v_max = 10 * v_min; + + if (state->polling_msecs >= v_max) { + state->polling_msecs = v_max; + return; + } + + state->polling_msecs += v_min; +} + static void smbd_smb2_lock_try(struct tevent_req *req) { struct smbd_smb2_lock_state *state = tevent_req_data( @@ -381,6 +408,7 @@ static void smbd_smb2_lock_try(struct tevent_req *req) uint64_t blocking_smblctx; NTSTATUS status; struct tevent_req *subreq = NULL; + struct timeval endtime = { 0 }; lck = get_existing_share_mode_lock( talloc_tos(), state->fsp->file_id); @@ -426,6 +454,15 @@ static void smbd_smb2_lock_try(struct tevent_req *req) return; } + if (blocking_smblctx == UINT64_MAX) { + smbd_smb2_lock_update_polling_msecs(state); + + DBG_DEBUG("Blocked on a posix lock. Retry in %"PRIu32" msecs\n", + state->polling_msecs); + + endtime = timeval_current_ofs_msec(state->polling_msecs); + } + DBG_DEBUG("Watching share mode lock\n"); subreq = dbwrap_watched_watch_send( @@ -435,6 +472,18 @@ static void smbd_smb2_lock_try(struct tevent_req *req) return; } tevent_req_set_callback(subreq, smbd_smb2_lock_retry, req); + + if (!timeval_is_zero(&endtime)) { + bool ok; + + ok = tevent_req_set_endtime(subreq, + state->ev, + endtime); + if (!ok) { + tevent_req_oom(req); + return; + } + } } static void smbd_smb2_lock_retry(struct tevent_req *subreq) @@ -457,6 +506,12 @@ static void smbd_smb2_lock_retry(struct tevent_req *subreq) status = dbwrap_watched_watch_recv(subreq, NULL, NULL); TALLOC_FREE(subreq); + if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) { + /* + * This is just a trigger for a timed retry. + */ + status = NT_STATUS_OK; + } if (tevent_req_nterror(req, status)) { return; } -- 2.17.1 From fc8715f7e2422b4881a9d5cc180bd917cecc8269 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 16 Aug 2019 12:28:39 +0200 Subject: [PATCH 24/39] s4:torture/raw: improvements for multilock2 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 8a7039be530adcdda9e7e7621bdcf902f5ca1721) --- source4/torture/raw/lock.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c index f1fbdd6da715..f264d0aea114 100644 --- a/source4/torture/raw/lock.c +++ b/source4/torture/raw/lock.c @@ -2449,7 +2449,7 @@ static bool test_multilock2(struct torture_context *tctx, lock[0].pid = cli->session->pid+2; io.lockx.in.lock_cnt = 1; req2 = smb_raw_lock_send(cli->tree, &io); - torture_assert(tctx,(req != NULL), talloc_asprintf(tctx, + torture_assert(tctx,(req2 != NULL), talloc_asprintf(tctx, "Failed to setup timed locks (%s)\n", __location__)); /* Unlock lock[0] */ @@ -2465,6 +2465,9 @@ static bool test_multilock2(struct torture_context *tctx, status = smbcli_request_simple_recv(req2); CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + /* Start the clock. */ t = time_mono(NULL); -- 2.17.1 From 52a6462c27f281cf968f390fb45efea0f9bc211a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 16 Aug 2019 12:13:15 +0200 Subject: [PATCH 25/39] s4:torture/raw: add multilock3 test This demonstrates that unrelated lock ranges are not blocked by other blocked requests on the same fsp. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 297763c6b618c07148d788b46218a0798225bf79) --- selftest/knownfail.d/multilock | 1 + source4/torture/raw/lock.c | 266 +++++++++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 selftest/knownfail.d/multilock diff --git a/selftest/knownfail.d/multilock b/selftest/knownfail.d/multilock new file mode 100644 index 000000000000..e0e6e81c453f --- /dev/null +++ b/selftest/knownfail.d/multilock @@ -0,0 +1 @@ +^samba3.raw.lock.multilock3.*nt4_dc diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c index f264d0aea114..bea55f6a6054 100644 --- a/source4/torture/raw/lock.c +++ b/source4/torture/raw/lock.c @@ -2494,6 +2494,271 @@ done: return ret; } +/* + test multi3 Locking&X operation + This test is designed to show that + lock precedence on the server is based + on the order received, not on the ability + to grant. + + Compared to test_multilock2() (above) + this test demonstrates that completely + unrelated ranges work independently. + + For example: + + A blocked lock request containing 2 locks + will be satified before a subsequent blocked + lock request over one of the same regions, + even if that region is then unlocked. But + a lock of a different region goes through. E.g. + + All locks are LOCKING_ANDX_EXCLUSIVE_LOCK (rw). + + (a) lock 100->109, 120->129 (granted) + (b) lock 100->109, 120->129 (blocks, timeout=20s) + (c) lock 100->109 (blocks, timeout=2s) + (d) lock 110->119 (granted) + (e) lock 110->119 (blocks, timeout=20s) + (f) unlock 100->109 (a) + (g) lock 100->109 (not granted, blocked by (b)) + (h) lock 100->109 (not granted, blocked by itself (b)) + (i) lock (c) will not be granted(conflict, times out) + as lock (b) will take precedence. + (j) unlock 110-119 (d) + (k) lock (e) completes and is not blocked by (a) nor (b) + (l) lock 100->109 (not granted(conflict), blocked by (b)) + (m) lock 100->109 (not granted(conflict), blocked by itself (b)) + (n) unlock 120-129 (a) + (o) lock (b) completes +*/ +static bool test_multilock3(struct torture_context *tctx, + struct smbcli_state *cli) +{ + union smb_lock io; + struct smb_lock_entry lock[2]; + union smb_lock io3; + struct smb_lock_entry lock3[1]; + NTSTATUS status; + bool ret = true; + int fnum; + const char *fname = BASEDIR "\\multilock3_test.txt"; + time_t t; + struct smbcli_request *req = NULL; + struct smbcli_request *req2 = NULL; + struct smbcli_request *req4 = NULL; + + torture_assert(tctx, torture_setup_dir(cli, BASEDIR), + "Failed to setup up test directory: " BASEDIR); + + torture_comment(tctx, "Testing LOCKING_ANDX multi-lock 3\n"); + io.generic.level = RAW_LOCK_LOCKX; + + /* Create the test file. */ + fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE); + torture_assert(tctx,(fnum != -1), talloc_asprintf(tctx, + "Failed to create %s - %s\n", + fname, smbcli_errstr(cli->tree))); + + /* + * a) + * Lock regions 100->109, 120->129 as + * two separate write locks in one request. + */ + io.lockx.level = RAW_LOCK_LOCKX; + io.lockx.in.file.fnum = fnum; + io.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 2; + io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock[0].pid = cli->session->pid; + lock[0].offset = 100; + lock[0].count = 10; + lock[1].pid = cli->session->pid; + lock[1].offset = 120; + lock[1].count = 10; + io.lockx.in.locks = &lock[0]; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * b) + * Now request the same locks on a different + * context as blocking locks. + */ + io.lockx.in.timeout = 20000; + lock[0].pid = cli->session->pid+1; + lock[1].pid = cli->session->pid+1; + req = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * c) + * Request the first lock again on a separate context. + * Wait 2 seconds. This should time out (the previous + * multi-lock request should take precedence). + */ + io.lockx.in.timeout = 2000; + lock[0].pid = cli->session->pid+2; + io.lockx.in.lock_cnt = 1; + req2 = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req2 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * d) + * Lock regions 110->119 + */ + io3.lockx.level = RAW_LOCK_LOCKX; + io3.lockx.in.file.fnum = fnum; + io3.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 0; + io3.lockx.in.lock_cnt = 1; + io3.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock3[0].pid = cli->session->pid+3; + lock3[0].offset = 110; + lock3[0].count = 10; + io3.lockx.in.locks = &lock3[0]; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * e) + * try 110-119 again + */ + io3.lockx.in.timeout = 20000; + lock3[0].pid = cli->session->pid+4; + req4 = smb_raw_lock_send(cli->tree, &io3); + torture_assert(tctx,(req4 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * f) + * Unlock (a) lock[0] 100-109 + */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[0]; + lock[0].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * g) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+5; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED); + + /* + * h) + * try to lock lock[0] 100-109 again with + * the pid that's still waiting + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req2->state <= SMBCLI_REQUEST_RECV, + "req2 should still wait"); + + /* + * i) + * Did the second lock complete (should time out) ? + */ + status = smbcli_request_simple_recv(req2); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + torture_assert(tctx, req4->state <= SMBCLI_REQUEST_RECV, + "req4 should still wait"); + + /* + * j) + * Unlock (d) lock[0] 110-119 + */ + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 1; + io3.lockx.in.lock_cnt = 0; + lock3[0].pid = cli->session->pid+3; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * k) + * receive the successful blocked lock request (e) + * on 110-119 while the 100-109/120-129 is still waiting. + */ + status = smbcli_request_simple_recv(req4); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * l) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+6; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + + /* + * m) + * try to lock lock[0] 100-109 again with + * the pid that's still waiting + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + + /* Start the clock. */ + t = time_mono(NULL); + + /* + * n) + * Unlock lock[1] 120-129 */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[1]; + lock[1].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * o) + * receive the successful blocked lock request (b) + */ + status = smbcli_request_simple_recv(req); + CHECK_STATUS(status, NT_STATUS_OK); + + /* Fail if this took more than 2 seconds. */ + torture_assert(tctx,!(time_mono(NULL) > t+2), talloc_asprintf(tctx, + "Blocking locks were not granted immediately (%s)\n", + __location__)); +done: + smb_raw_exit(cli->session); + smbcli_deltree(cli->tree, BASEDIR); + return ret; +} /* basic testing of lock calls @@ -2517,6 +2782,7 @@ struct torture_suite *torture_raw_lock(TALLOC_CTX *mem_ctx) torture_suite_add_1smb_test(suite, "zerobyteread", test_zerobyteread); torture_suite_add_1smb_test(suite, "multilock", test_multilock); torture_suite_add_1smb_test(suite, "multilock2", test_multilock2); + torture_suite_add_1smb_test(suite, "multilock3", test_multilock3); return suite; } -- 2.17.1 From bb896d6a15e4ef25bf3f9ef03b3e1ea4eb041046 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 16 Aug 2019 14:49:29 +0200 Subject: [PATCH 26/39] s4:torture/raw: add multilock4 test This is similar to multilock3, but uses read-only (LOCKING_ANDX_SHARED_LOCK) locks for the blocked requests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit d3e65ceb1ec25c7b62a7e908506126269011f30d) --- selftest/knownfail.d/multilock | 1 + source4/torture/raw/lock.c | 266 +++++++++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) diff --git a/selftest/knownfail.d/multilock b/selftest/knownfail.d/multilock index e0e6e81c453f..01538acc5a88 100644 --- a/selftest/knownfail.d/multilock +++ b/selftest/knownfail.d/multilock @@ -1 +1,2 @@ ^samba3.raw.lock.multilock3.*nt4_dc +^samba3.raw.lock.multilock4.*nt4_dc diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c index bea55f6a6054..26339f9b28b7 100644 --- a/source4/torture/raw/lock.c +++ b/source4/torture/raw/lock.c @@ -2760,6 +2760,271 @@ done: return ret; } +/* + test multi4 Locking&X operation + This test is designed to show that + lock precedence on the server is based + on the order received, not on the ability + to grant. + + Compared to test_multilock3() (above) + this test demonstrates that pending read-only/shared + locks doesn't block shared locks others. + + The outstanding requests build an implicit + database that's checked before checking + the already granted locks in the real database. + + For example: + + A blocked read-lock request containing 2 locks + will be still be blocked, while one region + is still write-locked. While it doesn't block + other read-lock requests for the other region. E.g. + + (a) lock(rw) 100->109, 120->129 (granted) + (b) lock(ro) 100->109, 120->129 (blocks, timeout=20s) + (c) lock(ro) 100->109 (blocks, timeout=MAX) + (d) lock(rw) 110->119 (granted) + (e) lock(rw) 110->119 (blocks, timeout=20s) + (f) unlock 100->109 (a) + (g) lock(ro) (c) completes and is not blocked by (a) nor (b) + (h) lock(rw) 100->109 (not granted, blocked by (c)) + (i) lock(rw) 100->109 (pid (b)) (not granted(conflict), blocked by (c)) + (j) unlock 110-119 + (k) lock (e) completes and is not blocked by (a) nor (b) + (l) lock 100->109 (not granted(conflict), blocked by (b)) + (m) lock 100->109 (pid (b)) (not granted(conflict), blocked by itself (b)) + (n) unlock 120-129 (a) + (o) lock (b) completes +*/ +static bool test_multilock4(struct torture_context *tctx, + struct smbcli_state *cli) +{ + union smb_lock io; + struct smb_lock_entry lock[2]; + union smb_lock io3; + struct smb_lock_entry lock3[1]; + NTSTATUS status; + bool ret = true; + int fnum; + const char *fname = BASEDIR "\\multilock4_test.txt"; + time_t t; + struct smbcli_request *req = NULL; + struct smbcli_request *req2 = NULL; + struct smbcli_request *req4 = NULL; + + torture_assert(tctx, torture_setup_dir(cli, BASEDIR), + "Failed to setup up test directory: " BASEDIR); + + torture_comment(tctx, "Testing LOCKING_ANDX multi-lock 4\n"); + io.generic.level = RAW_LOCK_LOCKX; + + /* Create the test file. */ + fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE); + torture_assert(tctx,(fnum != -1), talloc_asprintf(tctx, + "Failed to create %s - %s\n", + fname, smbcli_errstr(cli->tree))); + + /* + * a) + * Lock regions 100->109, 120->129 as + * two separate write locks in one request. + */ + io.lockx.level = RAW_LOCK_LOCKX; + io.lockx.in.file.fnum = fnum; + io.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 2; + io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock[0].pid = cli->session->pid; + lock[0].offset = 100; + lock[0].count = 10; + lock[1].pid = cli->session->pid; + lock[1].offset = 120; + lock[1].count = 10; + io.lockx.in.locks = &lock[0]; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * b) + * Now request the same locks on a different + * context as blocking locks. But readonly. + */ + io.lockx.in.timeout = 20000; + io.lockx.in.mode = LOCKING_ANDX_SHARED_LOCK; + lock[0].pid = cli->session->pid+1; + lock[1].pid = cli->session->pid+1; + req = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * c) + * Request the first lock again on a separate context. + * Wait forever. The previous multi-lock request (b) + * should take precedence. Also readonly. + */ + io.lockx.in.timeout = UINT32_MAX; + lock[0].pid = cli->session->pid+2; + io.lockx.in.lock_cnt = 1; + req2 = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req2 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * d) + * Lock regions 110->119 + */ + io3.lockx.level = RAW_LOCK_LOCKX; + io3.lockx.in.file.fnum = fnum; + io3.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 0; + io3.lockx.in.lock_cnt = 1; + io3.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock3[0].pid = cli->session->pid+3; + lock3[0].offset = 110; + lock3[0].count = 10; + io3.lockx.in.locks = &lock3[0]; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * e) + * try 110-119 again + */ + io3.lockx.in.timeout = 20000; + lock3[0].pid = cli->session->pid+4; + req4 = smb_raw_lock_send(cli->tree, &io3); + torture_assert(tctx,(req4 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * f) + * Unlock (a) lock[0] 100-109 + */ + io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[0]; + lock[0].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * g) + * receive the successful blocked lock request (c) + * on 110-119 while (b) 100-109/120-129 is still waiting. + */ + status = smbcli_request_simple_recv(req2); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * h) + * try to lock lock[0] 100-109 again + * (read/write) + */ + lock[0].pid = cli->session->pid+5; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED); + + /* + * i) + * try to lock lock[0] 100-109 again with the pid (b) + * that's still waiting. + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + torture_assert(tctx, req4->state <= SMBCLI_REQUEST_RECV, + "req4 should still wait"); + + /* + * j) + * Unlock (d) lock[0] 110-119 + */ + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 1; + io3.lockx.in.lock_cnt = 0; + lock3[0].pid = cli->session->pid+3; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * k) + * receive the successful blocked + * lock request (e) on 110-119. + */ + status = smbcli_request_simple_recv(req4); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * l) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+6; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + /* + * m) + * try to lock lock[0] 100-109 again with the pid (b) + * that's still waiting + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + + /* Start the clock. */ + t = time_mono(NULL); + + /* + * n) + * Unlock (a) lock[1] 120-129 + */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[1]; + lock[1].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * o) + * receive the successful blocked lock request (b) + */ + status = smbcli_request_simple_recv(req); + CHECK_STATUS(status, NT_STATUS_OK); + + /* Fail if this took more than 2 seconds. */ + torture_assert(tctx,!(time_mono(NULL) > t+2), talloc_asprintf(tctx, + "Blocking locks were not granted immediately (%s)\n", + __location__)); +done: + smb_raw_exit(cli->session); + smbcli_deltree(cli->tree, BASEDIR); + return ret; +} + /* basic testing of lock calls */ @@ -2783,6 +3048,7 @@ struct torture_suite *torture_raw_lock(TALLOC_CTX *mem_ctx) torture_suite_add_1smb_test(suite, "multilock", test_multilock); torture_suite_add_1smb_test(suite, "multilock2", test_multilock2); torture_suite_add_1smb_test(suite, "multilock3", test_multilock3); + torture_suite_add_1smb_test(suite, "multilock4", test_multilock4); return suite; } -- 2.17.1 From f8ec8832b1d59b3de919d3e6338959320e642ce4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 16 Aug 2019 15:12:01 +0200 Subject: [PATCH 27/39] s4:torture/raw: add multilock5 test This is similar to multilock3, but uses a read-only (LOCKING_ANDX_SHARED_LOCK) locks for the first lock request. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 6d4296aca0c9a9287c0c78c8f8847a560bd2ea24) --- selftest/knownfail.d/multilock | 1 + source4/torture/raw/lock.c | 269 +++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+) diff --git a/selftest/knownfail.d/multilock b/selftest/knownfail.d/multilock index 01538acc5a88..b3fe93fd34ed 100644 --- a/selftest/knownfail.d/multilock +++ b/selftest/knownfail.d/multilock @@ -1,2 +1,3 @@ ^samba3.raw.lock.multilock3.*nt4_dc ^samba3.raw.lock.multilock4.*nt4_dc +^samba3.raw.lock.multilock5.*nt4_dc diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c index 26339f9b28b7..c29a577b34a6 100644 --- a/source4/torture/raw/lock.c +++ b/source4/torture/raw/lock.c @@ -3025,6 +3025,274 @@ done: return ret; } +/* + test multi5 Locking&X operation + This test is designed to show that + lock precedence on the server is based + on the order received, not on the ability + to grant. + + Compared to test_multilock3() (above) + this test demonstrates that the initial + lock request that block the following + exclusive locks can be a shared lock. + + For example: + + All locks except (a) are LOCKING_ANDX_EXCLUSIVE_LOCK (rw). + + (a) lock(ro) 100->109, 120->129 (granted) + (b) lock 100->109, 120->129 (blocks, timeout=20s) + (c) lock 100->109 (blocks, timeout=2s) + (d) lock 110->119 (granted) + (e) lock 110->119 (blocks, timeout=20s) + (f) unlock 100->109 (a) + (g) lock 100->109 (not granted, blocked by (b)) + (h) lock 100->109 (not granted, blocked by itself (b)) + (i) lock (c) will not be granted(conflict, times out) + as lock (b) will take precedence. + (j) unlock 110-119 (d) + (k) lock (e) completes and is not blocked by (a) nor (b) + (l) lock 100->109 (not granted(conflict), blocked by (b)) + (m) lock 100->109 (not granted(conflict), blocked by itself (b)) + (n) unlock 120-129 (a) + (o) lock (b) completes +*/ +static bool test_multilock5(struct torture_context *tctx, + struct smbcli_state *cli) +{ + union smb_lock io; + struct smb_lock_entry lock[2]; + union smb_lock io3; + struct smb_lock_entry lock3[1]; + NTSTATUS status; + bool ret = true; + int fnum; + const char *fname = BASEDIR "\\multilock5_test.txt"; + time_t t; + struct smbcli_request *req = NULL; + struct smbcli_request *req2 = NULL; + struct smbcli_request *req4 = NULL; + + torture_assert(tctx, torture_setup_dir(cli, BASEDIR), + "Failed to setup up test directory: " BASEDIR); + + torture_comment(tctx, "Testing LOCKING_ANDX multi-lock 5\n"); + io.generic.level = RAW_LOCK_LOCKX; + + /* Create the test file. */ + fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE); + torture_assert(tctx,(fnum != -1), talloc_asprintf(tctx, + "Failed to create %s - %s\n", + fname, smbcli_errstr(cli->tree))); + + /* + * a) + * Lock regions 100->109, 120->129 as + * two separate write locks in one request. + * (read only) + */ + io.lockx.level = RAW_LOCK_LOCKX; + io.lockx.in.file.fnum = fnum; + io.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 2; + io.lockx.in.mode = LOCKING_ANDX_SHARED_LOCK; + lock[0].pid = cli->session->pid; + lock[0].offset = 100; + lock[0].count = 10; + lock[1].pid = cli->session->pid; + lock[1].offset = 120; + lock[1].count = 10; + io.lockx.in.locks = &lock[0]; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * b) + * Now request the same locks on a different + * context as blocking locks. + * (read write) + */ + io.lockx.in.timeout = 20000; + io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock[0].pid = cli->session->pid+1; + lock[1].pid = cli->session->pid+1; + req = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * c) + * Request the first lock again on a separate context. + * Wait 2 seconds. This should time out (the previous + * multi-lock request should take precedence). + * (read write) + */ + io.lockx.in.timeout = 2000; + lock[0].pid = cli->session->pid+2; + io.lockx.in.lock_cnt = 1; + req2 = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req2 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * d) + * Lock regions 110->119 + */ + io3.lockx.level = RAW_LOCK_LOCKX; + io3.lockx.in.file.fnum = fnum; + io3.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 0; + io3.lockx.in.lock_cnt = 1; + io3.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock3[0].pid = cli->session->pid+3; + lock3[0].offset = 110; + lock3[0].count = 10; + io3.lockx.in.locks = &lock3[0]; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * e) + * try 110-119 again + */ + io3.lockx.in.timeout = 20000; + lock3[0].pid = cli->session->pid+4; + req4 = smb_raw_lock_send(cli->tree, &io3); + torture_assert(tctx,(req4 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * f) + * Unlock (a) lock[0] 100-109 + * + * Note we send LOCKING_ANDX_EXCLUSIVE_LOCK + * while the lock used LOCKING_ANDX_SHARED_LOCK + * to check if that also works. + */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[0]; + lock[0].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * g) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+5; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED); + + /* + * h) + * try to lock lock[0] 100-109 again with the pid (b) + * that's still waiting. + * (read write) + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req2->state <= SMBCLI_REQUEST_RECV, + "req2 should still wait"); + + /* + * i) + * Did the second lock complete (should time out) ? + */ + status = smbcli_request_simple_recv(req2); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + torture_assert(tctx, req4->state <= SMBCLI_REQUEST_RECV, + "req4 should still wait"); + + /* + * j) + * Unlock (d) lock[0] 110-119 + */ + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 1; + io3.lockx.in.lock_cnt = 0; + lock3[0].pid = cli->session->pid+3; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * k) + * receive the successful blocked lock requests + * on 110-119 while the 100-109/120-129 is still waiting. + */ + status = smbcli_request_simple_recv(req4); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * l) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+6; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + /* + * m) + * try to lock lock[0] 100-109 again with the pid (b) + * that's still waiting + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + + /* Start the clock. */ + t = time_mono(NULL); + + /* + * n) + * Unlock (a) lock[1] 120-129 + */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[1]; + lock[1].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * o) + * receive the successful blocked lock request (b) + */ + status = smbcli_request_simple_recv(req); + CHECK_STATUS(status, NT_STATUS_OK); + + /* Fail if this took more than 2 seconds. */ + torture_assert(tctx,!(time_mono(NULL) > t+2), talloc_asprintf(tctx, + "Blocking locks were not granted immediately (%s)\n", + __location__)); +done: + smb_raw_exit(cli->session); + smbcli_deltree(cli->tree, BASEDIR); + return ret; +} + /* basic testing of lock calls */ @@ -3049,6 +3317,7 @@ struct torture_suite *torture_raw_lock(TALLOC_CTX *mem_ctx) torture_suite_add_1smb_test(suite, "multilock2", test_multilock2); torture_suite_add_1smb_test(suite, "multilock3", test_multilock3); torture_suite_add_1smb_test(suite, "multilock4", test_multilock4); + torture_suite_add_1smb_test(suite, "multilock5", test_multilock5); return suite; } -- 2.17.1 From 5b2e5677f3f19e8626d14fcb2b937cd50e707477 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 16 Aug 2019 16:24:34 +0200 Subject: [PATCH 28/39] s4:torture/raw: add multilock6 test This is similar to multilock3, but uses a read-only (LOCKING_ANDX_SHARED_LOCK) locks for the 2nd lock request. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit d3bc0199697fd7d6e04479321ca644a227bc4ede) --- selftest/knownfail | 1 + selftest/knownfail.d/multilock | 1 + source4/torture/raw/lock.c | 263 +++++++++++++++++++++++++++++++++ 3 files changed, 265 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index ded80b12259e..7b54b77a7084 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -109,6 +109,7 @@ .*net.api.delshare.* # DelShare isn't implemented yet ^samba4.smb2.oplock.doc ^samba4.smb2.lock.valid-request +^samba4.raw.lock.multilock6.ad_dc_ntvfs ^samba4.ldap.python \(ad_dc_default\).Test add_ldif\(\) with BASE64 security descriptor input using WRONG domain SID\(.*\)$ ^samba4.raw.lock.*.async # bug 6960 ^samba4.raw.open.ntcreatex_supersede diff --git a/selftest/knownfail.d/multilock b/selftest/knownfail.d/multilock index b3fe93fd34ed..9fa497bd6432 100644 --- a/selftest/knownfail.d/multilock +++ b/selftest/knownfail.d/multilock @@ -1,3 +1,4 @@ ^samba3.raw.lock.multilock3.*nt4_dc ^samba3.raw.lock.multilock4.*nt4_dc ^samba3.raw.lock.multilock5.*nt4_dc +^samba3.raw.lock.multilock6.*nt4_dc diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c index c29a577b34a6..f684e923a49c 100644 --- a/source4/torture/raw/lock.c +++ b/source4/torture/raw/lock.c @@ -3293,6 +3293,268 @@ done: return ret; } +/* + test multi6 Locking&X operation + This test is designed to show that + lock precedence on the server is based + on the order received, not on the ability + to grant. + + Compared to test_multilock4() (above) + this test demonstrates the behavior if + only just the first blocking lock + being a shared lock. + + For example: + + All locks except (b) are LOCKING_ANDX_EXCLUSIVE_LOCK (rw). + + (a) lock 100->109, 120->129 (granted) + (b) lock(ro) 100->109, 120->129 (blocks, timeout=20s) + (c) lock 100->109 (blocks, timeout=2s) + (d) lock 110->119 (granted) + (e) lock 110->119 (blocks, timeout=20s) + (f) unlock 100->109 (a) + (g) lock 100->109 (not granted, blocked by (b)) + (h) lock 100->109 (not granted, blocked by itself (b)) + (i) lock (c) will not be granted(conflict, times out) + as lock (b) will take precedence. + (j) unlock 110-119 (d) + (k) lock (e) completes and is not blocked by (a) nor (b) + (l) lock 100->109 (not granted(conflict), blocked by (b)) + (m) lock 100->109 (not granted(conflict), blocked by itself (b)) + (n) unlock 120-129 (a) + (o) lock (b) completes +*/ +static bool test_multilock6(struct torture_context *tctx, + struct smbcli_state *cli) +{ + union smb_lock io; + struct smb_lock_entry lock[2]; + union smb_lock io3; + struct smb_lock_entry lock3[1]; + NTSTATUS status; + bool ret = true; + int fnum; + const char *fname = BASEDIR "\\multilock6_test.txt"; + time_t t; + struct smbcli_request *req = NULL; + struct smbcli_request *req2 = NULL; + struct smbcli_request *req4 = NULL; + + torture_assert(tctx, torture_setup_dir(cli, BASEDIR), + "Failed to setup up test directory: " BASEDIR); + + torture_comment(tctx, "Testing LOCKING_ANDX multi-lock 6\n"); + io.generic.level = RAW_LOCK_LOCKX; + + /* Create the test file. */ + fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE); + torture_assert(tctx,(fnum != -1), talloc_asprintf(tctx, + "Failed to create %s - %s\n", + fname, smbcli_errstr(cli->tree))); + + /* + * a) + * Lock regions 100->109, 120->129 as + * two separate write locks in one request. + */ + io.lockx.level = RAW_LOCK_LOCKX; + io.lockx.in.file.fnum = fnum; + io.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 2; + io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock[0].pid = cli->session->pid; + lock[0].offset = 100; + lock[0].count = 10; + lock[1].pid = cli->session->pid; + lock[1].offset = 120; + lock[1].count = 10; + io.lockx.in.locks = &lock[0]; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * b) + * Now request the same locks on a different + * context as blocking locks. + * (read only) + */ + io.lockx.in.timeout = 20000; + io.lockx.in.mode = LOCKING_ANDX_SHARED_LOCK; + lock[0].pid = cli->session->pid+1; + lock[1].pid = cli->session->pid+1; + req = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * c) + * Request the first lock again on a separate context. + * Wait 2 seconds. This should time out (the previous + * multi-lock request should take precedence). + */ + io.lockx.in.timeout = 2000; + io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock[0].pid = cli->session->pid+2; + io.lockx.in.lock_cnt = 1; + req2 = smb_raw_lock_send(cli->tree, &io); + torture_assert(tctx,(req2 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * d) + * Lock regions 110->119 + */ + io3.lockx.level = RAW_LOCK_LOCKX; + io3.lockx.in.file.fnum = fnum; + io3.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 0; + io3.lockx.in.lock_cnt = 1; + io3.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; + lock3[0].pid = cli->session->pid+3; + lock3[0].offset = 110; + lock3[0].count = 10; + io3.lockx.in.locks = &lock3[0]; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * e) + * try 110-119 again + */ + io3.lockx.in.timeout = 20000; + lock3[0].pid = cli->session->pid+4; + req4 = smb_raw_lock_send(cli->tree, &io3); + torture_assert(tctx,(req4 != NULL), talloc_asprintf(tctx, + "Failed to setup timed locks (%s)\n", __location__)); + + /* + * f) + * Unlock (a) lock[0] 100-109 + */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[0]; + lock[0].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * g) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+5; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED); + + /* + * h) + * try to lock lock[0] 100-109 again with the pid (b) + * that's still waiting + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req2->state <= SMBCLI_REQUEST_RECV, + "req2 should still wait"); + + /* + * i) + * Did the second lock (c) complete (should time out) ? + */ + status = smbcli_request_simple_recv(req2); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + torture_assert(tctx, req4->state <= SMBCLI_REQUEST_RECV, + "req4 should still wait"); + + /* + * j) + * Unlock (d) lock[0] 110-119 + */ + io3.lockx.in.timeout = 0; + io3.lockx.in.ulock_cnt = 1; + io3.lockx.in.lock_cnt = 0; + lock3[0].pid = cli->session->pid+3; + status = smb_raw_lock(cli->tree, &io3); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * k) + * receive the successful blocked lock request (e) + * on 110-119 while (b) 100-109/120-129 is still waiting. + */ + status = smbcli_request_simple_recv(req4); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * l) + * try to lock lock[0] 100-109 again + */ + lock[0].pid = cli->session->pid+6; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + /* + * m) + * try to lock lock[0] 100-109 again with the pid (b) + * that's still waiting + */ + lock[0].pid = cli->session->pid+1; + io.lockx.in.ulock_cnt = 0; + io.lockx.in.lock_cnt = 1; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT); + + torture_assert(tctx, req->state <= SMBCLI_REQUEST_RECV, + "req should still wait"); + + /* Start the clock. */ + t = time_mono(NULL); + + /* + * n) + * Unlock (a) lock[1] 120-129 + */ + io.lockx.in.timeout = 0; + io.lockx.in.ulock_cnt = 1; + io.lockx.in.lock_cnt = 0; + io.lockx.in.locks = &lock[1]; + lock[1].pid = cli->session->pid; + status = smb_raw_lock(cli->tree, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + /* + * o) + * receive the successful blocked lock request (b) + */ + status = smbcli_request_simple_recv(req); + CHECK_STATUS(status, NT_STATUS_OK); + + /* Fail if this took more than 2 seconds. */ + torture_assert(tctx,!(time_mono(NULL) > t+2), talloc_asprintf(tctx, + "Blocking locks were not granted immediately (%s)\n", + __location__)); +done: + smb_raw_exit(cli->session); + smbcli_deltree(cli->tree, BASEDIR); + return ret; +} + /* basic testing of lock calls */ @@ -3318,6 +3580,7 @@ struct torture_suite *torture_raw_lock(TALLOC_CTX *mem_ctx) torture_suite_add_1smb_test(suite, "multilock3", test_multilock3); torture_suite_add_1smb_test(suite, "multilock4", test_multilock4); torture_suite_add_1smb_test(suite, "multilock5", test_multilock5); + torture_suite_add_1smb_test(suite, "multilock6", test_multilock6); return suite; } -- 2.17.1 From b0e7eb703d2f99dd6e7bf4306479e4b61ec8fded Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 18:17:47 +0200 Subject: [PATCH 29/39] s3:blocking: use timeval_expired(&state->endtime) to stop processing This is less racy than timeval_elapsed() > 0 as the current time is already expired and timeout = 0 will always work correct. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 5a841a43f9c4f862e2d7235429363b3066cf5850) --- source3/smbd/blocking.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index a87d62d910aa..9f64a86d3eea 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -159,9 +159,10 @@ struct tevent_req *smbd_smb1_do_locks_send( struct share_mode_lock *lck = NULL; struct server_id blocking_pid = { 0 }; uint64_t blocking_smblctx = 0; - struct timeval endtime; + struct timeval endtime = { 0 }; NTSTATUS status = NT_STATUS_OK; bool ok; + bool expired; req = tevent_req_create( mem_ctx, &state, struct smbd_smb1_do_locks_state); @@ -251,19 +252,28 @@ struct tevent_req *smbd_smb1_do_locks_send( state->timeout = lp_lock_spin_time(); } } + state->endtime = timeval_current_ofs_msec(state->timeout); DBG_DEBUG("timeout=%"PRIu32", blocking_smblctx=%"PRIu64"\n", state->timeout, blocking_smblctx); /* + * The client specified timeout expired + * avoid further retries. + * + * Otherwise keep waiting either waiting + * for changes in locking.tdb or the polling + * mode timers waiting for posix locks. + * * If the endtime is not elapsed yet, * it means we'll retry after a timeout. * In that case we'll have to return * NT_STATUS_FILE_LOCK_CONFLICT * instead of NT_STATUS_LOCK_NOT_GRANTED. */ - if (state->timeout == 0) { + expired = timeval_expired(&state->endtime); + if (expired) { status = state->deny_status; tevent_req_nterror(req, status); goto done; @@ -278,7 +288,6 @@ struct tevent_req *smbd_smb1_do_locks_send( TALLOC_FREE(lck); tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); - state->endtime = timeval_current_ofs_msec(state->timeout); endtime = state->endtime; if (blocking_smblctx == UINT64_MAX) { @@ -363,13 +372,13 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) struct smbd_smb1_do_locks_state *retry_state = tevent_req_data( retry_req, struct smbd_smb1_do_locks_state); struct share_mode_lock *lck; - struct timeval endtime; + struct timeval endtime = { 0 }; struct server_id blocking_pid = { 0 }; uint64_t blocking_smblctx = 0; struct tevent_req *subreq = NULL; NTSTATUS status; bool ok; - double elapsed; + bool expired; lck = get_existing_share_mode_lock(state, fsp->file_id); if (tevent_req_nomem(lck, req)) { @@ -393,7 +402,7 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) } /* - * The client specified timeout elapsed + * The client specified timeout expired * avoid further retries. * * Otherwise keep waiting either waiting @@ -406,8 +415,8 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) * NT_STATUS_FILE_LOCK_CONFLICT * instead of NT_STATUS_LOCK_NOT_GRANTED. */ - elapsed = timeval_elapsed(&state->endtime); - if (elapsed > 0) { + expired = timeval_expired(&state->endtime); + if (expired) { status = state->deny_status; goto done; } -- 2.17.1 From 0598625863f07760740817644b2258cc9c8bae06 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 18:02:57 +0200 Subject: [PATCH 30/39] s3:blocking: split out smbd_smb1_do_locks_setup_timeout() This function can be called multiple times, but only the first time will setup the endtime. And the endtime is relative to the request time and not the current time. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 8da7c10a58292022ee57406db9a365de9ffaf5cf) --- source3/smbd/blocking.c | 98 ++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 35 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 9f64a86d3eea..98074c0c09ab 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -117,6 +117,68 @@ static void smbd_smb1_do_locks_retry(struct tevent_req *subreq); static void smbd_smb1_blocked_locks_cleanup( struct tevent_req *req, enum tevent_req_state req_state); +static void smbd_smb1_do_locks_setup_timeout( + struct smbd_smb1_do_locks_state *state, + const struct smbd_lock_element *blocker) +{ + struct files_struct *fsp = state->fsp; + + if (!timeval_is_zero(&state->endtime)) { + /* + * already done + */ + return; + } + + if ((state->timeout != 0) && (state->timeout != UINT32_MAX)) { + /* + * Windows internal resolution for blocking locks + * seems to be about 200ms... Don't wait for less than + * that. JRA. + */ + state->timeout = MAX(state->timeout, lp_lock_spin_time()); + } + + if (state->timeout != 0) { + goto set_endtime; + } + + if (blocker == NULL) { + goto set_endtime; + } + + if ((blocker->offset >= 0xEF000000) && + ((blocker->offset >> 63) == 0)) { + /* + * This must be an optimization of an ancient + * application bug... + */ + state->timeout = lp_lock_spin_time(); + } + + if ((fsp->lock_failure_seen) && + (blocker->offset == fsp->lock_failure_offset)) { + /* + * Delay repeated lock attempts on the same + * lock. Maybe a more advanced version of the + * above check? + */ + DBG_DEBUG("Delaying lock request due to previous " + "failure\n"); + state->timeout = lp_lock_spin_time(); + } + +set_endtime: + /* + * Note state->timeout might still 0, + * but that's ok, as we don't want to retry + * in that case. + */ + state->endtime = timeval_add(&state->smbreq->request_time, + state->timeout / 1000, + (state->timeout % 1000) * 1000); +} + static void smbd_smb1_do_locks_update_polling_msecs( struct smbd_smb1_do_locks_state *state) { @@ -196,15 +258,6 @@ struct tevent_req *smbd_smb1_do_locks_send( return tevent_req_post(req, ev); } - if ((state->timeout != 0) && (state->timeout != UINT32_MAX)) { - /* - * Windows internal resolution for blocking locks - * seems to be about 200ms... Don't wait for less than - * that. JRA. - */ - state->timeout = MAX(state->timeout, lp_lock_spin_time()); - } - lck = get_existing_share_mode_lock(state, state->fsp->file_id); if (tevent_req_nomem(lck, req)) { DBG_DEBUG("Could not get share mode lock\n"); @@ -228,32 +281,7 @@ struct tevent_req *smbd_smb1_do_locks_send( goto done; } - if (state->timeout == 0) { - struct smbd_lock_element *blocker = &locks[state->blocker]; - - if ((blocker->offset >= 0xEF000000) && - ((blocker->offset >> 63) == 0)) { - /* - * This must be an optimization of an ancient - * application bug... - */ - state->timeout = lp_lock_spin_time(); - } - - if ((fsp->lock_failure_seen) && - (blocker->offset == fsp->lock_failure_offset)) { - /* - * Delay repeated lock attempts on the same - * lock. Maybe a more advanced version of the - * above check? - */ - DBG_DEBUG("Delaying lock request due to previous " - "failure\n"); - state->timeout = lp_lock_spin_time(); - } - } - state->endtime = timeval_current_ofs_msec(state->timeout); - + smbd_smb1_do_locks_setup_timeout(state, &locks[state->blocker]); DBG_DEBUG("timeout=%"PRIu32", blocking_smblctx=%"PRIu64"\n", state->timeout, blocking_smblctx); -- 2.17.1 From 60c542d72203f8f5445ac09e4c6d0e7919326c0d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 15:21:50 +0200 Subject: [PATCH 31/39] s3:blocking: do the timeout calculation before calling dbwrap_watched_watch_send() This makes the next commits easier to understand. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 997548a5f1a14d82f1e80cce6d9ee55e85b5107c) --- source3/smbd/blocking.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 98074c0c09ab..ac90f8c3ef16 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -308,14 +308,6 @@ struct tevent_req *smbd_smb1_do_locks_send( } state->deny_status = NT_STATUS_FILE_LOCK_CONFLICT; - subreq = dbwrap_watched_watch_send( - state, state->ev, lck->data->record, blocking_pid); - if (tevent_req_nomem(subreq, req)) { - goto done; - } - TALLOC_FREE(lck); - tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); - endtime = state->endtime; if (blocking_smblctx == UINT64_MAX) { @@ -330,6 +322,14 @@ struct tevent_req *smbd_smb1_do_locks_send( endtime = timeval_min(&endtime, &tmp); } + subreq = dbwrap_watched_watch_send( + state, state->ev, lck->data->record, blocking_pid); + if (tevent_req_nomem(subreq, req)) { + goto done; + } + TALLOC_FREE(lck); + tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); + ok = tevent_req_set_endtime(subreq, state->ev, endtime); if (!ok) { tevent_req_oom(req); @@ -450,14 +450,6 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) } state->deny_status = NT_STATUS_FILE_LOCK_CONFLICT; - subreq = dbwrap_watched_watch_send( - state, state->ev, lck->data->record, blocking_pid); - if (tevent_req_nomem(subreq, req)) { - goto done; - } - TALLOC_FREE(lck); - tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); - endtime = state->endtime; if (blocking_smblctx == UINT64_MAX) { @@ -472,6 +464,14 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) endtime = timeval_min(&endtime, &tmp); } + subreq = dbwrap_watched_watch_send( + state, state->ev, lck->data->record, blocking_pid); + if (tevent_req_nomem(subreq, req)) { + goto done; + } + TALLOC_FREE(lck); + tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); + ok = tevent_req_set_endtime(subreq, state->ev, endtime); if (!ok) { status = NT_STATUS_NO_MEMORY; -- 2.17.1 From eb6d78aa69cb119696feab228926b67adb72e818 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 16 Aug 2019 14:55:13 +0200 Subject: [PATCH 32/39] s3:blocking: fix the fsp->blocked_smb1_lock_reqs handling A new request is first checks against all pending requests before checking the already granted locks. Before we retried the lock array of another request (the first in the list), but then finished current request, which is wrong. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 312327106271abafeb53e62dfb71a38bf93e2d41) --- selftest/knownfail.d/multilock | 4 - source3/smbd/blocking.c | 134 ++++++++++++++++++++++++++++++--- 2 files changed, 125 insertions(+), 13 deletions(-) delete mode 100644 selftest/knownfail.d/multilock diff --git a/selftest/knownfail.d/multilock b/selftest/knownfail.d/multilock deleted file mode 100644 index 9fa497bd6432..000000000000 --- a/selftest/knownfail.d/multilock +++ /dev/null @@ -1,4 +0,0 @@ -^samba3.raw.lock.multilock3.*nt4_dc -^samba3.raw.lock.multilock4.*nt4_dc -^samba3.raw.lock.multilock5.*nt4_dc -^samba3.raw.lock.multilock6.*nt4_dc diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index ac90f8c3ef16..39042d2f46d6 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -116,6 +116,14 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req); static void smbd_smb1_do_locks_retry(struct tevent_req *subreq); static void smbd_smb1_blocked_locks_cleanup( struct tevent_req *req, enum tevent_req_state req_state); +static NTSTATUS smbd_smb1_do_locks_check( + struct files_struct *fsp, + enum brl_flavour lock_flav, + uint16_t num_locks, + struct smbd_lock_element *locks, + uint16_t *blocker_idx, + struct server_id *blocking_pid, + uint64_t *blocking_smblctx); static void smbd_smb1_do_locks_setup_timeout( struct smbd_smb1_do_locks_state *state, @@ -264,7 +272,7 @@ struct tevent_req *smbd_smb1_do_locks_send( return tevent_req_post(req, ev); } - status = smbd_do_locks_try( + status = smbd_smb1_do_locks_check( state->fsp, state->lock_flav, state->num_locks, @@ -390,15 +398,123 @@ static void smbd_smb1_blocked_locks_cleanup( fsp, blocked, struct tevent_req *, num_blocked-1); } +static NTSTATUS smbd_smb1_do_locks_check_blocked( + uint16_t num_blocked, + struct smbd_lock_element *blocked, + uint16_t num_locks, + struct smbd_lock_element *locks, + uint16_t *blocker_idx, + uint64_t *blocking_smblctx) +{ + uint16_t li; + + for (li=0; li < num_locks; li++) { + struct smbd_lock_element *l = &locks[li]; + uint16_t bi; + bool valid; + + valid = byte_range_valid(l->offset, l->count); + if (!valid) { + return NT_STATUS_INVALID_LOCK_RANGE; + } + + for (bi = 0; bi < num_blocked; bi++) { + struct smbd_lock_element *b = &blocked[li]; + bool overlap; + + /* Read locks never conflict. */ + if (l->brltype == READ_LOCK && b->brltype == READ_LOCK) { + continue; + } + + overlap = byte_range_overlap(l->offset, + l->count, + b->offset, + b->count); + if (!overlap) { + continue; + } + + *blocker_idx = li; + *blocking_smblctx = b->smblctx; + return NT_STATUS_LOCK_NOT_GRANTED; + } + } + + return NT_STATUS_OK; +} + +static NTSTATUS smbd_smb1_do_locks_check( + struct files_struct *fsp, + enum brl_flavour lock_flav, + uint16_t num_locks, + struct smbd_lock_element *locks, + uint16_t *blocker_idx, + struct server_id *blocking_pid, + uint64_t *blocking_smblctx) +{ + struct tevent_req **blocked = fsp->blocked_smb1_lock_reqs; + size_t num_blocked = talloc_array_length(blocked); + NTSTATUS status; + size_t bi; + + /* + * We check the pending/blocked requests + * from the oldest to the youngest request. + * + * Note due to the retry logic the current request + * might already be in the list. + */ + + for (bi = 0; bi < num_blocked; bi++) { + struct smbd_smb1_do_locks_state *blocked_state = + tevent_req_data(blocked[bi], + struct smbd_smb1_do_locks_state); + + if (blocked_state->locks == locks) { + SMB_ASSERT(blocked_state->num_locks == num_locks); + SMB_ASSERT(blocked_state->lock_flav == lock_flav); + + /* + * We found ourself... + */ + break; + } + + status = smbd_smb1_do_locks_check_blocked( + blocked_state->num_locks, + blocked_state->locks, + num_locks, + locks, + blocker_idx, + blocking_smblctx); + if (!NT_STATUS_IS_OK(status)) { + *blocking_pid = messaging_server_id( + fsp->conn->sconn->msg_ctx); + return status; + } + } + + status = smbd_do_locks_try( + fsp, + lock_flav, + num_locks, + locks, + blocker_idx, + blocking_pid, + blocking_smblctx); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + return NT_STATUS_OK; +} + static void smbd_smb1_do_locks_try(struct tevent_req *req) { struct smbd_smb1_do_locks_state *state = tevent_req_data( req, struct smbd_smb1_do_locks_state); struct files_struct *fsp = state->fsp; - struct tevent_req **blocked = fsp->blocked_smb1_lock_reqs; - struct tevent_req *retry_req = blocked[0]; - struct smbd_smb1_do_locks_state *retry_state = tevent_req_data( - retry_req, struct smbd_smb1_do_locks_state); struct share_mode_lock *lck; struct timeval endtime = { 0 }; struct server_id blocking_pid = { 0 }; @@ -414,11 +530,11 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) return; } - status = smbd_do_locks_try( + status = smbd_smb1_do_locks_check( fsp, - retry_state->lock_flav, - retry_state->num_locks, - retry_state->locks, + state->lock_flav, + state->num_locks, + state->locks, &state->blocker, &blocking_pid, &blocking_smblctx); -- 2.17.1 From 1b302b637c3464b74645e7611a832d26afaa9a8c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 18:18:50 +0200 Subject: [PATCH 33/39] s3:blocking: call smbd_smb1_do_locks_setup_timeout() also in smbd_smb1_do_locks_try() This is a noop if smbd_smb1_do_locks_setup_timeout() was called before. But it allows us to use smbd_smb1_do_locks_try() in smbd_smb1_do_locks_send() in a following commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 6e30a89b3f00ad55391454fbaa1272074e1962f0) --- source3/smbd/blocking.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 39042d2f46d6..77dfc5a3d446 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -545,6 +545,11 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) goto done; } + smbd_smb1_do_locks_setup_timeout(state, &state->locks[state->blocker]); + DBG_DEBUG("timeout=%"PRIu32", blocking_smblctx=%"PRIu64"\n", + state->timeout, + blocking_smblctx); + /* * The client specified timeout expired * avoid further retries. -- 2.17.1 From 8bf4f99928a5a557ddcabfeffc36cb503de666d2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 15 Aug 2019 20:09:55 +0200 Subject: [PATCH 34/39] s3:blocking: make use of smbd_smb1_do_locks_try() in smbd_smb1_do_locks_send() We only need the logic to call smbd_smb1_do_locks_check() and a possible retry once in smbd_smb1_do_locks_try(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 8975673e3c3f9f7dbdb7ba7562bb81a62cd24e2e) --- source3/smbd/blocking.c | 90 ++--------------------------------------- 1 file changed, 4 insertions(+), 86 deletions(-) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 77dfc5a3d446..514985bcd756 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -224,15 +224,9 @@ struct tevent_req *smbd_smb1_do_locks_send( uint16_t num_locks, struct smbd_lock_element *locks) { - struct tevent_req *req = NULL, *subreq = NULL; + struct tevent_req *req = NULL; struct smbd_smb1_do_locks_state *state = NULL; - struct share_mode_lock *lck = NULL; - struct server_id blocking_pid = { 0 }; - uint64_t blocking_smblctx = 0; - struct timeval endtime = { 0 }; - NTSTATUS status = NT_STATUS_OK; bool ok; - bool expired; req = tevent_req_create( mem_ctx, &state, struct smbd_smb1_do_locks_state); @@ -266,94 +260,18 @@ struct tevent_req *smbd_smb1_do_locks_send( return tevent_req_post(req, ev); } - lck = get_existing_share_mode_lock(state, state->fsp->file_id); - if (tevent_req_nomem(lck, req)) { - DBG_DEBUG("Could not get share mode lock\n"); + smbd_smb1_do_locks_try(req); + if (!tevent_req_is_in_progress(req)) { return tevent_req_post(req, ev); } - status = smbd_smb1_do_locks_check( - state->fsp, - state->lock_flav, - state->num_locks, - state->locks, - &state->blocker, - &blocking_pid, - &blocking_smblctx); - if (NT_STATUS_IS_OK(status)) { - tevent_req_done(req); - goto done; - } - if (!ERROR_WAS_LOCK_DENIED(status)) { - tevent_req_nterror(req, status); - goto done; - } - - smbd_smb1_do_locks_setup_timeout(state, &locks[state->blocker]); - DBG_DEBUG("timeout=%"PRIu32", blocking_smblctx=%"PRIu64"\n", - state->timeout, - blocking_smblctx); - - /* - * The client specified timeout expired - * avoid further retries. - * - * Otherwise keep waiting either waiting - * for changes in locking.tdb or the polling - * mode timers waiting for posix locks. - * - * If the endtime is not elapsed yet, - * it means we'll retry after a timeout. - * In that case we'll have to return - * NT_STATUS_FILE_LOCK_CONFLICT - * instead of NT_STATUS_LOCK_NOT_GRANTED. - */ - expired = timeval_expired(&state->endtime); - if (expired) { - status = state->deny_status; - tevent_req_nterror(req, status); - goto done; - } - state->deny_status = NT_STATUS_FILE_LOCK_CONFLICT; - - endtime = state->endtime; - - if (blocking_smblctx == UINT64_MAX) { - struct timeval tmp; - - smbd_smb1_do_locks_update_polling_msecs(state); - - DBG_DEBUG("Blocked on a posix lock. Retry in %"PRIu32" msecs\n", - state->polling_msecs); - - tmp = timeval_current_ofs_msec(state->polling_msecs); - endtime = timeval_min(&endtime, &tmp); - } - - subreq = dbwrap_watched_watch_send( - state, state->ev, lck->data->record, blocking_pid); - if (tevent_req_nomem(subreq, req)) { - goto done; - } - TALLOC_FREE(lck); - tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); - - ok = tevent_req_set_endtime(subreq, state->ev, endtime); - if (!ok) { - tevent_req_oom(req); - goto done; - } - ok = smbd_smb1_fsp_add_blocked_lock_req(fsp, req); if (!ok) { tevent_req_oom(req); - goto done; + return tevent_req_post(req, ev); } tevent_req_set_cleanup_fn(req, smbd_smb1_blocked_locks_cleanup); return req; -done: - TALLOC_FREE(lck); - return tevent_req_post(req, ev); } static void smbd_smb1_blocked_locks_cleanup( -- 2.17.1 From 58b7385989777f73d6a42439635d455fccf6bf17 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 15:29:32 +0200 Subject: [PATCH 35/39] s3:blocking: handle NT_STATUS_RETRY from the VFS backend This allows the VFS backends to implement async byte range locking. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 7d1cd6f22e7e3d95aba04c45776057945c2a5e30) --- source3/smbd/blocking.c | 84 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 514985bcd756..fbd1ea812f38 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -103,6 +103,7 @@ struct smbd_smb1_do_locks_state { struct files_struct *fsp; uint32_t timeout; uint32_t polling_msecs; + uint32_t retry_msecs; struct timeval endtime; bool large_offset; /* required for correct cancel */ enum brl_flavour lock_flav; @@ -187,6 +188,33 @@ set_endtime: (state->timeout % 1000) * 1000); } +static void smbd_smb1_do_locks_update_retry_msecs( + struct smbd_smb1_do_locks_state *state) +{ + /* + * The default lp_lock_spin_time() is 200ms, + * we just use half of it to trigger the first retry. + * + * v_min is in the range of 0.001 to 10 secs + * (0.1 secs by default) + * + * v_max is in the range of 0.01 to 100 secs + * (1.0 secs by default) + * + * The typical steps are: + * 0.1, 0.2, 0.3, 0.4, ... 1.0 + */ + uint32_t v_min = MAX(2, MIN(20000, lp_lock_spin_time()))/2; + uint32_t v_max = 10 * v_min; + + if (state->retry_msecs >= v_max) { + state->retry_msecs = v_max; + return; + } + + state->retry_msecs += v_min; +} + static void smbd_smb1_do_locks_update_polling_msecs( struct smbd_smb1_do_locks_state *state) { @@ -459,9 +487,60 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) if (NT_STATUS_IS_OK(status)) { goto done; } + if (NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) { + /* + * We got NT_STATUS_RETRY, + * we reset polling_msecs so that + * that the retries based on LOCK_NOT_GRANTED + * will later start with small intervalls again. + */ + state->polling_msecs = 0; + + /* + * The backend wasn't able to decide yet. + * We need to wait even for non-blocking + * locks. + * + * The backend uses blocking_smblctx == UINT64_MAX + * to indicate that we should use retry timers. + * + * It uses blocking_smblctx == 0 to indicate + * it will use share_mode_wakeup_waiters() + * to wake us. Note that unrelated changes in + * locking.tdb may cause retries. + */ + + if (blocking_smblctx != UINT64_MAX) { + SMB_ASSERT(blocking_smblctx == 0); + goto setup_retry; + } + + smbd_smb1_do_locks_update_retry_msecs(state); + + DBG_DEBUG("Waiting for a backend decision. " + "Retry in %"PRIu32" msecs\n", + state->retry_msecs); + + /* + * We completely ignore state->endtime here + * we we'll wait for a backend decision forever. + * If the backend is smart enough to implement + * some NT_STATUS_RETRY logic, it has to + * switch to any other status after in order + * to avoid waiting forever. + */ + endtime = timeval_current_ofs_msec(state->retry_msecs); + goto setup_retry; + } if (!ERROR_WAS_LOCK_DENIED(status)) { goto done; } + /* + * We got LOCK_NOT_GRANTED, make sure + * a following STATUS_RETRY will start + * with short intervalls again. + */ + state->retry_msecs = 0; smbd_smb1_do_locks_setup_timeout(state, &state->locks[state->blocker]); DBG_DEBUG("timeout=%"PRIu32", blocking_smblctx=%"PRIu64"\n", @@ -503,6 +582,7 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) endtime = timeval_min(&endtime, &tmp); } +setup_retry: subreq = dbwrap_watched_watch_send( state, state->ev, lck->data->record, blocking_pid); if (tevent_req_nomem(subreq, req)) { @@ -511,6 +591,10 @@ static void smbd_smb1_do_locks_try(struct tevent_req *req) TALLOC_FREE(lck); tevent_req_set_callback(subreq, smbd_smb1_do_locks_retry, req); + if (timeval_is_zero(&endtime)) { + return; + } + ok = tevent_req_set_endtime(subreq, state->ev, endtime); if (!ok) { status = NT_STATUS_NO_MEMORY; -- 2.17.1 From c605236657d96ce7914b7dc1a76cc0dafabecf5d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 16:25:59 +0200 Subject: [PATCH 36/39] s3:smb2_lock: handle NT_STATUS_RETRY from the VFS backend This allows the VFS backends to implement async byte range locking. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 7471b0f63276e707784c98b832992ff08b1898ef) --- source3/smbd/smb2_lock.c | 80 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index 8ba54fe69953..26de8b521ede 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -44,6 +44,7 @@ struct smbd_smb2_lock_state { struct files_struct *fsp; bool blocking; uint32_t polling_msecs; + uint32_t retry_msecs; uint16_t lock_count; struct smbd_lock_element *locks; }; @@ -372,6 +373,33 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, return req; } +static void smbd_smb2_lock_update_retry_msecs( + struct smbd_smb2_lock_state *state) +{ + /* + * The default lp_lock_spin_time() is 200ms, + * we just use half of it to trigger the first retry. + * + * v_min is in the range of 0.001 to 10 secs + * (0.1 secs by default) + * + * v_max is in the range of 0.01 to 100 secs + * (1.0 secs by default) + * + * The typical steps are: + * 0.1, 0.2, 0.3, 0.4, ... 1.0 + */ + uint32_t v_min = MAX(2, MIN(20000, lp_lock_spin_time()))/2; + uint32_t v_max = 10 * v_min; + + if (state->retry_msecs >= v_max) { + state->retry_msecs = v_max; + return; + } + + state->retry_msecs += v_min; +} + static void smbd_smb2_lock_update_polling_msecs( struct smbd_smb2_lock_state *state) { @@ -429,6 +457,51 @@ static void smbd_smb2_lock_try(struct tevent_req *req) tevent_req_done(req); return; } + if (NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) { + /* + * We got NT_STATUS_RETRY, + * we reset polling_msecs so that + * that the retries based on LOCK_NOT_GRANTED + * will later start with small intervalls again. + */ + state->polling_msecs = 0; + + /* + * The backend wasn't able to decide yet. + * We need to wait even for non-blocking + * locks. + * + * The backend uses blocking_smblctx == UINT64_MAX + * to indicate that we should use retry timers. + * + * It uses blocking_smblctx == 0 to indicate + * it will use share_mode_wakeup_waiters() + * to wake us. Note that unrelated changes in + * locking.tdb may cause retries. + */ + + if (blocking_smblctx != UINT64_MAX) { + SMB_ASSERT(blocking_smblctx == 0); + goto setup_retry; + } + + smbd_smb2_lock_update_retry_msecs(state); + + DBG_DEBUG("Waiting for a backend decision. " + "Retry in %"PRIu32" msecs\n", + state->retry_msecs); + + /* + * We completely ignore state->endtime here + * we we'll wait for a backend decision forever. + * If the backend is smart enough to implement + * some NT_STATUS_RETRY logic, it has to + * switch to any other status after in order + * to avoid waiting forever. + */ + endtime = timeval_current_ofs_msec(state->retry_msecs); + goto setup_retry; + } if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT)) { /* * This is a bug and will be changed into an assert @@ -447,6 +520,12 @@ static void smbd_smb2_lock_try(struct tevent_req *req) tevent_req_nterror(req, status); return; } + /* + * We got LOCK_NOT_GRANTED, make sure + * a following STATUS_RETRY will start + * with short intervalls again. + */ + state->retry_msecs = 0; if (!state->blocking) { TALLOC_FREE(lck); @@ -463,6 +542,7 @@ static void smbd_smb2_lock_try(struct tevent_req *req) endtime = timeval_current_ofs_msec(state->polling_msecs); } +setup_retry: DBG_DEBUG("Watching share mode lock\n"); subreq = dbwrap_watched_watch_send( -- 2.17.1 From 6988d2a94ed6b93c50aed68cafa9b0f8cb68f629 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 8 Aug 2019 19:26:28 +0200 Subject: [PATCH 37/39] s3:locking: add brl_req_guid() and brl_req_mem_ctx() helper functions This allows the vfs backend to detect a retry and keep state between the retries. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 66d92f37c3a643d97489a59bb6d1e75e91528c20) --- source3/include/locking.h | 2 ++ source3/locking/brlock.c | 45 +++++++++++++++++++++++++++++++++---- source3/locking/locking.c | 11 ++++++++- source3/locking/proto.h | 8 +++++++ source3/modules/vfs_fruit.c | 13 +++++++++++ source3/smbd/blocking.c | 2 ++ source3/smbd/globals.c | 18 +++++++++++++++ source3/smbd/globals.h | 2 ++ source3/smbd/reply.c | 7 ++++++ source3/smbd/smb2_lock.c | 1 + source3/smbd/trans2.c | 2 ++ 11 files changed, 106 insertions(+), 5 deletions(-) diff --git a/source3/include/locking.h b/source3/include/locking.h index 3e7560bef9ec..0175db2dd473 100644 --- a/source3/include/locking.h +++ b/source3/include/locking.h @@ -30,6 +30,7 @@ enum brl_type {READ_LOCK, WRITE_LOCK, UNLOCK_LOCK}; enum brl_flavour {WINDOWS_LOCK = 0, POSIX_LOCK = 1}; #include "librpc/gen_ndr/server_id.h" +#include "librpc/gen_ndr/misc.h" /* This contains elements that differentiate locks. The smbpid is a client supplied pid, and is essentially the locking context for @@ -62,6 +63,7 @@ struct lock_struct { }; struct smbd_lock_element { + struct GUID req_guid; uint64_t smblctx; enum brl_type brltype; uint64_t offset; diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c index 0a85bd0b057b..f22580164f96 100644 --- a/source3/locking/brlock.c +++ b/source3/locking/brlock.c @@ -46,6 +46,8 @@ static struct db_context *brlock_db; struct byte_range_lock { struct files_struct *fsp; + TALLOC_CTX *req_mem_ctx; + const struct GUID *req_guid; unsigned int num_locks; bool modified; struct lock_struct *lock_data; @@ -84,6 +86,25 @@ struct files_struct *brl_fsp(struct byte_range_lock *brl) return brl->fsp; } +TALLOC_CTX *brl_req_mem_ctx(const struct byte_range_lock *brl) +{ + if (brl->req_mem_ctx == NULL) { + return talloc_get_type_abort(brl, struct byte_range_lock); + } + + return brl->req_mem_ctx; +} + +const struct GUID *brl_req_guid(const struct byte_range_lock *brl) +{ + if (brl->req_guid == NULL) { + static const struct GUID brl_zero_req_guid; + return &brl_zero_req_guid; + } + + return brl->req_guid; +} + /**************************************************************************** See if two locking contexts are equal. ****************************************************************************/ @@ -1823,6 +1844,25 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp) return br_lck; } +struct byte_range_lock *brl_get_locks_for_locking(TALLOC_CTX *mem_ctx, + files_struct *fsp, + TALLOC_CTX *req_mem_ctx, + const struct GUID *req_guid) +{ + struct byte_range_lock *br_lck = NULL; + + br_lck = brl_get_locks(mem_ctx, fsp); + if (br_lck == NULL) { + return NULL; + } + SMB_ASSERT(req_mem_ctx != NULL); + br_lck->req_mem_ctx = req_mem_ctx; + SMB_ASSERT(req_guid != NULL); + br_lck->req_guid = req_guid; + + return br_lck; +} + struct brl_get_locks_readonly_state { TALLOC_CTX *mem_ctx; struct byte_range_lock **br_lock; @@ -1884,14 +1924,11 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp) /* * No locks on this file. Return an empty br_lock. */ - br_lock = talloc(fsp, struct byte_range_lock); + br_lock = talloc_zero(fsp, struct byte_range_lock); if (br_lock == NULL) { return NULL; } - br_lock->num_locks = 0; - br_lock->lock_data = NULL; - } else if (!NT_STATUS_IS_OK(status)) { DEBUG(3, ("Could not parse byte range lock record: " "%s\n", nt_errstr(status))); diff --git a/source3/locking/locking.c b/source3/locking/locking.c index d87a882d14fb..8fa1237d6ad8 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -232,6 +232,8 @@ static void decrement_current_lock_count(files_struct *fsp, struct do_lock_state { struct files_struct *fsp; + TALLOC_CTX *req_mem_ctx; + const struct GUID *req_guid; uint64_t smblctx; uint64_t count; uint64_t offset; @@ -251,7 +253,10 @@ static void do_lock_fn( struct do_lock_state *state = private_data; struct byte_range_lock *br_lck = NULL; - br_lck = brl_get_locks(talloc_tos(), state->fsp); + br_lck = brl_get_locks_for_locking(talloc_tos(), + state->fsp, + state->req_mem_ctx, + state->req_guid); if (br_lck == NULL) { state->status = NT_STATUS_NO_MEMORY; return; @@ -272,6 +277,8 @@ static void do_lock_fn( } NTSTATUS do_lock(files_struct *fsp, + TALLOC_CTX *req_mem_ctx, + const struct GUID *req_guid, uint64_t smblctx, uint64_t count, uint64_t offset, @@ -282,6 +289,8 @@ NTSTATUS do_lock(files_struct *fsp, { struct do_lock_state state = { .fsp = fsp, + .req_mem_ctx = req_mem_ctx, + .req_guid = req_guid, .smblctx = smblctx, .count = count, .offset = offset, diff --git a/source3/locking/proto.h b/source3/locking/proto.h index 7cb8bf3e3c99..7cf681561bc0 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -30,6 +30,8 @@ void brl_shutdown(void); unsigned int brl_num_locks(const struct byte_range_lock *brl); struct files_struct *brl_fsp(struct byte_range_lock *brl); +TALLOC_CTX *brl_req_mem_ctx(const struct byte_range_lock *brl); +const struct GUID *brl_req_guid(const struct byte_range_lock *brl); bool byte_range_valid(uint64_t ofs, uint64_t len); bool byte_range_overlap(uint64_t ofs1, @@ -76,6 +78,10 @@ int brl_forall(void (*fn)(struct file_id id, struct server_id pid, br_off start, br_off size, void *private_data), void *private_data); +struct byte_range_lock *brl_get_locks_for_locking(TALLOC_CTX *mem_ctx, + files_struct *fsp, + TALLOC_CTX *req_mem_ctx, + const struct GUID *req_guid); struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp); struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp); @@ -100,6 +106,8 @@ NTSTATUS query_lock(files_struct *fsp, enum brl_type *plock_type, enum brl_flavour lock_flav); NTSTATUS do_lock(files_struct *fsp, + TALLOC_CTX *req_mem_ctx, + const struct GUID *req_guid, uint64_t smblctx, uint64_t count, uint64_t offset, diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 0be7f20e9af1..85c7af21d580 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -2621,6 +2621,7 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle, bool netatalk_already_open_for_writing = false; bool netatalk_already_open_with_deny_read = false; bool netatalk_already_open_with_deny_write = false; + struct GUID req_guid = GUID_random(); /* FIXME: hardcoded data fork, add resource fork */ enum apple_fork fork_type = APPLE_FORK_DATA; @@ -2684,8 +2685,11 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle, /* Set NetAtalk locks matching our access */ if (access_mask & FILE_READ_DATA) { off = access_to_netatalk_brl(fork_type, FILE_READ_DATA); + req_guid.time_hi_and_version = __LINE__; status = do_lock( fsp, + talloc_tos(), + &req_guid, fsp->op->global->open_persistent_id, 1, off, @@ -2701,8 +2705,11 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle, if (!share_for_read) { off = denymode_to_netatalk_brl(fork_type, DENY_READ); + req_guid.time_hi_and_version = __LINE__; status = do_lock( fsp, + talloc_tos(), + &req_guid, fsp->op->global->open_persistent_id, 1, off, @@ -2718,8 +2725,11 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle, if (access_mask & FILE_WRITE_DATA) { off = access_to_netatalk_brl(fork_type, FILE_WRITE_DATA); + req_guid.time_hi_and_version = __LINE__; status = do_lock( fsp, + talloc_tos(), + &req_guid, fsp->op->global->open_persistent_id, 1, off, @@ -2735,8 +2745,11 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle, if (!share_for_write) { off = denymode_to_netatalk_brl(fork_type, DENY_WRITE); + req_guid.time_hi_and_version = __LINE__; status = do_lock( fsp, + talloc_tos(), + &req_guid, fsp->op->global->open_persistent_id, 1, off, diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index fbd1ea812f38..94e75a9b4058 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -45,6 +45,8 @@ NTSTATUS smbd_do_locks_try( status = do_lock( fsp, + locks, /* req_mem_ctx */ + &e->req_guid, e->smblctx, e->count, e->offset, diff --git a/source3/smbd/globals.c b/source3/smbd/globals.c index 6bc448b901d6..0cdce20d1228 100644 --- a/source3/smbd/globals.c +++ b/source3/smbd/globals.c @@ -109,3 +109,21 @@ void smbd_init_globals(void) ZERO_STRUCT(sec_ctx_stack); } + +struct GUID smbd_request_guid(struct smb_request *smb1req, uint16_t idx) +{ + struct GUID v = { + .time_low = (uint32_t)smb1req->mid, + .time_hi_and_version = idx, + }; + + if (smb1req->smb2req != NULL) { + v.time_mid = (uint16_t)smb1req->smb2req->current_idx; + } else { + v.time_mid = (uint16_t)(uintptr_t)smb1req->vwv; + } + + SBVAL((uint8_t *)&v, 8, (uintptr_t)smb1req->xconn); + + return v; +} diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 03d50882d163..47916ba29a18 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -115,6 +115,8 @@ DATA_BLOB negprot_spnego(TALLOC_CTX *ctx, struct smbXsrv_connection *xconn); void smbd_lock_socket(struct smbXsrv_connection *xconn); void smbd_unlock_socket(struct smbXsrv_connection *xconn); +struct GUID smbd_request_guid(struct smb_request *smb1req, uint16_t idx); + NTSTATUS smbd_do_unlocking(struct smb_request *req, files_struct *fsp, uint16_t num_ulocks, diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 2622681a2da0..dec67a10caeb 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3842,6 +3842,7 @@ void reply_lockread(struct smb_request *req) */ *lck = (struct smbd_lock_element) { + .req_guid = smbd_request_guid(req, 0), .smblctx = req->smbpid, .brltype = WRITE_LOCK, .count = SVAL(req->vwv+1, 0), @@ -4869,6 +4870,7 @@ void reply_writeunlock(struct smb_request *req) if (numtowrite && !fsp->print_file) { struct smbd_lock_element l = { + .req_guid = smbd_request_guid(req, 0), .smblctx = req->smbpid, .brltype = UNLOCK_LOCK, .offset = startpos, @@ -5764,6 +5766,7 @@ void reply_lock(struct smb_request *req) } *lck = (struct smbd_lock_element) { + .req_guid = smbd_request_guid(req, 0), .smblctx = req->smbpid, .brltype = WRITE_LOCK, .count = IVAL(req->vwv+1, 0), @@ -5855,6 +5858,7 @@ void reply_unlock(struct smb_request *req) } lck = (struct smbd_lock_element) { + .req_guid = smbd_request_guid(req, 0), .smblctx = req->smbpid, .brltype = UNLOCK_LOCK, .offset = IVAL(req->vwv+3, 0), @@ -8433,6 +8437,8 @@ void reply_lockingX(struct smb_request *req) * smb_unlkrng structs */ for (i = 0; i < num_ulocks; i++) { + ulocks[i].req_guid = smbd_request_guid(req, + UINT16_MAX - i), ulocks[i].smblctx = get_lock_pid( data, i, large_file_format); ulocks[i].count = get_lock_count( @@ -8490,6 +8496,7 @@ void reply_lockingX(struct smb_request *req) } for (i = 0; i < num_locks; i++) { + locks[i].req_guid = smbd_request_guid(req, i), locks[i].smblctx = get_lock_pid(data, i, large_file_format); locks[i].count = get_lock_count(data, i, large_file_format); locks[i].offset = get_lock_offset(data, i, large_file_format); diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c index 26de8b521ede..381aae6cb60c 100644 --- a/source3/smbd/smb2_lock.c +++ b/source3/smbd/smb2_lock.c @@ -318,6 +318,7 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + locks[i].req_guid = smbd_request_guid(smb2req->smb1req, i); locks[i].smblctx = fsp->op->global->open_persistent_id; locks[i].offset = in_locks[i].offset; locks[i].count = in_locks[i].length; diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 5b99240e9e88..0539b35bb739 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7572,6 +7572,7 @@ static NTSTATUS smb_set_posix_lock(connection_struct *conn, if (lock_type == UNLOCK_LOCK) { struct smbd_lock_element l = { + .req_guid = smbd_request_guid(req, 0), .smblctx = smblctx, .brltype = UNLOCK_LOCK, .offset = offset, @@ -7587,6 +7588,7 @@ static NTSTATUS smb_set_posix_lock(connection_struct *conn, } *lck = (struct smbd_lock_element) { + .req_guid = smbd_request_guid(req, 0), .smblctx = smblctx, .brltype = lock_type, .count = count, -- 2.17.1 From cba95afb506c072ff19436c5adde4f98c73cc293 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 19 Aug 2019 18:22:38 +0200 Subject: [PATCH 38/39] vfs_delay_inject: add support for brl_[un]lock_windows() This demonstrates the two ways to handle the retry: - smb layer retry => plock->context.smblctx = UINT64_MAX - vfs backend retry => plock->context.smblctx = 0 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit c2503a5c68e967054ab84ca0d8ce693200c2e002) --- source3/modules/vfs_delay_inject.c | 117 +++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/source3/modules/vfs_delay_inject.c b/source3/modules/vfs_delay_inject.c index d561fadb03be..569bd40054a7 100644 --- a/source3/modules/vfs_delay_inject.c +++ b/source3/modules/vfs_delay_inject.c @@ -304,12 +304,129 @@ static ssize_t vfs_delay_inject_pwrite_recv(struct tevent_req *req, return state->ret; } +struct vfs_delay_inject_brl_lock_state { + struct vfs_delay_inject_brl_lock_state *prev, *next; + struct files_struct *fsp; + struct GUID req_guid; + struct timeval delay_tv; + struct tevent_timer *delay_te; +}; + +static struct vfs_delay_inject_brl_lock_state *brl_lock_states; + +static int vfs_delay_inject_brl_lock_state_destructor(struct vfs_delay_inject_brl_lock_state *state) +{ + DLIST_REMOVE(brl_lock_states, state); + return 0; +} + +static void vfs_delay_inject_brl_lock_timer(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval current_time, + void *private_data) +{ + struct vfs_delay_inject_brl_lock_state *state = + talloc_get_type_abort(private_data, + struct vfs_delay_inject_brl_lock_state); + NTSTATUS status; + + TALLOC_FREE(state->delay_te); + + status = share_mode_wakeup_waiters(state->fsp->file_id); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("share_mode_wakeup_waiters(%s) %s\n", + file_id_string_tos(&state->fsp->file_id), + nt_errstr(status)); + } +} + +static NTSTATUS vfs_delay_inject_brl_lock_windows(struct vfs_handle_struct *handle, + struct byte_range_lock *br_lck, + struct lock_struct *plock) +{ + struct files_struct *fsp = brl_fsp(br_lck); + TALLOC_CTX *req_mem_ctx = brl_req_mem_ctx(br_lck); + const struct GUID *req_guid = brl_req_guid(br_lck); + struct vfs_delay_inject_brl_lock_state *state = NULL; + bool expired; + + for (state = brl_lock_states; state != NULL; state = state->next) { + bool match; + + match = GUID_equal(&state->req_guid, req_guid); + if (match) { + break; + } + } + + if (state == NULL) { + int delay; + bool use_timer; + + state = talloc_zero(req_mem_ctx, + struct vfs_delay_inject_brl_lock_state); + if (state == NULL) { + return NT_STATUS_NO_MEMORY; + } + state->fsp = fsp; + state->req_guid = *req_guid; + + delay = lp_parm_int(SNUM(handle->conn), + "delay_inject", "brl_lock_windows", 0); + state->delay_tv = timeval_current_ofs_msec(delay); + + use_timer = lp_parm_bool(SNUM(handle->conn), + "delay_inject", "brl_lock_windows_use_timer", true); + + if (use_timer) { + state->delay_te = tevent_add_timer( + global_event_context(), + state, + state->delay_tv, + vfs_delay_inject_brl_lock_timer, + state); + if (state->delay_te == NULL) { + return NT_STATUS_NO_MEMORY; + } + } + + talloc_set_destructor(state, + vfs_delay_inject_brl_lock_state_destructor); + DLIST_ADD_END(brl_lock_states, state); + } + + if (state->delay_te != NULL) { + plock->context.smblctx = 0; + return NT_STATUS_RETRY; + } + + expired = timeval_expired(&state->delay_tv); + if (!expired) { + plock->context.smblctx = UINT64_MAX; + return NT_STATUS_RETRY; + } + + TALLOC_FREE(state); + + return SMB_VFS_NEXT_BRL_LOCK_WINDOWS(handle, br_lck, plock); +} + +static bool vfs_delay_inject_brl_unlock_windows(struct vfs_handle_struct *handle, + struct byte_range_lock *br_lck, + const struct lock_struct *plock) +{ + return SMB_VFS_NEXT_BRL_UNLOCK_WINDOWS(handle, br_lck, plock); +} + static struct vfs_fn_pointers vfs_delay_inject_fns = { .ntimes_fn = vfs_delay_inject_ntimes, .pread_send_fn = vfs_delay_inject_pread_send, .pread_recv_fn = vfs_delay_inject_pread_recv, .pwrite_send_fn = vfs_delay_inject_pwrite_send, .pwrite_recv_fn = vfs_delay_inject_pwrite_recv, + + .brl_lock_windows_fn = vfs_delay_inject_brl_lock_windows, + .brl_unlock_windows_fn = vfs_delay_inject_brl_unlock_windows, }; static_decl_vfs; -- 2.17.1 From 2c8733c194fd3797ed5c98ae6c799db5e066be6b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 20 Aug 2019 15:53:59 +0200 Subject: [PATCH 39/39] s3:selftest: add delay_inject:brl_lock_windows testing BUG: https://bugzilla.samba.org/show_bug.cgi?id=14113 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Mon Sep 9 15:42:45 UTC 2019 on sn-devel-184 (cherry picked from commit 2b43ce6704ecf035e6734337a2dea3458153a4b2) --- selftest/target/Samba3.pm | 12 ++++++++++++ source3/selftest/tests.py | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 9638bb44f08b..131d576a767a 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2263,6 +2263,18 @@ sub provision($$$$$$$$$) delay_inject:pread_send = 2000 delay_inject:pwrite_send = 2000 +[brl_delay_inject1] + copy = tmp + vfs objects = delay_inject + delay_inject:brl_lock_windows = 90 + delay_inject:brl_lock_windows_use_timer = yes + +[brl_delay_inject2] + copy = tmp + vfs objects = delay_inject + delay_inject:brl_lock_windows = 90 + delay_inject:brl_lock_windows_use_timer = no + [delete_readonly] path = $prefix_abs/share delete readonly = yes diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index ebc366de3ea2..31cb8ca33f19 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -531,6 +531,10 @@ for t in tests: plansmbtorture4testsuite(t, env, '//$SERVER/tmp -k no -U$DC_USERNAME@$REALM%$DC_PASSWORD', description='ntlm user@realm') elif t == "raw.samba3posixtimedlock" or t == "smb2.samba3misc": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmpguest -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share') + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/brl_delay_inject1 -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share', + description="brl_delay_inject1") + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/brl_delay_inject2 -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share', + description="brl_delay_inject2") plansmbtorture4testsuite(t, "ad_dc", '//$SERVER_IP/tmpguest -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/ad_dc/share') elif t == "raw.chkpath": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmpcase -U$USERNAME%$PASSWORD') -- 2.17.1