[RFC] Simplify async path based VFS functions: remove tevent and pthreadpool_tevent wrappers
Ralph Böhme
slow at samba.org
Mon Jan 14 13:09:51 UTC 2019
On Fri, Jan 11, 2019 at 03:59:52PM -0800, Jeremy Allison wrote:
>On Fri, Jan 11, 2019 at 11:10:53PM +0100, Stefan Metzmacher wrote:
>> > A pipeline is running here:
>> > https://gitlab.com/samba-team/devel/samba/pipelines/43026374
>> >
>> > We plan to push this one the pipeline reports success.
>>
>> Pushed to autobuild...
>
>Thanks a *LOT* for this rollback. It makes the async
>VFS code a lot easier to understand (for me at least).
It also requires careful explicit impersonation in many places which we actually
forgot to add.
Attached patchset adds it, please review&push if happy. Thanks!
For completeness: https://gitlab.com/samba-team/devel/samba/pipelines/43265119
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG-Fingerprint FAE2C6088A24252051C559E4AA1E9B7126399E46
-------------- next part --------------
From cad96dab4ebe8345ed007ff5b94043b8c05ca1c6 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:50:31 +0100
Subject: [PATCH 1/8] vfs: use struct initializer in
smb_vfs_call_get_dos_attributes_send
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/vfs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 5e0ca091f7b..6d304f068de 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -2409,7 +2409,10 @@ struct tevent_req *smb_vfs_call_get_dos_attributes_send(
}
VFS_FIND(get_dos_attributes_send);
- state->recv_fn = handle->fns->get_dos_attributes_recv_fn;
+
+ *state = (struct smb_vfs_call_get_dos_attributes_state) {
+ .recv_fn = handle->fns->get_dos_attributes_recv_fn,
+ };
subreq = handle->fns->get_dos_attributes_send_fn(mem_ctx,
ev,
--
2.17.2
From 2231d40d615aa154e0f829e6c2dac272e1ea070c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:51:23 +0100
Subject: [PATCH 2/8] vfs: perform impersonation in
smb_vfs_call_get_dos_attributes_done()
This is needed as the callback might be called in an arbitrary
impersonation state.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/vfs.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 6d304f068de..2910128ccf6 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -2382,6 +2382,7 @@ NTSTATUS smb_vfs_call_offload_write_recv(struct vfs_handle_struct *handle,
}
struct smb_vfs_call_get_dos_attributes_state {
+ files_struct *dir_fsp;
NTSTATUS (*recv_fn)(struct tevent_req *req,
struct vfs_aio_state *aio_state,
uint32_t *dosmode);
@@ -2411,6 +2412,7 @@ struct tevent_req *smb_vfs_call_get_dos_attributes_send(
VFS_FIND(get_dos_attributes_send);
*state = (struct smb_vfs_call_get_dos_attributes_state) {
+ .dir_fsp = dir_fsp,
.recv_fn = handle->fns->get_dos_attributes_recv_fn,
};
@@ -2440,6 +2442,13 @@ static void smb_vfs_call_get_dos_attributes_done(struct tevent_req *subreq)
tevent_req_data(req,
struct smb_vfs_call_get_dos_attributes_state);
NTSTATUS status;
+ bool ok;
+
+ /*
+ * Make sure we run as the user again
+ */
+ ok = change_to_user_by_fsp(state->dir_fsp);
+ SMB_ASSERT(ok);
status = state->recv_fn(subreq,
&state->aio_state,
--
2.17.2
From 736eed06a10b9d4021eca6d15f02a7173930b08b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:52:51 +0100
Subject: [PATCH 3/8] vfs: use struct initializer in
smb_vfs_call_getxattrat_send()
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/vfs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 2910128ccf6..d31610f8ebb 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -2690,7 +2690,10 @@ struct tevent_req *smb_vfs_call_getxattrat_send(
}
VFS_FIND(getxattrat_send);
- state->recv_fn = handle->fns->getxattrat_recv_fn;
+
+ *state = (struct smb_vfs_call_getxattrat_state) {
+ .recv_fn = handle->fns->getxattrat_recv_fn,
+ };
subreq = handle->fns->getxattrat_send_fn(mem_ctx,
ev,
--
2.17.2
From 5adce655cb1b0cb8725d35078521a00bd4f88261 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:51:23 +0100
Subject: [PATCH 4/8] vfs: perform impersonation in
smb_vfs_call_getxattrat_done()
This is needed as the callback might be called in an arbitrary
impersonation state.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/vfs.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index d31610f8ebb..d3bb9c5d63f 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -2659,6 +2659,7 @@ ssize_t smb_vfs_call_getxattr(struct vfs_handle_struct *handle,
struct smb_vfs_call_getxattrat_state {
+ files_struct *dir_fsp;
ssize_t (*recv_fn)(struct tevent_req *req,
struct vfs_aio_state *aio_state,
TALLOC_CTX *mem_ctx,
@@ -2692,6 +2693,7 @@ struct tevent_req *smb_vfs_call_getxattrat_send(
VFS_FIND(getxattrat_send);
*state = (struct smb_vfs_call_getxattrat_state) {
+ .dir_fsp = dir_fsp,
.recv_fn = handle->fns->getxattrat_recv_fn,
};
@@ -2717,6 +2719,13 @@ static void smb_vfs_call_getxattrat_done(struct tevent_req *subreq)
subreq, struct tevent_req);
struct smb_vfs_call_getxattrat_state *state = tevent_req_data(
req, struct smb_vfs_call_getxattrat_state);
+ bool ok;
+
+ /*
+ * Make sure we run as the user again
+ */
+ ok = change_to_user_by_fsp(state->dir_fsp);
+ SMB_ASSERT(ok);
state->retval = state->recv_fn(subreq,
&state->aio_state,
--
2.17.2
From 5b72473b48ed417119511c8ff1afe0e6699becd7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:54:29 +0100
Subject: [PATCH 5/8] vfs_default: use change_to_user_by_fsp() instead of
change_to_user()
Cosmetic change.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 8d40cd64540..a27d33a6bea 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -3177,12 +3177,8 @@ static void vfswrap_getxattrat_done(struct tevent_req *subreq)
/*
* Make sure we run as the user again
*/
- ok = change_to_user(state->dir_fsp->conn,
- state->dir_fsp->vuid);
- if (!ok) {
- smb_panic("Can't change to user");
- return;
- }
+ ok = change_to_user_by_fsp(state->dir_fsp);
+ SMB_ASSERT(ok);
ret = pthreadpool_tevent_job_recv(subreq);
TALLOC_FREE(subreq);
--
2.17.2
From 1bfa57d7e616a64e1ad743555643f197d072d813 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:51:23 +0100
Subject: [PATCH 6/8] s3:smbd: perform impersonation in
dos_mode_at_vfs_get_dosmode_done()
This is needed as the callback might be called in an arbitrary
impersonation state.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/dosmode.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
index 177fe68c894..a4625718a56 100644
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -811,6 +811,13 @@ static void dos_mode_at_vfs_get_dosmode_done(struct tevent_req *subreq)
struct smb_filename *smb_path = NULL;
struct vfs_aio_state aio_state;
NTSTATUS status;
+ bool ok;
+
+ /*
+ * Make sure we run as the user again
+ */
+ ok = change_to_user_by_fsp(state->dir_fsp);
+ SMB_ASSERT(ok);
status = SMB_VFS_GET_DOS_ATTRIBUTES_RECV(subreq,
&aio_state,
--
2.17.2
From 817967045708a4604a66f98798d938e6d9418544 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:51:23 +0100
Subject: [PATCH 7/8] s3:smbd: perform impersonation in
smb2_query_directory_dos_mode_done()
This is needed as the callback might be called in an arbitrary
impersonation state.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_query_directory.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source3/smbd/smb2_query_directory.c b/source3/smbd/smb2_query_directory.c
index fdb87188f93..2a426a0dfa3 100644
--- a/source3/smbd/smb2_query_directory.c
+++ b/source3/smbd/smb2_query_directory.c
@@ -752,6 +752,13 @@ static void smb2_query_directory_dos_mode_done(struct tevent_req *subreq)
tevent_req_data(req,
struct smbd_smb2_query_directory_state);
NTSTATUS status;
+ bool ok;
+
+ /*
+ * Make sure we run as the user again
+ */
+ ok = change_to_user_by_fsp(state->fsp);
+ SMB_ASSERT(ok);
status = fetch_dos_mode_recv(subreq);
TALLOC_FREE(subreq);
--
2.17.2
From 8d3cf763625b92fe7aa9dcb8ba0ed6897393a4fe Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 14 Jan 2019 13:51:23 +0100
Subject: [PATCH 8/8] s3:smbd: perform impersonation in
smb2_query_directory_fetch_write_time_done()
This is not strictly required, as we ne never trigger additional VFS
requests via this codepath. But for safety reasons ensure we're running
in the correct impersonation state.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_query_directory.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source3/smbd/smb2_query_directory.c b/source3/smbd/smb2_query_directory.c
index 2a426a0dfa3..13fb820ce6c 100644
--- a/source3/smbd/smb2_query_directory.c
+++ b/source3/smbd/smb2_query_directory.c
@@ -730,6 +730,13 @@ static void smb2_query_directory_fetch_write_time_done(struct tevent_req *subreq
struct smbd_smb2_query_directory_state *state = tevent_req_data(
req, struct smbd_smb2_query_directory_state);
NTSTATUS status;
+ bool ok;
+
+ /*
+ * Make sure we run as the user again
+ */
+ ok = change_to_user_by_fsp(state->fsp);
+ SMB_ASSERT(ok);
state->async_sharemode_count--;
--
2.17.2
More information about the samba-technical
mailing list