[PATCH] Allow passing a context down
Jeremy Allison
jra at samba.org
Tue Feb 20 20:54:19 UTC 2018
On Tue, Feb 20, 2018 at 04:32:49PM +0100, Andreas Schneider via samba-technical wrote:
> Hi Volker,
>
> we are not very valgrind clean, because for a lot of allocations we just use
> the NULL context and do not free memory allocated there.
>
> The attached patch shows that for a lot of these things we could pass down the
> right memory context. However we do not have a memory context for the lifetime
> of a daemon. The stackframe is freed and allocated again before we fork, so
> this can't be used.
>
> Does it make sense to allocate a "global" memory contexts which only is
> allowed to be used during daemon init and then fried when it dies?
An enthusiastic +1 and 'Reviewed-by:' from me. Moving us to
a non-null context-based system is something I've been slowly
doing for a long time.
Eventually I want us to get to the "global" memory context, but
I'd prefer to move cautiously on this to ensure we don't
screw anything up :-).
Jeremy.
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> From 2667e48e40da8a6d34a6aacfae2926bdb34fa21d Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 13 Feb 2018 12:05:29 +0100
> Subject: [PATCH 1/3] s3:auth: Pass a mem_ctx to make_new_session_info_guest()
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/auth/auth_util.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
> index f543b33eead..377d56d0488 100644
> --- a/source3/auth/auth_util.c
> +++ b/source3/auth/auth_util.c
> @@ -798,8 +798,12 @@ static NTSTATUS get_guest_info3(TALLOC_CTX *mem_ctx,
> left as-is for now.
> ***************************************************************************/
>
> -static NTSTATUS make_new_session_info_guest(struct auth_session_info **session_info, struct auth_serversupplied_info **server_info)
> +static NTSTATUS make_new_session_info_guest(TALLOC_CTX *mem_ctx,
> + struct auth_session_info **_session_info,
> + struct auth_serversupplied_info **_server_info)
> {
> + struct auth_session_info *session_info = NULL;
> + struct auth_serversupplied_info *server_info = NULL;
> const char *guest_account = lp_guest_account();
> const char *domain = lp_netbios_name();
> struct netr_SamInfo3 info3;
> @@ -823,7 +827,7 @@ static NTSTATUS make_new_session_info_guest(struct auth_session_info **session_i
> status = make_server_info_info3(tmp_ctx,
> guest_account,
> domain,
> - server_info,
> + &server_info,
> &info3);
> if (!NT_STATUS_IS_OK(status)) {
> DEBUG(0, ("make_server_info_info3 failed with %s\n",
> @@ -831,25 +835,26 @@ static NTSTATUS make_new_session_info_guest(struct auth_session_info **session_i
> goto done;
> }
>
> - (*server_info)->guest = true;
> + server_info->guest = true;
>
> /* This should not be done here (we should produce a server
> * info, and later construct a session info from it), but for
> * now this does not change the previous behavior */
> - status = create_local_token(tmp_ctx, *server_info, NULL,
> - (*server_info)->info3->base.account_name.string,
> - session_info);
> + status = create_local_token(tmp_ctx, server_info, NULL,
> + server_info->info3->base.account_name.string,
> + &session_info);
> if (!NT_STATUS_IS_OK(status)) {
> DEBUG(0, ("create_local_token failed: %s\n",
> nt_errstr(status)));
> goto done;
> }
> - talloc_steal(NULL, *session_info);
> - talloc_steal(NULL, *server_info);
>
> /* annoying, but the Guest really does have a session key, and it is
> all zeros! */
> - (*session_info)->session_key = data_blob_talloc_zero(NULL, 16);
> + session_info->session_key = data_blob_talloc_zero(session_info, 16);
> +
> + *_session_info = talloc_move(mem_ctx, &session_info);
> + *_server_info = talloc_move(mem_ctx, &server_info);
>
> status = NT_STATUS_OK;
> done:
> @@ -1136,7 +1141,7 @@ bool init_guest_info(void)
> if (guest_info != NULL)
> return true;
>
> - return NT_STATUS_IS_OK(make_new_session_info_guest(&guest_info, &guest_server_info));
> + return NT_STATUS_IS_OK(make_new_session_info_guest(NULL, &guest_info, &guest_server_info));
> }
>
> NTSTATUS make_server_info_guest(TALLOC_CTX *mem_ctx,
> --
> 2.16.1
>
>
> From 2609c03cc9e8bcf4eb75c436bbc18aeb140dc29f Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 13 Feb 2018 12:09:12 +0100
> Subject: [PATCH 2/3] s3:auth: Pass mem_ctx to init_guest_session_info()
>
> Use a mem_ctx which gets freed if possible.
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/auth/auth_util.c | 9 +++++++--
> source3/auth/proto.h | 2 +-
> source3/smbd/server.c | 2 +-
> source3/torture/vfstest.c | 2 +-
> 4 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
> index 377d56d0488..c81432d277d 100644
> --- a/source3/auth/auth_util.c
> +++ b/source3/auth/auth_util.c
> @@ -1136,12 +1136,17 @@ static struct auth_session_info *guest_info = NULL;
>
> static struct auth_serversupplied_info *guest_server_info = NULL;
>
> -bool init_guest_info(void)
> +bool init_guest_session_info(TALLOC_CTX *mem_ctx)
> {
> + NTSTATUS status;
> +
> if (guest_info != NULL)
> return true;
>
> - return NT_STATUS_IS_OK(make_new_session_info_guest(NULL, &guest_info, &guest_server_info));
> + status = make_new_session_info_guest(mem_ctx,
> + &guest_info,
> + &guest_server_info);
> + return NT_STATUS_IS_OK(status);
> }
>
> NTSTATUS make_server_info_guest(TALLOC_CTX *mem_ctx,
> diff --git a/source3/auth/proto.h b/source3/auth/proto.h
> index ca851c21d4b..6d6f789d8b6 100644
> --- a/source3/auth/proto.h
> +++ b/source3/auth/proto.h
> @@ -240,7 +240,7 @@ NTSTATUS make_session_info_from_username(TALLOC_CTX *mem_ctx,
> struct auth_session_info **session_info);
> struct auth_session_info *copy_session_info(TALLOC_CTX *mem_ctx,
> const struct auth_session_info *src);
> -bool init_guest_info(void);
> +bool init_guest_session_info(TALLOC_CTX *mem_ctx);
> NTSTATUS init_system_session_info(void);
> bool session_info_set_session_key(struct auth_session_info *info,
> DATA_BLOB session_key);
> diff --git a/source3/smbd/server.c b/source3/smbd/server.c
> index 99baf9d519d..d80ea7311bd 100644
> --- a/source3/smbd/server.c
> +++ b/source3/smbd/server.c
> @@ -1991,7 +1991,7 @@ extern void build_options(bool screen);
> return -1;
> }
>
> - if (!init_guest_info()) {
> + if (!init_guest_session_info(NULL)) {
> DEBUG(0,("ERROR: failed to setup guest info.\n"));
> return -1;
> }
> diff --git a/source3/torture/vfstest.c b/source3/torture/vfstest.c
> index f156def4647..17c19012384 100644
> --- a/source3/torture/vfstest.c
> +++ b/source3/torture/vfstest.c
> @@ -525,7 +525,7 @@ int main(int argc, const char *argv[])
>
> /* some basic initialization stuff */
> sec_init();
> - init_guest_info();
> + init_guest_session_info(frame);
> locking_init();
> vfs = talloc_zero(NULL, struct vfs_state);
> if (vfs == NULL) {
> --
> 2.16.1
>
>
> From 919de40aa78ab719d6e056297684b69ccbb64c0e Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 13 Feb 2018 12:12:06 +0100
> Subject: [PATCH 3/3] s3:auth: Pass mem_ctx to init_system_session_info()
>
> We have a stackframe we can use for the lifetime of the session.
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/auth/auth_util.c | 4 ++--
> source3/auth/proto.h | 2 +-
> source3/smbd/server.c | 2 +-
> source3/winbindd/winbindd.c | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
> index c81432d277d..4b2026127ed 100644
> --- a/source3/auth/auth_util.c
> +++ b/source3/auth/auth_util.c
> @@ -1169,12 +1169,12 @@ NTSTATUS make_session_info_guest(TALLOC_CTX *mem_ctx,
>
> static struct auth_session_info *system_info = NULL;
>
> -NTSTATUS init_system_session_info(void)
> +NTSTATUS init_system_session_info(TALLOC_CTX *mem_ctx)
> {
> if (system_info != NULL)
> return NT_STATUS_OK;
>
> - return make_new_session_info_system(NULL, &system_info);
> + return make_new_session_info_system(mem_ctx, &system_info);
> }
>
> NTSTATUS make_session_info_system(TALLOC_CTX *mem_ctx,
> diff --git a/source3/auth/proto.h b/source3/auth/proto.h
> index 6d6f789d8b6..bdefeaf8ec5 100644
> --- a/source3/auth/proto.h
> +++ b/source3/auth/proto.h
> @@ -241,7 +241,7 @@ NTSTATUS make_session_info_from_username(TALLOC_CTX *mem_ctx,
> struct auth_session_info *copy_session_info(TALLOC_CTX *mem_ctx,
> const struct auth_session_info *src);
> bool init_guest_session_info(TALLOC_CTX *mem_ctx);
> -NTSTATUS init_system_session_info(void);
> +NTSTATUS init_system_session_info(TALLOC_CTX *mem_ctx);
> bool session_info_set_session_key(struct auth_session_info *info,
> DATA_BLOB session_key);
> NTSTATUS make_server_info_guest(TALLOC_CTX *mem_ctx,
> diff --git a/source3/smbd/server.c b/source3/smbd/server.c
> index d80ea7311bd..e7e297f1f18 100644
> --- a/source3/smbd/server.c
> +++ b/source3/smbd/server.c
> @@ -1984,7 +1984,7 @@ extern void build_options(bool screen);
> exit_daemon("ERROR: failed to load share info db.", EACCES);
> }
>
> - status = init_system_session_info();
> + status = init_system_session_info(NULL);
> if (!NT_STATUS_IS_OK(status)) {
> DEBUG(1, ("ERROR: failed to setup system user info: %s.\n",
> nt_errstr(status)));
> diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
> index 6e3df1f18a8..9611f737378 100644
> --- a/source3/winbindd/winbindd.c
> +++ b/source3/winbindd/winbindd.c
> @@ -1768,7 +1768,7 @@ int main(int argc, const char **argv)
> exit(1);
> }
>
> - status = init_system_session_info();
> + status = init_system_session_info(NULL);
> if (!NT_STATUS_IS_OK(status)) {
> exit_daemon("Winbindd failed to setup system user info", map_errno_from_nt_status(status));
> }
> --
> 2.16.1
>
More information about the samba-technical
mailing list