[PATCH] Two CIDs and a few cleanups
Jeremy Allison
jra at samba.org
Fri Oct 27 18:18:55 UTC 2017
On Fri, Oct 27, 2017 at 08:08:36AM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
>
> Review appreciated!
RB+. Really nice cleanups. I especially like the smb2_negprot
fixup, really elegant !
Pushed.
Jeremy.
> --
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de
> From 8694444c73a8d6d13c2b5772bd701e2b217c4579 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 26 Oct 2017 21:08:14 +0200
> Subject: [PATCH 1/7] samba: Fix CID 1420180 Null pointer dereferences
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source4/smbd/process_prefork.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source4/smbd/process_prefork.c b/source4/smbd/process_prefork.c
> index 8e4d62470a7..a4bd7ba74b7 100644
> --- a/source4/smbd/process_prefork.c
> +++ b/source4/smbd/process_prefork.c
> @@ -318,7 +318,7 @@ static void prefork_new_task(
> struct tfork* w = NULL;
>
> w = tfork_create();
> - if (t == NULL) {
> + if (w == NULL) {
> smb_panic("failure in tfork\n");
> }
>
> --
> 2.11.0
>
>
> From 7e8bb71671965bc12ea4df5209b8dfd283036a1f Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 26 Oct 2017 21:13:52 +0200
> Subject: [PATCH 2/7] samba: Fix CID 1420179 Code maintainability issues
> UNUSED_VALUE
>
> I don't think pid is used at all here.
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source4/smbd/process_prefork.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/source4/smbd/process_prefork.c b/source4/smbd/process_prefork.c
> index a4bd7ba74b7..f6fb80b986d 100644
> --- a/source4/smbd/process_prefork.c
> +++ b/source4/smbd/process_prefork.c
> @@ -339,7 +339,6 @@ static void prefork_new_task(
> free(w);
>
> TALLOC_FREE(ev);
> - pid = getpid();
> setproctitle("task[%s] pre-forked worker",
> service_name);
> prefork_reload_after_fork();
> --
> 2.11.0
>
>
> From f59dce6e4e30a533950bdd8a56d96d30536cdf83 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 19 Oct 2017 08:13:59 +0200
> Subject: [PATCH 3/7] smbd: Remove an indentation level in smb2_negprot
>
> Do an early return. Best viewed with "git show -b"
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/smb2_negprot.c | 147 ++++++++++++++++++++++----------------------
> 1 file changed, 74 insertions(+), 73 deletions(-)
>
> diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
> index d9ccdbeea8e..f8133b239af 100644
> --- a/source3/smbd/smb2_negprot.c
> +++ b/source3/smbd/smb2_negprot.c
> @@ -128,6 +128,7 @@ enum protocol_types smbd_smb2_protocol_dialect_match(const uint8_t *indyn,
> NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
> {
> struct smbXsrv_connection *xconn = req->xconn;
> + struct smbXsrv_client_global0 *global0 = NULL;
> NTSTATUS status;
> const uint8_t *inbody;
> const uint8_t *indyn = NULL;
> @@ -612,91 +613,91 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
>
> req->sconn->using_smb2 = true;
>
> - if (dialect != SMB2_DIALECT_REVISION_2FF) {
> - struct smbXsrv_client_global0 *global0 = NULL;
> -
> - status = smbXsrv_connection_init_tables(xconn, protocol);
> - if (!NT_STATUS_IS_OK(status)) {
> - return smbd_smb2_request_error(req, status);
> - }
> + if (dialect == SMB2_DIALECT_REVISION_2FF) {
> + return smbd_smb2_request_done(req, outbody, &outdyn);
> + }
>
> - xconn->smb2.client.capabilities = in_capabilities;
> - xconn->smb2.client.security_mode = in_security_mode;
> - xconn->smb2.client.guid = in_guid;
> - xconn->smb2.client.num_dialects = dialect_count;
> - xconn->smb2.client.dialects = talloc_array(xconn,
> - uint16_t,
> - dialect_count);
> - if (xconn->smb2.client.dialects == NULL) {
> - return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
> - }
> - for (c=0; c < dialect_count; c++) {
> - xconn->smb2.client.dialects[c] = SVAL(indyn, c*2);
> - }
> + status = smbXsrv_connection_init_tables(xconn, protocol);
> + if (!NT_STATUS_IS_OK(status)) {
> + return smbd_smb2_request_error(req, status);
> + }
>
> - xconn->smb2.server.capabilities = capabilities;
> - xconn->smb2.server.security_mode = security_mode;
> - xconn->smb2.server.guid = out_guid;
> - xconn->smb2.server.dialect = dialect;
> - xconn->smb2.server.max_trans = max_trans;
> - xconn->smb2.server.max_read = max_read;
> - xconn->smb2.server.max_write = max_write;
> + xconn->smb2.client.capabilities = in_capabilities;
> + xconn->smb2.client.security_mode = in_security_mode;
> + xconn->smb2.client.guid = in_guid;
> + xconn->smb2.client.num_dialects = dialect_count;
> + xconn->smb2.client.dialects = talloc_array(xconn,
> + uint16_t,
> + dialect_count);
> + if (xconn->smb2.client.dialects == NULL) {
> + return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
> + }
> + for (c=0; c < dialect_count; c++) {
> + xconn->smb2.client.dialects[c] = SVAL(indyn, c*2);
> + }
>
> - if (xconn->protocol < PROTOCOL_SMB2_10) {
> - /*
> - * SMB2_02 doesn't support client guids
> - */
> - return smbd_smb2_request_done(req, outbody, &outdyn);
> - }
> + xconn->smb2.server.capabilities = capabilities;
> + xconn->smb2.server.security_mode = security_mode;
> + xconn->smb2.server.guid = out_guid;
> + xconn->smb2.server.dialect = dialect;
> + xconn->smb2.server.max_trans = max_trans;
> + xconn->smb2.server.max_read = max_read;
> + xconn->smb2.server.max_write = max_write;
>
> - if (!xconn->client->server_multi_channel_enabled) {
> - /*
> - * Only deal with the client guid database
> - * if multi-channel is enabled.
> - */
> - return smbd_smb2_request_done(req, outbody, &outdyn);
> - }
> + if (xconn->protocol < PROTOCOL_SMB2_10) {
> + /*
> + * SMB2_02 doesn't support client guids
> + */
> + return smbd_smb2_request_done(req, outbody, &outdyn);
> + }
>
> - if (xconn->smb2.client.guid_verified) {
> - /*
> - * The connection was passed from another
> - * smbd process.
> - */
> - return smbd_smb2_request_done(req, outbody, &outdyn);
> - }
> + if (!xconn->client->server_multi_channel_enabled) {
> + /*
> + * Only deal with the client guid database
> + * if multi-channel is enabled.
> + */
> + return smbd_smb2_request_done(req, outbody, &outdyn);
> + }
>
> - status = smb2srv_client_lookup_global(xconn->client,
> - xconn->smb2.client.guid,
> - req, &global0);
> + if (xconn->smb2.client.guid_verified) {
> /*
> - * TODO: check for races...
> + * The connection was passed from another
> + * smbd process.
> */
> - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECTID_NOT_FOUND)) {
> - /*
> - * This stores the new client information in
> - * smbXsrv_client_global.tdb
> - */
> - xconn->client->global->client_guid =
> - xconn->smb2.client.guid;
> - status = smbXsrv_client_update(xconn->client);
> - if (!NT_STATUS_IS_OK(status)) {
> - return status;
> - }
> + return smbd_smb2_request_done(req, outbody, &outdyn);
> + }
>
> - xconn->smb2.client.guid_verified = true;
> - } else if (NT_STATUS_IS_OK(status)) {
> - status = smb2srv_client_connection_pass(req,
> - global0);
> - if (!NT_STATUS_IS_OK(status)) {
> - return smbd_smb2_request_error(req, status);
> - }
> + status = smb2srv_client_lookup_global(xconn->client,
> + xconn->smb2.client.guid,
> + req, &global0);
> + /*
> + * TODO: check for races...
> + */
> + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECTID_NOT_FOUND)) {
> + /*
> + * This stores the new client information in
> + * smbXsrv_client_global.tdb
> + */
> + xconn->client->global->client_guid =
> + xconn->smb2.client.guid;
> + status = smbXsrv_client_update(xconn->client);
> + if (!NT_STATUS_IS_OK(status)) {
> + return status;
> + }
>
> - smbd_server_connection_terminate(xconn,
> - "passed connection");
> - return NT_STATUS_OBJECTID_EXISTS;
> - } else {
> + xconn->smb2.client.guid_verified = true;
> + } else if (NT_STATUS_IS_OK(status)) {
> + status = smb2srv_client_connection_pass(req,
> + global0);
> + if (!NT_STATUS_IS_OK(status)) {
> return smbd_smb2_request_error(req, status);
> }
> +
> + smbd_server_connection_terminate(xconn,
> + "passed connection");
> + return NT_STATUS_OBJECTID_EXISTS;
> + } else {
> + return smbd_smb2_request_error(req, status);
> }
>
> return smbd_smb2_request_done(req, outbody, &outdyn);
> --
> 2.11.0
>
>
> From d04b148dfcf40a95dadd721b874831f18fdb7aa5 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 19 Oct 2017 17:52:31 +0200
> Subject: [PATCH 4/7] lib: Use all_zero where appropriate
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> lib/util/util.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/util/util.c b/lib/util/util.c
> index ef148e98d29..4291bfa5d57 100644
> --- a/lib/util/util.c
> +++ b/lib/util/util.c
> @@ -434,7 +434,6 @@ void dump_data_cb(const uint8_t *buf, int len,
> void *private_data)
> {
> int i=0;
> - static const uint8_t empty[16] = { 0, };
> bool skipped = false;
> char tmp[16];
>
> @@ -446,7 +445,7 @@ void dump_data_cb(const uint8_t *buf, int len,
> if ((omit_zero_bytes == true) &&
> (i > 0) &&
> (len > i+16) &&
> - (memcmp(&buf[i], &empty, 16) == 0))
> + all_zero(&buf[i], 16))
> {
> i +=16;
> continue;
> @@ -473,7 +472,7 @@ void dump_data_cb(const uint8_t *buf, int len,
>
> if ((omit_zero_bytes == true) &&
> (len > i+16) &&
> - (memcmp(&buf[i], &empty, 16) == 0)) {
> + all_zero(&buf[i], 16)) {
> if (!skipped) {
> cb("skipping zero buffer bytes\n",
> private_data);
> --
> 2.11.0
>
>
> From f3ea878adf107c95ea8e7ec1b67de55589f45503 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 4 Oct 2017 15:04:01 +0200
> Subject: [PATCH 5/7] smbd: Fix the memory hierarchy in the unix token
>
> "groups" should hang off the token itself, not its parent
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/auth/auth_util.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
> index 1021f2a6fef..8e7fa914748 100644
> --- a/source3/auth/auth_util.c
> +++ b/source3/auth/auth_util.c
> @@ -639,7 +639,8 @@ NTSTATUS create_local_token(TALLOC_CTX *mem_ctx,
> sid_string_dbg(&t->sids[i])));
> continue;
> }
> - if (!add_gid_to_array_unique(session_info, ids[i].id,
> + if (!add_gid_to_array_unique(session_info->unix_token,
> + ids[i].id,
> &session_info->unix_token->groups,
> &session_info->unix_token->ngroups)) {
> return NT_STATUS_NO_MEMORY;
> --
> 2.11.0
>
>
> From d5ca260c2d9ccfaf455f3d9c64e1bb9911a268fd Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 4 Oct 2017 13:27:43 +0200
> Subject: [PATCH 6/7] printing: Avoid an "extern current_user"
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/printing/print_generic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/source3/printing/print_generic.c b/source3/printing/print_generic.c
> index d77fb21e3fd..b6b50062caf 100644
> --- a/source3/printing/print_generic.c
> +++ b/source3/printing/print_generic.c
> @@ -19,8 +19,8 @@
>
> #include "includes.h"
> #include "printing.h"
> +#include "smbd/proto.h"
>
> -extern struct current_user current_user;
> extern userdom_struct current_user_info;
>
> /****************************************************************************
> @@ -76,7 +76,7 @@ static int print_run_command(int snum, const char* printername, bool do_sub,
> lp_servicename(talloc_tos(), snum),
> current_user_info.unix_name,
> "",
> - current_user.ut.gid,
> + get_current_gid(NULL),
> get_current_username(),
> current_user_info.domain,
> syscmd);
> --
> 2.11.0
>
>
> From 630b95c9301c2e4676dd8b234b0202a39df00b81 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 4 Oct 2017 13:30:13 +0200
> Subject: [PATCH 7/7] printing: Avoid an "extern current_user"
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/printing/printing.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/source3/printing/printing.c b/source3/printing/printing.c
> index c6c42f3b0b1..e4bb1d84f69 100644
> --- a/source3/printing/printing.c
> +++ b/source3/printing/printing.c
> @@ -38,7 +38,6 @@
> #include "lib/param/loadparm.h"
> #include "lib/util/sys_rw_data.h"
>
> -extern struct current_user current_user;
> extern userdom_struct current_user_info;
>
> /* Current printer interface */
> @@ -1722,7 +1721,7 @@ static void print_queue_update(struct messaging_context *msg_ctx,
> lp_servicename(talloc_tos(), snum),
> current_user_info.unix_name,
> "",
> - current_user.ut.gid,
> + get_current_gid(NULL),
> get_current_username(),
> current_user_info.domain,
> lpqcommand);
> @@ -1742,7 +1741,7 @@ static void print_queue_update(struct messaging_context *msg_ctx,
> lp_servicename(talloc_tos(), snum),
> current_user_info.unix_name,
> "",
> - current_user.ut.gid,
> + get_current_gid(NULL),
> get_current_username(),
> current_user_info.domain,
> lprmcommand);
> @@ -3026,7 +3025,7 @@ NTSTATUS print_job_end(struct messaging_context *msg_ctx, int snum,
> lp_servicename(talloc_tos(), snum),
> current_user_info.unix_name,
> "",
> - current_user.ut.gid,
> + get_current_gid(NULL),
> get_current_username(),
> current_user_info.domain,
> lpq_cmd);
> --
> 2.11.0
>
More information about the samba-technical
mailing list