[PATCH] some cleanups
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Mar 11 07:55:21 MDT 2015
Hi!
Review&push appreciated, this just survived a private
autobuild.
Thanks,
Volker
--
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
-------------- next part --------------
From 042db963d5bb36bdfee6141af8a9c1f0202623e4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Mar 2015 10:58:11 +0000
Subject: [PATCH 1/7] ctdb: Fix CID 1288201 Array compared against 0
"helper_prog" is now declared as a static array
Signed-off-by: Volker Lendecke <vl at samba.org>
---
ctdb/server/eventscript.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/ctdb/server/eventscript.c b/ctdb/server/eventscript.c
index d776121..e70db79 100644
--- a/ctdb/server/eventscript.c
+++ b/ctdb/server/eventscript.c
@@ -450,10 +450,6 @@ static void ctdb_run_debug_hung_script(struct ctdb_context *ctdb, struct debug_h
const char **argv;
int i;
- if (helper_prog == NULL) {
- return;
- }
-
if (pipe(fd) < 0) {
DEBUG(DEBUG_ERR,("Failed to create pipe fd for debug hung script\n"));
return;
--
1.9.1
From 5a7772a5c16852c99c2a19649135ad5464333e85 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 9 Mar 2015 21:44:04 +0100
Subject: [PATCH 2/7] lib: Fix whitespace
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/system_smbd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/lib/system_smbd.c b/source3/lib/system_smbd.c
index 10d7f38..5438162 100644
--- a/source3/lib/system_smbd.c
+++ b/source3/lib/system_smbd.c
@@ -222,7 +222,7 @@ bool getgroups_unix_user(TALLOC_CTX *mem_ctx, const char *user,
if (!temp_groups) {
return False;
}
-
+
if (sys_getgrouplist(user, primary_gid,
temp_groups, &max_grp) == -1) {
DEBUG(0, ("get_user_groups: failed to get the unix "
@@ -231,7 +231,7 @@ bool getgroups_unix_user(TALLOC_CTX *mem_ctx, const char *user,
return False;
}
}
-
+
ngrp = 0;
groups = NULL;
--
1.9.1
From 8767c0ff49dc7230f5a72ba7ae9ed3598f636d4c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 9 Mar 2015 22:04:04 +0100
Subject: [PATCH 3/7] lib: Avoid a malloc/realloc in getgroups_unix_user
This avoids a malloc/free in the most common case of a user with just a few
group memberships
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/system_smbd.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/source3/lib/system_smbd.c b/source3/lib/system_smbd.c
index 5438162..3b1ac9c 100644
--- a/source3/lib/system_smbd.c
+++ b/source3/lib/system_smbd.c
@@ -205,29 +205,26 @@ bool getgroups_unix_user(TALLOC_CTX *mem_ctx, const char *user,
gid_t primary_gid,
gid_t **ret_groups, uint32_t *p_ngroups)
{
+ int max_grp = MIN(128, groups_max());
+ gid_t stack_groups[max_grp];
uint32_t ngrp;
- int max_grp;
- gid_t *temp_groups;
+ gid_t *temp_groups = stack_groups;
+ gid_t *to_free = NULL;
gid_t *groups;
int i;
- max_grp = MIN(128, groups_max());
- temp_groups = SMB_MALLOC_ARRAY(gid_t, max_grp);
- if (! temp_groups) {
- return False;
- }
-
if (sys_getgrouplist(user, primary_gid, temp_groups, &max_grp) == -1) {
- temp_groups = SMB_REALLOC_ARRAY(temp_groups, gid_t, max_grp);
- if (!temp_groups) {
+ to_free = talloc_array(mem_ctx, gid_t, max_grp);
+ if (!to_free) {
return False;
}
+ temp_groups = to_free;
if (sys_getgrouplist(user, primary_gid,
temp_groups, &max_grp) == -1) {
DEBUG(0, ("get_user_groups: failed to get the unix "
"group list\n"));
- SAFE_FREE(temp_groups);
+ TALLOC_FREE(to_free);
return False;
}
}
@@ -237,20 +234,20 @@ bool getgroups_unix_user(TALLOC_CTX *mem_ctx, const char *user,
/* Add in primary group first */
if (!add_gid_to_array_unique(mem_ctx, primary_gid, &groups, &ngrp)) {
- SAFE_FREE(temp_groups);
+ TALLOC_FREE(to_free);
return False;
}
for (i=0; i<max_grp; i++) {
if (!add_gid_to_array_unique(mem_ctx, temp_groups[i],
&groups, &ngrp)) {
- SAFE_FREE(temp_groups);
+ TALLOC_FREE(to_free);
return False;
}
}
*p_ngroups = ngrp;
*ret_groups = groups;
- SAFE_FREE(temp_groups);
+ TALLOC_FREE(to_free);
return True;
}
--
1.9.1
From 201a18bee301039e36342252a62ad2e41a7edfa2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 10 Mar 2015 20:55:22 +0100
Subject: [PATCH 4/7] smbd: Put a variable definition closer to its use
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/auth/token_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/auth/token_util.c b/source3/auth/token_util.c
index 9bb014c..08be9f7 100644
--- a/source3/auth/token_util.c
+++ b/source3/auth/token_util.c
@@ -672,7 +672,6 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
TALLOC_CTX *tmp_ctx = talloc_stackframe();
gid_t *gids;
struct dom_sid *group_sids;
- struct dom_sid unix_group_sid;
uint32_t num_group_sids;
uint32_t num_gids;
uint32_t i;
@@ -872,6 +871,7 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
num_gids = num_group_sids;
range_ok = lp_idmap_default_range(&low, &high);
for ( i=0; i<num_gids; i++ ) {
+ struct dom_sid unix_group_sid;
/* don't pickup anything managed by Winbind */
if (range_ok && (gids[i] >= low) && (gids[i] <= high)) {
--
1.9.1
From f191c751ebad89d89603c91f04dd43bbea328708 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 10 Mar 2015 21:03:15 +0100
Subject: [PATCH 5/7] smbd: Streamline the gids handling in
create_token_from_sid()
Usually, I'm all for avoiding talloc. But in this case I believe that this
routine is complex enough to justify this change. For an hour or so I suspect
that the winbind case had an uninitialized "*gid" until I discovered the
sid_to_gid(). This makes it more obvious that *gid is assigned.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/auth/token_util.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/source3/auth/token_util.c b/source3/auth/token_util.c
index 08be9f7..bb87be3 100644
--- a/source3/auth/token_util.c
+++ b/source3/auth/token_util.c
@@ -846,17 +846,23 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
goto done;
}
+ gids = talloc_array(tmp_ctx, gid_t, num_group_sids);
+ if (gids == NULL) {
+ result = NT_STATUS_NO_MEMORY;
+ goto done;
+ }
+
sid_copy(&group_sids[0], user_sid);
sid_split_rid(&group_sids[0], NULL);
sid_append_rid(&group_sids[0], DOMAIN_RID_USERS);
- if (!sid_to_gid(&group_sids[0], gid)) {
+ if (!sid_to_gid(&group_sids[0], &gids[0])) {
DEBUG(1, ("sid_to_gid(%s) failed\n",
sid_string_dbg(&group_sids[0])));
goto done;
}
- gids = gid;
+ *gid = gids[0];
*found_username = NULL;
}
--
1.9.1
From bb4c2485900fa3f551b5c855ce635169ea0761a1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 10 Mar 2015 21:09:53 +0100
Subject: [PATCH 6/7] smbd: Simplify create_token_from_sid()
With the previous commit all 3 branches do the same
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/auth/token_util.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/source3/auth/token_util.c b/source3/auth/token_util.c
index bb87be3..6129cad 100644
--- a/source3/auth/token_util.c
+++ b/source3/auth/token_util.c
@@ -716,8 +716,6 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
/* see the smb_panic() in pdb_default_enum_group_memberships */
SMB_ASSERT(num_group_sids > 0);
- *gid = gids[0];
-
/* Ensure we're returning the found_username on the right context. */
*found_username = talloc_strdup(mem_ctx,
pdb_get_username(sam_acct));
@@ -813,8 +811,6 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
/* In getgroups_unix_user we always set the primary gid */
SMB_ASSERT(num_group_sids > 0);
- *gid = gids[0];
-
/* Ensure we're returning the found_username on the right context. */
*found_username = talloc_strdup(mem_ctx, pass->pw_name);
if (*found_username == NULL) {
@@ -862,11 +858,11 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
goto done;
}
- *gid = gids[0];
-
*found_username = NULL;
}
+ *gid = gids[0];
+
/* Add the "Unix Group" SID for each gid to catch mapped groups
and their Unix equivalent. This is to solve the backwards
compatibility problem of 'valid users = +ntadmin' where
--
1.9.1
From 96e0d32c30242d4cf5d83f020cbcc50ba53244e5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 10 Mar 2015 21:13:56 +0100
Subject: [PATCH 7/7] smbd: Simplify create_token_from_sid()
This if-statement is unnecessary. First, talloc_array returns non-NULL
even if asked for 0 elements. Second, a bit further down we do a
SMB_ASSERT(num_group_sids > 0);
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/auth/token_util.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/source3/auth/token_util.c b/source3/auth/token_util.c
index 6129cad..1f95be6 100644
--- a/source3/auth/token_util.c
+++ b/source3/auth/token_util.c
@@ -793,15 +793,11 @@ static NTSTATUS create_token_from_sid(TALLOC_CTX *mem_ctx,
}
num_group_sids = getgroups_num_group_sids;
- if (num_group_sids) {
- group_sids = talloc_array(tmp_ctx, struct dom_sid, num_group_sids);
- if (group_sids == NULL) {
- DEBUG(1, ("talloc_array failed\n"));
- result = NT_STATUS_NO_MEMORY;
- goto done;
- }
- } else {
- group_sids = NULL;
+ group_sids = talloc_array(tmp_ctx, struct dom_sid, num_group_sids);
+ if (group_sids == NULL) {
+ DEBUG(1, ("talloc_array failed\n"));
+ result = NT_STATUS_NO_MEMORY;
+ goto done;
}
for (i=0; i<num_group_sids; i++) {
--
1.9.1
More information about the samba-technical
mailing list