From 84f1b94d381e573abb7f3c9063cc5701ff839adc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 17:49:31 +0200 Subject: [PATCH 01/14] smbtorture: remove unused torture_lease_ignore_handler() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit e1a38cd3f9f5665c9f7dd202fec1c7ec72fa419c) --- source4/torture/smb2/lease_break_handler.c | 12 ------------ source4/torture/smb2/lease_break_handler.h | 3 --- 2 files changed, 15 deletions(-) diff --git a/source4/torture/smb2/lease_break_handler.c b/source4/torture/smb2/lease_break_handler.c index 6c865dc20b1c..ca4cf2290d6b 100644 --- a/source4/torture/smb2/lease_break_handler.c +++ b/source4/torture/smb2/lease_break_handler.c @@ -88,18 +88,6 @@ bool torture_lease_handler(struct smb2_transport *transport, return true; } -/* - * A lease break handler which ignores incoming lease break requests - * To be used in cases where the client is expected to ignore incoming - * lease break requests - */ -bool torture_lease_ignore_handler(struct smb2_transport *transport, - const struct smb2_lease_break *lb, - void *private_data) -{ - return true; -} - /* Timer handler function notifies the registering function that time is up */ diff --git a/source4/torture/smb2/lease_break_handler.h b/source4/torture/smb2/lease_break_handler.h index f839bcaeaa37..5dffbb0f3b83 100644 --- a/source4/torture/smb2/lease_break_handler.h +++ b/source4/torture/smb2/lease_break_handler.h @@ -147,9 +147,6 @@ extern struct lease_break_info lease_break_info; bool torture_lease_handler(struct smb2_transport *transport, const struct smb2_lease_break *lb, void *private_data); -bool torture_lease_ignore_handler(struct smb2_transport *transport, - const struct smb2_lease_break *lb, - void *private_data); void torture_wait_for_lease_break(struct torture_context *tctx); static inline void torture_reset_lease_break_info(struct torture_context *tctx, -- 2.49.0 From 74f970f437379bdb27c79229e8cf8bdf0d1d8427 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 17:50:09 +0200 Subject: [PATCH 02/14] smbtorture: make torture_lease_break_callback() static It's only used in this compilation unit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 56fe5e8ef192f38b928eed9b454709242f02699e) --- source4/torture/smb2/lease_break_handler.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/smb2/lease_break_handler.c b/source4/torture/smb2/lease_break_handler.c index ca4cf2290d6b..b33e0cb31e94 100644 --- a/source4/torture/smb2/lease_break_handler.c +++ b/source4/torture/smb2/lease_break_handler.c @@ -31,7 +31,7 @@ struct lease_break_info lease_break_info; -void torture_lease_break_callback(struct smb2_request *req) +static void torture_lease_break_callback(struct smb2_request *req) { NTSTATUS status; -- 2.49.0 From 0980e271dc514929ce03b5fbde696b259f222ea1 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 17:50:46 +0200 Subject: [PATCH 03/14] smbtorture: add support for closing a handle when receiving a lease break BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit d88885b77cd9d86969eac2cd2010ed31e329106a) --- source4/torture/smb2/lease_break_handler.c | 30 ++++++++++++++++++++++ source4/torture/smb2/lease_break_handler.h | 4 +++ 2 files changed, 34 insertions(+) diff --git a/source4/torture/smb2/lease_break_handler.c b/source4/torture/smb2/lease_break_handler.c index b33e0cb31e94..ea0dfc40e26e 100644 --- a/source4/torture/smb2/lease_break_handler.c +++ b/source4/torture/smb2/lease_break_handler.c @@ -42,6 +42,17 @@ static void torture_lease_break_callback(struct smb2_request *req) return; } +static void torture_lease_break_close_callback(struct smb2_request *req) +{ + NTSTATUS status; + + status = smb2_close_recv(req, &lease_break_info.close); + if (!NT_STATUS_IS_OK(status)) { + lease_break_info.failures++; + } + return; +} + /* a lease break request handler */ bool torture_lease_handler(struct smb2_transport *transport, const struct smb2_lease_break *lb, @@ -64,6 +75,25 @@ bool torture_lease_handler(struct smb2_transport *transport, lease_break_info.lease_break = *lb; lease_break_info.count++; + if (!smb2_util_handle_empty(lease_break_info.lease_handle) && + (lb->current_lease.lease_state & SMB2_LEASE_HANDLE) && + !(lb->new_lease_state & SMB2_LEASE_HANDLE)) + { + torture_comment(lease_break_info.tctx, + "transport[%p] closing handle\n", + transport); + + ZERO_STRUCT(lease_break_info.close); + lease_break_info.close.in.file.handle = + lease_break_info.lease_handle; + ZERO_STRUCT(lease_break_info.lease_handle); + + req = smb2_close_send(tree, &lease_break_info.close); + req->async.fn = torture_lease_break_close_callback; + req->async.private_data = NULL; + return true; + } + if (lease_break_info.lease_skip_ack) { torture_comment(lease_break_info.tctx, "transport[%p] Skip %s to %s in lease handler\n", diff --git a/source4/torture/smb2/lease_break_handler.h b/source4/torture/smb2/lease_break_handler.h index 5dffbb0f3b83..731ac3a9e12f 100644 --- a/source4/torture/smb2/lease_break_handler.h +++ b/source4/torture/smb2/lease_break_handler.h @@ -28,6 +28,10 @@ struct lease_break_info { struct smb2_transport *lease_transport; bool lease_skip_ack; struct smb2_lease_break_ack lease_break_ack; + + struct smb2_handle lease_handle; + struct smb2_close close; + int count; int failures; -- 2.49.0 From 4d55e165b17120c8bb68ad28bd409ba7ad8203e2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 16:56:45 +0200 Subject: [PATCH 04/14] smbtorture: add test smb2.dirlease.rename_dst_parent BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit a2827f4d6d8b56de08adaee35a051022f255769e) --- source4/torture/smb2/lease.c | 110 +++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c index 60d5f81ae80f..c2bcda1d887b 100644 --- a/source4/torture/smb2/lease.c +++ b/source4/torture/smb2/lease.c @@ -6685,6 +6685,115 @@ static bool test_rename(struct torture_context *tctx, return ret; } +static bool test_rename_dst_parent(struct torture_context *tctx, + struct smb2_tree *tree) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + const char *srcdir = "test_rename_dst_parent_src"; + const char *srcfile = "test_rename_dst_parent_src\\file"; + const char *dstdir = "test_rename_dst_parent_dst"; + const char *dstfile = "test_rename_dst_parent_dst\\file"; + struct smb2_create create_dp; + struct smb2_lease lease_dp; + struct smb2_handle h1 = {}; + struct smb2_handle handle_sf = {}; + struct smb2_handle handle_dp = {}; + union smb_setfileinfo sfinfo = {}; + NTSTATUS status; + bool ret = true; + + tree->session->transport->lease.handler = torture_lease_handler; + tree->session->transport->lease.private_data = tree; + torture_reset_lease_break_info(tctx, &lease_break_info); + + status = torture_smb2_testdir(tree, srcdir, &h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testdir\n"); + smb2_util_close(tree, h1); + + status = torture_smb2_testdir(tree, dstdir, &h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testdir\n"); + smb2_util_close(tree, h1); + + status = smb2_create_simple_file(tctx, tree, srcfile, &handle_sf); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create_simple_file\n"); + + /* Get a RH lease on the destination parent */ + + smb2_lease_v2_create_share(&create_dp, &lease_dp, true, dstdir, + smb2_util_share_access("RWD"), + 0x01, NULL, + smb2_util_lease_state("RH"), 0); + create_dp.in.desired_access = DELETE_ACCESS; + status = smb2_create(tree, mem_ctx, &create_dp); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + handle_dp = create_dp.out.file.handle; + CHECK_LEASE_V2(&create_dp, "RH", true, 0x01, 0, 0, 1); + + /* + * Rename, expect break on dst parent. As we'll be keeping our + * conflicting open, the rename should fail. + */ + + sfinfo.generic.level = RAW_SFILEINFO_RENAME_INFORMATION; + sfinfo.generic.in.file.handle = handle_sf; + sfinfo.rename_information.in.new_name = dstfile; + + status = smb2_setinfo_file(tree, &sfinfo); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_SHARING_VIOLATION, ret, done, + "smb2_setinfo_file\n"); + + CHECK_BREAK_INFO_V2(tree->session->transport, + "RH", "R", 0x01, 2); + + /* Upgrade lease to "RH" */ + + smb2_lease_v2_create_share(&create_dp, &lease_dp, true, dstdir, + smb2_util_share_access("RWD"), + 0x01, NULL, + smb2_util_lease_state("RH"), 0); + create_dp.in.desired_access = DELETE_ACCESS; + status = smb2_create(tree, mem_ctx, &create_dp); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h1 = create_dp.out.file.handle; + smb2_util_close(tree, h1); + CHECK_LEASE_V2(&create_dp, "RH", true, 0x01, 0, 0, 3); + + /* + * Rename, expect break on dst parent. Let the break + * handler close the handle so the rename should pass. + */ + + torture_reset_lease_break_info(tctx, &lease_break_info); + lease_break_info.lease_handle = handle_dp; + + sfinfo.generic.level = RAW_SFILEINFO_RENAME_INFORMATION; + sfinfo.generic.in.file.handle = handle_sf; + sfinfo.rename_information.in.new_name = dstfile; + + status = smb2_setinfo_file(tree, &sfinfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_setinfo_file failed\n"); + + CHECK_LEASE_BREAK(&lease_break_info.lease_break, "RH", "R", 0x01); + +done: + if (!smb2_util_handle_empty(handle_sf)) { + smb2_util_close(tree, handle_sf); + } + if (!smb2_util_handle_empty(handle_dp)) { + smb2_util_close(tree, handle_dp); + } + smb2_deltree(tree, srcdir); + smb2_deltree(tree, dstdir); + return ret; +} + static bool test_overwrite(struct torture_context *tctx, struct smb2_tree *tree, struct smb2_tree *tree2) @@ -7624,6 +7733,7 @@ struct torture_suite *torture_smb2_dirlease_init(TALLOC_CTX *ctx) torture_suite_add_2smb2_test(suite, "setctime", test_dirlease_setctime); torture_suite_add_2smb2_test(suite, "setatime", test_dirlease_setatime); torture_suite_add_1smb2_test(suite, "rename", test_rename); + torture_suite_add_1smb2_test(suite, "rename_dst_parent", test_rename_dst_parent); torture_suite_add_2smb2_test(suite, "overwrite", test_overwrite); torture_suite_add_1smb2_test(suite, "hardlink", test_hardlink); torture_suite_add_1smb2_test(suite, "unlink_same_set_and_close", test_unlink_same_set_and_close); -- 2.49.0 From 97878402fff10c6aa45732e9053678e054ab3e77 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 23 May 2025 07:26:53 +0200 Subject: [PATCH 05/14] smbtorture: fix test smb2.notify-inotify.inotify-rename Need to remove SEC_STD_DELETE from the access mask, otherwise we can't move files into this directory. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 8b346857b837259c017b47cb6a935ed54afc8c60) --- source4/torture/smb2/notify.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c index b76cb7967aa2..06917abec01c 100644 --- a/source4/torture/smb2/notify.c +++ b/source4/torture/smb2/notify.c @@ -2532,7 +2532,11 @@ static bool torture_smb2_inotify_rename(struct torture_context *torture, torture_comment(torture, "Testing change notify of a rename with inotify\n"); - status = torture_smb2_testdir(tree1, BASEDIR_INR, &h1); + status = torture_smb2_testdir_access( + tree1, + BASEDIR_INR, + &h1, + SEC_RIGHTS_DIR_ALL & ~SEC_STD_DELETE); torture_assert_ntstatus_ok_goto(torture, status, ok, done, "torture_smb2_testdir failed"); ZERO_STRUCT(create); -- 2.49.0 From 8be358056847b2d58ed626f75bc61626942483a4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 23 May 2025 16:35:35 +0200 Subject: [PATCH 06/14] smbd: expand logging in contend_dirleases() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 509081e7ed064899701a9e53b1597c33bcf5b77d) --- source3/smbd/smb2_oplock.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c index c278db252399..4e900647ef5d 100644 --- a/source3/smbd/smb2_oplock.c +++ b/source3/smbd/smb2_oplock.c @@ -1336,8 +1336,13 @@ void contend_dirleases(struct connection_struct *conn, ret = SMB_VFS_STAT(conn, parent_fname); if (ret != 0) { - DBG_ERR("Failed to stat [%s]: %s\n", - smb_fname_str_dbg(parent_fname), strerror(errno)); + DBG_ERR("Trigger [conn: %s] [smb_fname: %s] cwd [%s], " + "failed to stat parent [%s]: %s\n", + conn->connectpath, + smb_fname_str_dbg(smb_fname), + get_current_dir_name(), + smb_fname_str_dbg(parent_fname), + strerror(errno)); TALLOC_FREE(parent_fname); return; } -- 2.49.0 From f4f8cab0950ec0c8fd70260e0a3b705792db3ff2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 16:57:49 +0200 Subject: [PATCH 07/14] smbd: support breaking leases on an object where we don't have an own internal open So far, when dealing with the rename destination, we had an internal open on that which ensured get_existing_share_mode_lock() would always return a lock. Subsequently I'll want delay_for_handle_lease_break_send() to be callable on an object that doesn't have a full internal open including locking.tdb entry, but merely an open handle from filename_convert_dirfsp(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 03c46342e2a65e6d81bd581471310c0ec2cbbdfb) --- source3/smbd/smb2_oplock.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c index 4e900647ef5d..16a4c70da2c0 100644 --- a/source3/smbd/smb2_oplock.c +++ b/source3/smbd/smb2_oplock.c @@ -1639,6 +1639,12 @@ static void delay_for_handle_lease_break_fsp_check(struct tevent_req *req) DBG_DEBUG("fsp [%s]\n", fsp_str_dbg(state->fsp)); + if (state->lck == NULL) { + DBG_DEBUG("fsp [%s] all opens are gone\n", + fsp_str_dbg(state->fsp)); + return; + } + ok = share_mode_forall_leases(state->lck, delay_for_handle_lease_break_fn, state); @@ -1696,11 +1702,6 @@ static void delay_for_handle_lease_break_fsp_done(struct tevent_req *subreq) } state->lck = get_existing_share_mode_lock(state, state->fsp->file_id); - if (state->lck == NULL) { - tevent_req_nterror(req, NT_STATUS_UNSUCCESSFUL); - return; - } - /* * This could potentially end up looping for some if a client * aggressively reaquires H-leases on the file, but we have a @@ -1964,11 +1965,6 @@ static void delay_for_handle_lease_break_below_done(struct tevent_req *subreq) state->recursive_h_leases_break = false; state->lck = get_existing_share_mode_lock(state, state->fsp->file_id); - if (state->lck == NULL) { - tevent_req_nterror(req, NT_STATUS_UNSUCCESSFUL); - return; - } - delay_for_handle_lease_break_check(req); } -- 2.49.0 From 830c0aaf844a0871d2a5f0c07cec28360d206f68 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 21 May 2025 19:17:54 +0200 Subject: [PATCH 08/14] smbd: add has_delete_access_opens() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 1351b613679acb063d7ef8bc63d438e1ec973a9a) --- source3/smbd/close.c | 41 +++++++++++++++++++++++++++++++++++++++++ source3/smbd/proto.h | 4 ++++ 2 files changed, 45 insertions(+) diff --git a/source3/smbd/close.c b/source3/smbd/close.c index cf0e0e6419db..53032a345f0b 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -278,6 +278,47 @@ bool has_other_nonposix_opens(struct share_mode_lock *lck, return state.found_another; } +struct has_delete_access_opens_state { + bool delete_access; + bool delete_pending; +}; + +static bool has_delete_access_opens_fn( + struct share_mode_entry *e, + bool *modified, + void *private_data) +{ + struct has_delete_access_opens_state *state = private_data; + + if (share_entry_stale_pid(e)) { + return false; + } + + if (e->access_mask & SEC_STD_DELETE) { + state->delete_access = true; + } + return false; +} + +bool has_delete_opens(struct files_struct *fsp, + struct share_mode_lock *lck, + bool *delete_access, + bool *delete_pending) +{ + struct has_delete_access_opens_state state = {}; + bool ok; + + ok = share_mode_forall_entries( + lck, has_delete_access_opens_fn, &state); + if (!ok) { + return false; + } + + *delete_access = state.delete_access; + *delete_pending = is_delete_on_close_set(lck, fsp->name_hash); + return true; +} + bool has_nonposix_opens(struct share_mode_lock *lck) { struct has_other_nonposix_opens_state state = {}; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index b5704aeefff1..9c89bcc96f57 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -145,6 +145,10 @@ NTSTATUS recursive_rmdir(TALLOC_CTX *ctx, bool has_other_nonposix_opens(struct share_mode_lock *lck, struct files_struct *fsp); bool has_nonposix_opens(struct share_mode_lock *lck); +bool has_delete_opens(struct files_struct *fsp, + struct share_mode_lock *lck, + bool *delete_access, + bool *delete_pending); /* The following definitions come from smbd/conn.c */ -- 2.49.0 From 696faa445f59745371ef6f721b369670d7bba76e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 23 May 2025 17:06:50 +0200 Subject: [PATCH 09/14] smbd: add access_mask to delay_for_handle_lease_break_send() No change in behaviour, existing caller all pass SEC_RIGHTS_DIR_ALL. Prepares for selectively breaking only H-leases if the access_mask of the corresponding open contains DELETE_ACCESS. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 5d4565c1f974b75a1c080f4503613201ecaf7001) --- source3/smbd/proto.h | 1 + source3/smbd/smb2_close.c | 1 + source3/smbd/smb2_oplock.c | 7 +++++++ source3/smbd/smb2_setinfo.c | 2 ++ source3/smbd/smbXsrv_session.c | 1 + 5 files changed, 12 insertions(+) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 9c89bcc96f57..d227e639e1ca 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1247,6 +1247,7 @@ struct tevent_req *delay_for_handle_lease_break_send( struct tevent_context *ev, struct timeval timeout, struct files_struct *fsp, + uint32_t access_mask, bool recursive, struct share_mode_lock **lck); diff --git a/source3/smbd/smb2_close.c b/source3/smbd/smb2_close.c index d68c0bd9e6c2..4195adc3804e 100644 --- a/source3/smbd/smb2_close.c +++ b/source3/smbd/smb2_close.c @@ -369,6 +369,7 @@ static struct tevent_req *smbd_smb2_close_send(TALLOC_CTX *mem_ctx, ev, timeout, in_fsp, + SEC_RIGHTS_DIR_ALL, false, &lck); if (tevent_req_nomem(subreq, req)) { diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c index 16a4c70da2c0..4b7a6c0363c1 100644 --- a/source3/smbd/smb2_oplock.c +++ b/source3/smbd/smb2_oplock.c @@ -1516,6 +1516,7 @@ struct delay_for_handle_lease_break_state { TALLOC_CTX *mem_ctx; struct tevent_context *ev; struct timeval timeout; + uint32_t access_mask; bool recursive; bool recursive_h_leases_break; struct files_struct *fsp; @@ -1546,6 +1547,7 @@ struct tevent_req *delay_for_handle_lease_break_send( struct tevent_context *ev, struct timeval timeout, struct files_struct *fsp, + uint32_t access_mask, bool recursive, struct share_mode_lock **lck) { @@ -1564,6 +1566,7 @@ struct tevent_req *delay_for_handle_lease_break_send( .mem_ctx = mem_ctx, .ev = ev, .timeout = timeout, + .access_mask = access_mask, .recursive = recursive, .recursive_h_leases_break = recursive, .fsp = fsp, @@ -1604,6 +1607,10 @@ static bool delay_for_handle_lease_break_fn(struct share_mode_entry *e, } } + if ((state->access_mask & e->access_mask) == 0) { + return false; + } + lease_type = get_lease_type(e, fsp->file_id); if ((lease_type & SMB2_LEASE_HANDLE) == 0) { return false; diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c index 91280a52c077..86cd0eac0cac 100644 --- a/source3/smbd/smb2_setinfo.c +++ b/source3/smbd/smb2_setinfo.c @@ -489,6 +489,7 @@ static void smbd_smb2_setinfo_lease_break_fsp_check(struct tevent_req *req) state->ev, timeout, fsp, + SEC_RIGHTS_DIR_ALL, rename, &state->lck); if (tevent_req_nomem(subreq, req)) { @@ -656,6 +657,7 @@ static void smbd_smb2_setinfo_rename_dst_check(struct tevent_req *req) state->ev, timeout, state->dstfsp, + SEC_RIGHTS_DIR_ALL, false, &state->lck); if (subreq == NULL) { diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c index c074bb851de0..3d4f0967570f 100644 --- a/source3/smbd/smbXsrv_session.c +++ b/source3/smbd/smbXsrv_session.c @@ -2618,6 +2618,7 @@ static struct files_struct *smbXsrv_wait_for_handle_lease_break_fn( state->ev, timeout, fsp, + SEC_RIGHTS_DIR_ALL, false, &lck); if (tevent_req_nomem(subreq, state->req)) { -- 2.49.0 From b6e1abbddda394b084cef5d239e1efa60ea4bc77 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 11:42:13 +0200 Subject: [PATCH 10/14] smbd: implement H-lease breaks on parent directory of rename target BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 91b7a5e7ac308040bd60a172280e4429ee25f3b0) --- selftest/knownfail | 1 - source3/smbd/smb2_setinfo.c | 178 ++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail b/selftest/knownfail index 51aae99d6b48..3b02dcef2de6 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -204,7 +204,6 @@ ^samba4.smb2.ioctl-on-stream.ioctl-on-stream\(ad_dc_ntvfs\) ^samba3.smb2.dir.one ^samba3.smb2.dir.modify -^samba3.smb2.oplock.batch20 ^samba3.smb2.oplock.stream1 ^samba3.smb2.streams.rename ^samba3.smb2.streams.rename2 diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c index 86cd0eac0cac..b6256907c9c2 100644 --- a/source3/smbd/smb2_setinfo.c +++ b/source3/smbd/smb2_setinfo.c @@ -175,10 +175,12 @@ struct smbd_smb2_setinfo_state { struct files_struct *fsp; struct share_mode_lock *lck; struct files_struct *dstfsp; + struct files_struct *dst_parent_dirfsp; uint16_t file_info_level; DATA_BLOB data; bool delay; bool rename_dst_check_done; + bool rename_dst_parent_check_done; }; static void smbd_smb2_setinfo_lease_break_check(struct tevent_req *req); @@ -394,6 +396,9 @@ static void smbd_smb2_setinfo_lease_break_fsp_check(struct tevent_req *req); static void smbd_smb2_setinfo_lease_break_fsp_done(struct tevent_req *subreq); static void smbd_smb2_setinfo_rename_dst_check(struct tevent_req *req); static void smbd_smb2_setinfo_rename_dst_delay_done(struct tevent_req *subreq); +static void smbd_smb2_setinfo_rename_dst_parent_check(struct tevent_req *req); +static void smbd_smb2_setinfo_rename_dst_parent_delay_done( + struct tevent_req *subreq); static void smbd_smb2_setinfo_lease_break_check(struct tevent_req *req) { @@ -413,6 +418,16 @@ static void smbd_smb2_setinfo_lease_break_check(struct tevent_req *req) return; } + smbd_smb2_setinfo_rename_dst_parent_check(req); + if (!tevent_req_is_in_progress(req)) { + return; + } + if (state->delay) { + DBG_DEBUG("Waiting for h-lease breaks on rename destination " + "parent directory\n"); + return; + } + smbd_smb2_setinfo_lease_break_fsp_check(req); if (!tevent_req_is_in_progress(req)) { return; @@ -767,6 +782,169 @@ static void smbd_smb2_setinfo_rename_dst_delay_done(struct tevent_req *subreq) smbd_smb2_setinfo_lease_break_check(req); } +static void smbd_smb2_setinfo_rename_dst_parent_check(struct tevent_req *req) +{ + struct smbd_smb2_setinfo_state *state = tevent_req_data( + req, struct smbd_smb2_setinfo_state); + struct tevent_req *subreq = NULL; + struct timeval timeout; + bool overwrite = false; + struct files_struct *fsp = state->fsp; + struct files_struct *dst_parent_dirfsp = NULL; + struct smb_filename *smb_fname_dst = NULL; + char *dst_original_lcomp = NULL; + NTSTATUS status; + + if (state->rename_dst_parent_check_done) { + return; + } + state->rename_dst_parent_check_done = true; + + if (state->file_info_level != SMB2_FILE_RENAME_INFORMATION_INTERNAL) { + return; + } + if (is_named_stream(fsp->fsp_name)) { + return; + } + status = smb2_parse_file_rename_information(state, + fsp->conn, + state->smb2req->smb1req, + (char *)state->data.data, + state->data.length, + fsp, + fsp->fsp_name, + &overwrite, + &dst_parent_dirfsp, + &smb_fname_dst, + &dst_original_lcomp); + if (tevent_req_nterror(req, status)) { + return; + } + SMB_ASSERT(dst_parent_dirfsp != NULL); + state->dst_parent_dirfsp = dst_parent_dirfsp; + + state->lck = get_existing_share_mode_lock(state, + dst_parent_dirfsp->file_id); + if (state->lck == NULL) { + /* No opens around */ + return; + } + + timeout = tevent_timeval_set(OPLOCK_BREAK_TIMEOUT, 0); + timeout = timeval_sum(&state->smb2req->request_time, &timeout); + + subreq = delay_for_handle_lease_break_send(state, + state->ev, + timeout, + dst_parent_dirfsp, + SEC_STD_DELETE, + false, + &state->lck); + if (tevent_req_nomem(subreq, req)) { + return; + } + if (tevent_req_is_in_progress(subreq)) { + state->delay = true; + tevent_req_set_callback( + subreq, + smbd_smb2_setinfo_rename_dst_parent_delay_done, + req); + return; + } + + status = delay_for_handle_lease_break_recv(subreq, state, &state->lck); + TALLOC_FREE(subreq); + if (tevent_req_nterror(req, status)) { + return; + } + + if (state->lck != NULL) { + bool delete_open; + bool detete_pending; + bool ok; + + ok = has_delete_opens(dst_parent_dirfsp, + state->lck, + &delete_open, + &detete_pending); + TALLOC_FREE(state->lck); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + + if (detete_pending) { + tevent_req_nterror(req, NT_STATUS_DELETE_PENDING); + return; + } + if (delete_open) { + tevent_req_nterror(req, NT_STATUS_SHARING_VIOLATION); + return; + } + } + + return; +} + +static void smbd_smb2_setinfo_rename_dst_parent_delay_done( + struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct smbd_smb2_setinfo_state *state = tevent_req_data( + req, struct smbd_smb2_setinfo_state); + struct smbXsrv_session *session = state->smb2req->session; + NTSTATUS status; + bool ok; + + status = delay_for_handle_lease_break_recv(subreq, state, &state->lck); + TALLOC_FREE(subreq); + if (tevent_req_nterror(req, status)) { + return; + } + + /* + * Make sure we run as the user again + */ + ok = change_to_user_and_service(state->fsp->conn, + session->global->session_wire_id); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return; + } + + if (state->lck != NULL) { + bool delete_open; + bool detete_pending; + + ok = has_delete_opens(state->dst_parent_dirfsp, + state->lck, + &delete_open, + &detete_pending); + TALLOC_FREE(state->lck); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + + if (detete_pending) { + tevent_req_nterror(req, NT_STATUS_DELETE_PENDING); + return; + } + if (delete_open) { + tevent_req_nterror(req, NT_STATUS_SHARING_VIOLATION); + return; + } + } + + /* + * We've finished breaking H-lease on the rename destination, now + * trigger the fsp check. + */ + state->rename_dst_parent_check_done = true; + smbd_smb2_setinfo_lease_break_check(req); +} + static NTSTATUS smbd_smb2_setinfo_recv(struct tevent_req *req) { NTSTATUS status; -- 2.49.0 From 860fe79c5b80880965380214ad84ef60539ae337 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 26 May 2025 11:18:57 +0200 Subject: [PATCH 11/14] selftest: stop running smb2.streams against "ad_dc" environment Drop running smb2.streams tests against the "ad_dc" environment, to simplify test failure handling with the knownfail file. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 8c0888a94bbcd55b5746305ab82c9ce50095db32) --- source3/selftest/tests.py | 1 - source4/torture/smb2/streams.c | 20 +++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 437a4a8ee137..3768b919c3ef 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -1363,7 +1363,6 @@ tests = base + raw + smb2 + rpc + unix + local + rap + nbt + idmap + vfs plansmbtorture4testsuite(t, "ad_dc_smb1", '//$SERVER/tmp -U$USERNAME%$PASSWORD --option=torture:wksname=samba3rpctest') elif t == "smb2.streams": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') - plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/streams_xattr -U$USERNAME%$PASSWORD', 'streams_xattr') elif t == "smb2.stream-inherit-perms": plansmbtorture4testsuite(t, "fileserver", '//$SERVER/inherit_perms -U$USERNAME%$PASSWORD') diff --git a/source4/torture/smb2/streams.c b/source4/torture/smb2/streams.c index bfdb41a3a38c..84f0d009f3ff 100644 --- a/source4/torture/smb2/streams.c +++ b/source4/torture/smb2/streams.c @@ -1522,17 +1522,15 @@ static bool test_stream_rename2(struct torture_context *tctx, status = smb2_setinfo_file(tree, &sinfo); CHECK_STATUS(status, NT_STATUS_SHARING_VIOLATION); - if (!torture_setting_bool(tctx, "samba4", false)) { - /* - * Check SMB2 rename to the default stream using :. - */ - torture_comment(tctx, "(%s) Checking SMB2 rename to default stream " - "using :\n", __location__); - sinfo.rename_information.in.file.handle = h1; - sinfo.rename_information.in.new_name = stream_name_default; - status = smb2_setinfo_file(tree, &sinfo); - CHECK_STATUS(status, NT_STATUS_OK); - } + /* + * Check SMB2 rename to the default stream using :. + */ + torture_comment(tctx, "(%s) Checking SMB2 rename to default stream " + "using :\n", __location__); + sinfo.rename_information.in.file.handle = h1; + sinfo.rename_information.in.new_name = stream_name_default; + status = smb2_setinfo_file(tree, &sinfo); + CHECK_STATUS(status, NT_STATUS_OK); smb2_util_close(tree, h1); -- 2.49.0 From 927da11e67a7b9b8cc0e499889e004a470ced2e4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 26 May 2025 12:32:16 +0200 Subject: [PATCH 12/14] selftest: stop running smb2.streams against the ad_dc_ntvfs This will soon start failing with UNEXPECTED(failure): samba4.smb2.streams.rename2(ad_dc_ntvfs) REASON: Exception: Exception: ../../source4/torture/smb2/streams.c:1533: status was NT_STATUS_OBJECT_NAME_COLLISION, expected NT_STATUS_OK: CHECK_STATUS and I don't see the point in tracking this down for a dead product. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 69a7d5881bd0d91d270b4a8be4c75dcd94f19897) --- source4/selftest/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index fab95fcfef3f..9c5e85e428af 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -443,6 +443,7 @@ smb2_s3only = [ "smb2.twrp", "smb2.ea", "smb2.create_no_streams", + "smb2.streams", ] smb2 = [x for x in smbtorture4_testsuites("smb2.") if x not in smb2_s3only] -- 2.49.0 From a61784972e916039fc4e961cfad237cd387e30d0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 22 May 2025 17:52:28 +0200 Subject: [PATCH 13/14] smbd: remove parent_dirname_compatible_open() This is now handled correctly smbd_smb2_setinfo_rename_dst_parent_check(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke (cherry picked from commit 17ed8c0f9a0ab8b4b7feb213b4e3a0765f8cc0cd) --- selftest/knownfail | 2 -- source3/smbd/smb2_reply.c | 48 --------------------------------------- 2 files changed, 50 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index 3b02dcef2de6..103a0bb1d76a 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -205,9 +205,7 @@ ^samba3.smb2.dir.one ^samba3.smb2.dir.modify ^samba3.smb2.oplock.stream1 -^samba3.smb2.streams.rename ^samba3.smb2.streams.rename2 -^samba3.smb2.streams streams_xattr.rename\(nt4_dc\) ^samba3.smb2.streams streams_xattr.rename2\(nt4_dc\) ^samba3.smb2.getinfo.complex ^samba3.smb2.getinfo.fsinfo # quotas don't work yet diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c index d3220978db32..15e070714b70 100644 --- a/source3/smbd/smb2_reply.c +++ b/source3/smbd/smb2_reply.c @@ -1343,49 +1343,6 @@ static void notify_rename(struct connection_struct *conn, TALLOC_FREE(parent_dir_dst); } -/**************************************************************************** - Returns an error if the parent directory for a filename is open in an - incompatible way. -****************************************************************************/ - -static NTSTATUS parent_dirname_compatible_open(connection_struct *conn, - const struct smb_filename *smb_fname_dst_in) -{ - struct smb_filename *smb_fname_parent = NULL; - struct file_id id; - files_struct *fsp = NULL; - int ret; - NTSTATUS status; - - status = SMB_VFS_PARENT_PATHNAME(conn, - talloc_tos(), - smb_fname_dst_in, - &smb_fname_parent, - NULL); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - - ret = vfs_stat(conn, smb_fname_parent); - if (ret == -1) { - return map_nt_error_from_unix(errno); - } - - /* - * We're only checking on this smbd here, mostly good - * enough.. and will pass tests. - */ - - id = vfs_file_id_from_sbuf(conn, &smb_fname_parent->st); - for (fsp = file_find_di_first(conn->sconn, id, true); fsp; - fsp = file_find_di_next(fsp, true)) { - if (fsp->access_mask & DELETE_ACCESS) { - return NT_STATUS_SHARING_VIOLATION; - } - } - return NT_STATUS_OK; -} - /**************************************************************************** Rename an open file - given an fsp. ****************************************************************************/ @@ -1414,11 +1371,6 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, true : conn->case_preserve; struct vfs_rename_how rhow = { .flags = 0, }; - status = parent_dirname_compatible_open(conn, smb_fname_dst_in); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - if (file_has_open_streams(fsp)) { return NT_STATUS_ACCESS_DENIED; } -- 2.49.0 From 03d8641600ce08cc0833812b157af1058b186674 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 24 May 2025 11:47:37 +0200 Subject: [PATCH 14/14] smbd: use fsp->name_hash in check_parent_access_fsp() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG: https://bugzilla.samba.org/show_bug.cgi?id=15861 Signed-off-by: Ralph Boehme Reviewed-by: Bjoern Jacke Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Wed May 28 16:03:59 UTC 2025 on atb-devel-224 (backported from commit 9b9fc589e55d467c97fd4580c2d6d9aa8cb73b13) [slow@samba.org: removed write_time arg to get_file_infos() in master] --- source3/smbd/open.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index e23f5d29e6e5..1c920816a263 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -346,7 +346,6 @@ NTSTATUS check_parent_access_fsp(struct files_struct *fsp, NTSTATUS status; struct security_descriptor *parent_sd = NULL; uint32_t access_granted = 0; - uint32_t name_hash; bool delete_on_close_set; TALLOC_CTX *frame = talloc_stackframe(); @@ -406,15 +405,7 @@ NTSTATUS check_parent_access_fsp(struct files_struct *fsp, goto out; } - /* Check if the directory has delete-on-close set */ - status = file_name_hash(fsp->conn, - fsp->fsp_name->base_name, - &name_hash); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - get_file_infos(fsp->file_id, name_hash, &delete_on_close_set, NULL); + get_file_infos(fsp->file_id, fsp->name_hash, &delete_on_close_set, NULL); if (delete_on_close_set) { status = NT_STATUS_DELETE_PENDING; goto out; -- 2.49.0