In debugging Microsoft functional test failures against Samba, I noticed that lease breaks are not issued on renames of the parent directory (leases are enabled, as are durable handles, kernel oplocks are disabled in my smb.conf for testing leasev2 and durable handles obviously). I created a modified torture test (not very useful by itself unfortunately) which opens a file with a RWH lease and just sits for 15 seconds. I then rename the parent directory from a cifs.ko mount. In Samba 4.2 I don't see a lease break on the child file's lease, but in Windows 8.1 server I do.
Created attachment 10826 [details] wireshark trace - samba does not issue lease break to open child files on rename of parent
Created attachment 10827 [details] wireshark trace -Windows 8.1 does issue lease break to open child file on rename of parent Client is smbtorture (smb2.lease lightly modified) See frame 40 (lease break issued by server due to rename of parent directory)
Comment on attachment 10826 [details] wireshark trace - samba does not issue lease break to open child files on rename of parent client in both of these is a modified lease.c (source4/smbtorture)
Various Microsoft tests generate it - but in case you prefer smbtorture, I do a rename of the directory smb2_dir (from a cifs mount) while the smbtorture test below is blocked (I added a sleep of 12 seconds in the middle of the below) in order to demonstrate the problem diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c index 4b435a1..96df6fe 100644 --- a/source4/torture/smb2/lease.c +++ b/source4/torture/smb2/lease.c @@ -3882,6 +3882,206 @@ static bool test_lease_dynamic_share(struct torture_context *tctx, return ret; } +#define DNAME "smb2_dir" +#define FNAME "smb2_dir\\smb2_lease_file_for_rename_test" +#define NFILES 10 + +struct file_elem { + char *name; + NTTIME create_time; + bool found; +}; + +static bool dir_rename_of_parent(struct torture_context *tctx, + struct smb2_tree *tree) +{ + struct smb2_create create; + char **strs = NULL; + NTSTATUS status; + bool ret; + int i; + int nfiles = NFILES; + TALLOC_CTX *mem_ctx = talloc_new(tctx); + struct smb2_handle *h_out; + struct file_elem files[NFILES] = {}; + + smb2_deltree(tree, DNAME); + + ZERO_STRUCT(create); + create.in.desired_access = SEC_RIGHTS_DIR_ALL; + create.in.create_options = NTCREATEX_OPTIONS_DIRECTORY; + create.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; + create.in.share_access = NTCREATEX_SHARE_ACCESS_READ | + NTCREATEX_SHARE_ACCESS_WRITE | + NTCREATEX_SHARE_ACCESS_DELETE; + create.in.create_disposition = NTCREATEX_DISP_CREATE; + create.in.fname = DNAME; + + status = smb2_create(tree, mem_ctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, ""); + *h_out = create.out.file.handle; + + ZERO_STRUCT(create); + create.in.desired_access = SEC_RIGHTS_FILE_ALL; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.create_disposition = NTCREATEX_DISP_CREATE; + +/* create file, open file with lease, rename parent directory */ + + strs = generate_unique_strs(mem_ctx, 8, nfiles); + if (strs == NULL) { + status = NT_STATUS_OBJECT_NAME_COLLISION; + goto done; + } + for (i = 0; i < nfiles; i++) { + files[i].name = strs[i]; + create.in.fname = talloc_asprintf(mem_ctx, "%s\\%s", + DNAME, files[i].name); + status = smb2_create(tree, mem_ctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, ""); + files[i].create_time = create.out.create_time; + smb2_util_close(tree, create.out.file.handle); + } + done: + return true; +} + +static bool create_dir(struct torture_context *tctx, struct smb2_tree *tree) +{ + NTSTATUS status; + struct smb2_create io; + struct smb2_handle handle; + union smb_fileinfo q; + union smb_setfileinfo set; + struct security_descriptor *sd, *sd_orig; + const char *owner_sid; + uint32_t perms = 0; + bool ret; + + torture_comment(tctx, "Creating Directory for testing: %s\n", DNAME); + + ZERO_STRUCT(io); + io.level = RAW_OPEN_SMB2; + io.in.create_flags = 0; + io.in.desired_access = + SEC_STD_READ_CONTROL | + SEC_STD_WRITE_DAC | + SEC_STD_WRITE_OWNER; + io.in.create_options = NTCREATEX_OPTIONS_DIRECTORY; + io.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; + io.in.share_access = + NTCREATEX_SHARE_ACCESS_READ | + NTCREATEX_SHARE_ACCESS_WRITE; + io.in.alloc_size = 0; + io.in.create_disposition = NTCREATEX_DISP_OPEN_IF; + io.in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS; + io.in.security_flags = 0; + io.in.fname = DNAME; + status = smb2_create(tree, tctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + handle = io.out.file.handle; + + torture_comment(tctx, "get the original sd\n"); + q.query_secdesc.level = RAW_FILEINFO_SEC_DESC; + q.query_secdesc.in.file.handle = handle; + q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER; + status = smb2_getinfo_file(tree, tctx, &q); + CHECK_STATUS(status, NT_STATUS_OK); + sd_orig = q.query_secdesc.out.sd; + + owner_sid = dom_sid_string(tctx, sd_orig->owner_sid); + + /* + * We create an SD that allows us to do most things but we do not + * get DELETE and DELETE CHILD access! + */ + + perms = SEC_STD_SYNCHRONIZE | SEC_STD_WRITE_OWNER | + SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL | + SEC_DIR_WRITE_ATTRIBUTE | SEC_DIR_READ_ATTRIBUTE | + SEC_DIR_TRAVERSE | SEC_DIR_WRITE_EA | + SEC_FILE_READ_EA | SEC_FILE_APPEND_DATA | + SEC_FILE_WRITE_DATA | SEC_FILE_READ_DATA; + + torture_comment(tctx, "Setting permissions on dir to 0x1e01bf\n"); + sd = security_descriptor_dacl_create(tctx, + 0, owner_sid, NULL, + owner_sid, + SEC_ACE_TYPE_ACCESS_ALLOWED, + perms, + SEC_ACE_FLAG_OBJECT_INHERIT, + NULL); + + set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC; + set.set_secdesc.in.file.handle = handle; + set.set_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER; + set.set_secdesc.in.sd = sd; + + status = smb2_setinfo_file(tree, &set); + CHECK_STATUS(status, NT_STATUS_OK); + + status = smb2_util_close(tree, handle); +done: + return true; +} + +static bool sample_test_doc_create(struct torture_context *tctx, struct smb2_tree *tree) +{ + struct smb2_create io; + NTSTATUS status; + uint32_t perms = 0; + struct smb2_lease ls; + bool ret; + + smb2_deltree(tree, DNAME); + + create_dir(tctx, tree); + + torture_comment(tctx, "Create file with DeleteOnClose on non-existent file (CREATE) \n"); + torture_comment(tctx, "We expect NT_STATUS_OK\n"); + + perms = SEC_STD_SYNCHRONIZE | SEC_STD_READ_CONTROL | SEC_STD_DELETE | + SEC_DIR_WRITE_ATTRIBUTE | SEC_DIR_READ_ATTRIBUTE | + SEC_DIR_WRITE_EA | SEC_FILE_APPEND_DATA | + SEC_FILE_WRITE_DATA; + + ZERO_STRUCT(io); + smb2_lease_create(&io, &ls, false, FNAME, LEASE2, smb2_util_lease_state("RHW")); + io.in.desired_access = perms; + io.in.file_attributes = 0; + io.in.create_disposition = NTCREATEX_DISP_CREATE; + io.in.share_access = NTCREATEX_SHARE_ACCESS_DELETE; + io.in.create_options = NTCREATEX_OPTIONS_DELETE_ON_CLOSE | + NTCREATEX_OPTIONS_NON_DIRECTORY_FILE; + io.in.fname = FNAME; + + status = smb2_create(tree, tctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + smb_msleep(12000); /* wait 12 seconds to give me time to rename parent from another client */ + + /* While this is blocked sleeping - do a "mv smb2_dir somedirname" from any cifs/smb2/smb3 client (I used Linux cifs.ko) */ + + status = smb2_util_close(tree, io.out.file.handle); + + /* Check it was deleted */ + ZERO_STRUCT(io); + io.in.desired_access = perms; + io.in.file_attributes = 0; + io.in.create_disposition = NTCREATEX_DISP_OPEN; + io.in.share_access = NTCREATEX_SHARE_ACCESS_DELETE; + io.in.create_options = 0; + io.in.fname = FNAME; + + torture_comment(tctx, "Testing if the file was deleted when closed\n"); + torture_comment(tctx, "We expect NT_STATUS_OBJECT_NAME_NOT_FOUND\n"); + + status = smb2_create(tree, tctx, &io); + CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND); +done: + return true; +} + struct torture_suite *torture_smb2_lease_init(void) { struct torture_suite *suite = @@ -3919,6 +4119,7 @@ struct torture_suite *torture_smb2_lease_init(void) torture_suite_add_1smb2_test(suite, "v2_rename", test_lease_v2_rename); torture_suite_add_1smb2_test(suite, "dynamic_share", test_lease_dynamic_share); torture_suite_add_1smb2_test(suite, "timeout", test_lease_timeout); + torture_suite_add_1smb2_test(suite, "rename_parent", sample_test_doc_create); suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
In the test above, I cut-and-pasted from a delete on close test, but delete on close doesn't have things to do with it.
Metze suggested setting "strict rename = yes" in smb.conf and retrying these tests
Adding "strict rename = yes" to the share definition did not alter the test case behavior. The Samba behavior seems to violate MS-SMB2 (see e.g. 3.3.1.4) HANDLE caching permits one or more SMB2 clients to delay closing handles it holds open, or to defer sending opens. Before processing one of the following operations, the underlying object store MUST request that the server revoke HANDLE caching, and the object store MUST wait for acknowledgment before proceeding with the operation: HANDLE caching on a file: A file is opened with an access or share mode incompatible with opens from different ClientIds or local applications as described in [MS-FSA] section 2.1.5.1.2. The parent directory is being renamed.
This should be taken care of by f435f1b3acb75c065166e3077c01acbd88601f34: Author: Jeremy Allison <jra@samba.org> AuthorDate: Thu Dec 4 21:19:32 2014 -0800 Commit: Jeremy Allison <jra@samba.org> CommitDate: Fri Dec 5 18:37:10 2014 +0100 s3: leases: Make SMB2 setinfo SMB2_FILE_RENAME_INFORMATION_INTERNAL async. If there are any RH leases we must break them to read and must wait for the client response before doing the rename. Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> Signed-off-by: Jeremy Allison <jra@samba.org> Signed-off-by: Stefan Metzmacher <metze@samba.org> Are you sure the Samba version you tested got that one?
This is definitely fixed in master.