From 1ccc55d132015839a568a8132a5e607ca11f293c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 15:26:42 -0700 Subject: [PATCH 1/7] s3: tests: Our tests for "smbd async dosmode = yes" haven't been working correctly as the parameter has been set incorrectly. If must be "smbd async dosmode", not "smbd:async dosmode" BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- selftest/target/Samba3.pm | 20 ++++++++++---------- source3/modules/vfs_catia.c | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 054ceb38a09..dc1c14e9628 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1470,7 +1470,7 @@ sub setup_simpleserver read only = no vfs objects = aio_pthread aio_pthread:aio open = yes - smbd:async dosmode = no + smbd async dosmode = no [vfs_aio_pthread_async_dosmode_default1] path = $prefix_abs/share @@ -1478,7 +1478,7 @@ sub setup_simpleserver vfs objects = aio_pthread store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes [vfs_aio_pthread_async_dosmode_default2] path = $prefix_abs/share @@ -1486,7 +1486,7 @@ sub setup_simpleserver vfs objects = aio_pthread xattr_tdb store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes [vfs_aio_pthread_async_dosmode_force_sync1] path = $prefix_abs/share @@ -1494,7 +1494,7 @@ sub setup_simpleserver vfs objects = aio_pthread store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes # This simulates non linux systems smbd:force sync user path safe threadpool = yes smbd:force sync user chdir safe threadpool = yes @@ -1507,7 +1507,7 @@ sub setup_simpleserver vfs objects = aio_pthread xattr_tdb store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes # This simulates non linux systems smbd:force sync user path safe threadpool = yes smbd:force sync user chdir safe threadpool = yes @@ -1834,7 +1834,7 @@ sub setup_fileserver_smb1 read only = no vfs objects = aio_pthread aio_pthread:aio open = yes - smbd:async dosmode = no + smbd async dosmode = no [vfs_aio_pthread_async_dosmode_default1] path = $prefix_abs/share @@ -1842,7 +1842,7 @@ sub setup_fileserver_smb1 vfs objects = aio_pthread store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes [vfs_aio_pthread_async_dosmode_default2] path = $prefix_abs/share @@ -1850,7 +1850,7 @@ sub setup_fileserver_smb1 vfs objects = aio_pthread xattr_tdb store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes [vfs_aio_pthread_async_dosmode_force_sync1] path = $prefix_abs/share @@ -1858,7 +1858,7 @@ sub setup_fileserver_smb1 vfs objects = aio_pthread store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes # This simulates non linux systems smbd:force sync user path safe threadpool = yes smbd:force sync user chdir safe threadpool = yes @@ -1871,7 +1871,7 @@ sub setup_fileserver_smb1 vfs objects = aio_pthread xattr_tdb store dos attributes = yes aio_pthread:aio open = yes - smbd:async dosmode = yes + smbd async dosmode = yes # This simulates non linux systems smbd:force sync user path safe threadpool = yes smbd:force sync user chdir safe threadpool = yes diff --git a/source3/modules/vfs_catia.c b/source3/modules/vfs_catia.c index bf1c1cd425a..ce5a91c9f10 100644 --- a/source3/modules/vfs_catia.c +++ b/source3/modules/vfs_catia.c @@ -166,7 +166,7 @@ static int catia_connect(struct vfs_handle_struct *handle, * Unless we have an async implementation of get_dos_attributes turn * this off. */ - lp_do_parameter(SNUM(handle->conn), "smbd:async dosmode", "false"); + lp_do_parameter(SNUM(handle->conn), "smbd async dosmode", "false"); return SMB_VFS_NEXT_CONNECT(handle, service, user); } -- 2.27.0 From a9675d2e6cc6e3d46f351e23642ce4d7985fb0d4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 15:29:01 -0700 Subject: [PATCH 2/7] s3: tests: Add "SMB2-LIST-DIR-ASYNC" test. Add as knownfail. Shows our "smbd async dosmode" code wasn't working. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/smb2_list_dir_async | 1 + source3/selftest/tests.py | 16 +++++ source3/torture/proto.h | 1 + source3/torture/test_smb2.c | 84 ++++++++++++++++++++++++ source3/torture/torture.c | 4 ++ 5 files changed, 106 insertions(+) create mode 100644 selftest/knownfail.d/smb2_list_dir_async diff --git a/selftest/knownfail.d/smb2_list_dir_async b/selftest/knownfail.d/smb2_list_dir_async new file mode 100644 index 00000000000..bc3858276f7 --- /dev/null +++ b/selftest/knownfail.d/smb2_list_dir_async @@ -0,0 +1 @@ +^samba3.smbtorture_s3.plain.SMB2-LIST-DIR-ASYNC.smbtorture\(simpleserver\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 876cd41e486..cf745907219 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -242,6 +242,22 @@ plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-STREAM-ACL", "", "-l $LOCAL_PATH"]) +# +# SMB2-LIST-DIR-ASYNC needs to run against a special share vfs_aio_pthread_async_dosmode_default1 +# +plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-LIST-DIR-ASYNC", + "simpleserver", + [os.path.join(samba3srcdir, + "script/tests/test_smbtorture_s3.sh"), + 'SMB2-LIST-DIR-ASYNC', + '//$SERVER_IP/vfs_aio_pthread_async_dosmode_default1', + '$USERNAME', + '$PASSWORD', + smbtorture3, + "", + "-l $LOCAL_PATH"]) + + shares = [ "vfs_aio_pthread_async_dosmode_default1", "vfs_aio_pthread_async_dosmode_default2", diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 90363577ad9..4db267c92b0 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -119,6 +119,7 @@ bool run_smb2_path_slash(int dummy); bool run_smb2_sacl(int dummy); bool run_smb2_quota1(int dummy); bool run_smb2_stream_acl(int dummy); +bool run_list_dir_async_test(int dummy); bool run_chain3(int dummy); bool run_local_conv_auth_info(int dummy); bool run_local_sprintf_append(int dummy); diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c index feb8b654f3b..befdc7ad463 100644 --- a/source3/torture/test_smb2.c +++ b/source3/torture/test_smb2.c @@ -3144,3 +3144,87 @@ bool run_smb2_stream_acl(int dummy) (void)cli_unlink(cli, fname, 0); return ret; } + +static NTSTATUS list_fn(struct file_info *finfo, + const char *name, + void *state) +{ + bool *matched = (bool *)state; + if (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) { + *matched = true; + } + return NT_STATUS_OK; +} + +/* + * Must be run against a share with "smbd async dosmode = yes". + * Checks we can return DOS attriutes other than "N". + * BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 + */ + +bool run_list_dir_async_test(int dummy) +{ + struct cli_state *cli = NULL; + NTSTATUS status; + const char *dname = "ASYNC_DIR"; + bool ret = false; + bool matched = false; + + printf("SMB2 list dir async\n"); + + if (!torture_init_connection(&cli)) { + return false; + } + + status = smbXcli_negprot(cli->conn, + cli->timeout, + PROTOCOL_SMB2_02, + PROTOCOL_SMB3_11); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_negprot returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_session_setup_creds(cli, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_session_setup returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_tree_connect(cli, share, "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_tree_connect returned %s\n", nt_errstr(status)); + return false; + } + + /* Ensure directory doesn't exist. */ + (void)cli_rmdir(cli, dname); + + status = cli_mkdir(cli, dname); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_mkdir %s returned %s\n", dname, nt_errstr(status)); + return false; + } + + status = cli_list(cli, + dname, + FILE_ATTRIBUTE_NORMAL|FILE_ATTRIBUTE_DIRECTORY, + list_fn, + &matched); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_list %s returned %s\n", dname, nt_errstr(status)); + goto fail; + } + + if (!matched) { + printf("Failed to find %s\n", dname); + goto fail; + } + + ret = true; + + fail: + + (void)cli_rmdir(cli, dname); + return ret; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 5deffab6dc2..79a9c65073c 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -15245,6 +15245,10 @@ static struct { .name = "SMB2-STREAM-ACL", .fn = run_smb2_stream_acl, }, + { + .name = "SMB2-LIST-DIR-ASYNC", + .fn = run_list_dir_async_test, + }, { .name = "CLEANUP1", .fn = run_cleanup1, -- 2.27.0 From c5084eb061f67e447e7ce4d5223baeaed2b7be05 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 15:00:13 -0700 Subject: [PATCH 3/7] s3: smbd: Allow "smbd async dosmode = yes" to return valid DOS attributes again. We already have a valid smb_fname->fsp, don't drop it when returning from smbd_dirptr_lanman2_entry() to allow it to be reused inside dos_mode_at_send(). Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/smb2_list_dir_async | 1 - source3/smbd/trans2.c | 28 ++++++++++++++---------- 2 files changed, 16 insertions(+), 13 deletions(-) delete mode 100644 selftest/knownfail.d/smb2_list_dir_async diff --git a/selftest/knownfail.d/smb2_list_dir_async b/selftest/knownfail.d/smb2_list_dir_async deleted file mode 100644 index bc3858276f7..00000000000 --- a/selftest/knownfail.d/smb2_list_dir_async +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.SMB2-LIST-DIR-ASYNC.smbtorture\(simpleserver\) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index c69e48a4a56..b9e2786eda6 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2551,23 +2551,27 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx, } if (_smb_fname != NULL) { - struct smb_filename *name = NULL; - - name = synthetic_smb_fname(ctx, - fname, - NULL, - &smb_fname->st, - smb_fname->twrp, - 0); - if (name == NULL) { + /* + * smb_fname is already talloc'ed off ctx. + * We just need to make sure we don't return + * any stream_name, and replace base_name + * with fname in case base_name got mangled. + * This allows us to preserve any smb_fname->fsp + * for asynchronous handle lookups. + */ + TALLOC_FREE(smb_fname->stream_name); + TALLOC_FREE(smb_fname->base_name); + smb_fname->base_name = talloc_strdup(smb_fname, fname); + + if (smb_fname->base_name == NULL) { TALLOC_FREE(smb_fname); TALLOC_FREE(fname); return NT_STATUS_NO_MEMORY; } - *_smb_fname = name; + *_smb_fname = smb_fname; + } else { + TALLOC_FREE(smb_fname); } - - TALLOC_FREE(smb_fname); TALLOC_FREE(fname); if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) { -- 2.27.0 From 3da04b53c97e2be4963dfefb92d35f1173b0adc0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 11:17:49 -0700 Subject: [PATCH 4/7] s3: VFS: default: Move vfswrap_fgetxattr() before the async versions. We want to re-use this and don't want to have to add forward declarations. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 60 +++++++++++++++++------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 102938e332e..e9bd2471556 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3425,6 +3425,36 @@ static int vfswrap_sys_acl_delete_def_fd(vfs_handle_struct *handle, Extended attribute operations. *****************************************************************/ +static ssize_t vfswrap_fgetxattr(struct vfs_handle_struct *handle, + struct files_struct *fsp, + const char *name, + void *value, + size_t size) +{ + int fd = fsp_get_pathref_fd(fsp); + + if (!fsp->fsp_flags.is_pathref) { + return fgetxattr(fd, name, value, size); + } + + if (fsp->fsp_flags.have_proc_fds) { + const char *p = NULL; + char buf[PATH_MAX]; + + p = sys_proc_fd_path(fd, buf, sizeof(buf)); + if (p == NULL) { + return -1; + } + + return getxattr(p, name, value, size); + } + + /* + * This is no longer a handle based call. + */ + return getxattr(fsp->fsp_name->base_name, name, value, size); +} + struct vfswrap_getxattrat_state { struct tevent_context *ev; files_struct *dir_fsp; @@ -3742,36 +3772,6 @@ static ssize_t vfswrap_getxattrat_recv(struct tevent_req *req, return xattr_size; } -static ssize_t vfswrap_fgetxattr(struct vfs_handle_struct *handle, - struct files_struct *fsp, - const char *name, - void *value, - size_t size) -{ - int fd = fsp_get_pathref_fd(fsp); - - if (!fsp->fsp_flags.is_pathref) { - return fgetxattr(fd, name, value, size); - } - - if (fsp->fsp_flags.have_proc_fds) { - const char *p = NULL; - char buf[PATH_MAX]; - - p = sys_proc_fd_path(fd, buf, sizeof(buf)); - if (p == NULL) { - return -1; - } - - return getxattr(p, name, value, size); - } - - /* - * This is no longer a handle based call. - */ - return getxattr(fsp->fsp_name->base_name, name, value, size); -} - static ssize_t vfswrap_flistxattr(struct vfs_handle_struct *handle, struct files_struct *fsp, char *list, size_t size) { int fd = fsp_get_pathref_fd(fsp); -- 2.27.0 From ea6b2c1546e0c5a80c85376b3400a2ff50294d03 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 11:35:06 -0700 Subject: [PATCH 5/7] s3: VFS: default: Add 'handle' member to struct vfswrap_getxattrat_state Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index e9bd2471556..9d7c0539528 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3457,6 +3457,7 @@ static ssize_t vfswrap_fgetxattr(struct vfs_handle_struct *handle, struct vfswrap_getxattrat_state { struct tevent_context *ev; + struct vfs_handle_struct *handle; files_struct *dir_fsp; const struct smb_filename *smb_fname; @@ -3509,6 +3510,7 @@ static struct tevent_req *vfswrap_getxattrat_send( } *state = (struct vfswrap_getxattrat_state) { .ev = ev, + .handle = handle, .dir_fsp = dir_fsp, .smb_fname = smb_fname, }; -- 2.27.0 From 3266196c1046ff75d77870b48cccd89b932eae00 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 11:23:03 -0700 Subject: [PATCH 6/7] s3: VFS: default. In vfswrap_getxattrat_do_sync() always use the pathref fsp. This is always called via a path that mandates smb_fname->fsp is valid. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 9d7c0539528..cb50342ce67 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3609,31 +3609,19 @@ static void vfswrap_getxattrat_do_sync(struct tevent_req *req) { struct vfswrap_getxattrat_state *state = tevent_req_data( req, struct vfswrap_getxattrat_state); - char *path = NULL; - char *tofree = NULL; - char pathbuf[PATH_MAX+1]; - ssize_t pathlen; - int err; + struct files_struct *fsp = state->smb_fname->fsp; - pathlen = full_path_tos(state->dir_fsp->fsp_name->base_name, - state->smb_fname->base_name, - pathbuf, - sizeof(pathbuf), - &path, - &tofree); - if (pathlen == -1) { - tevent_req_error(req, ENOMEM); - return; + if (fsp->base_fsp != NULL) { + fsp = fsp->base_fsp; } - state->xattr_size = getxattr(path, - state->xattr_name, - state->xattr_value, - talloc_array_length(state->xattr_value)); - err = errno; - TALLOC_FREE(tofree); + state->xattr_size = vfswrap_fgetxattr(state->handle, + fsp, + state->xattr_name, + state->xattr_value, + talloc_array_length(state->xattr_value)); if (state->xattr_size == -1) { - tevent_req_error(req, err); + tevent_req_error(req, errno); return; } -- 2.27.0 From e3d4f7170b0dc17313722b12add4e8ebd7d328e9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 14 Jul 2021 11:23:54 -0700 Subject: [PATCH 7/7] s3: VFS: default. In vfswrap_getxattrat_do_async() always use the pathref fsp. This is always called via a path that mandates smb_fname->fsp is valid. https://bugzilla.samba.org/show_bug.cgi?id=14758 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index cb50342ce67..aa7dfe3192f 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3636,6 +3636,11 @@ static void vfswrap_getxattrat_do_async(void *private_data) struct timespec start_time; struct timespec end_time; int ret; + struct files_struct *fsp = state->smb_fname->fsp; + + if (fsp->base_fsp != NULL) { + fsp = fsp->base_fsp; + } PROFILE_TIMESTAMP(&start_time); SMBPROFILE_BYTES_ASYNC_SET_BUSY(state->profile_bytes); @@ -3658,17 +3663,11 @@ static void vfswrap_getxattrat_do_async(void *private_data) goto end_profile; } - ret = fchdir(fsp_get_pathref_fd(state->dir_fsp)); - if (ret == -1) { - state->xattr_size = -1; - state->vfs_aio_state.error = errno; - goto end_profile; - } - - state->xattr_size = getxattr(state->name, - state->xattr_name, - state->xattr_value, - talloc_array_length(state->xattr_value)); + state->xattr_size = vfswrap_fgetxattr(state->handle, + fsp, + state->xattr_name, + state->xattr_value, + talloc_array_length(state->xattr_value)); if (state->xattr_size == -1) { state->vfs_aio_state.error = errno; } -- 2.27.0