[PATCH] A few cosmetic patches to libcli/security

Volker Lendecke Volker.Lendecke at SerNet.DE
Tue May 27 02:20:18 MDT 2014


On Tue, May 27, 2014 at 09:19:04AM +0200, Volker Lendecke wrote:
> Hi!
> 
> Review & push would be appreciated!

New version with the already-reviewed and pushed one
removed. This does contain a few "bool ret;" ones, but
those were already around.

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 6505907b285a5ac6173b3a6780639dbe927d6127 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 26 May 2014 20:57:31 +0000
Subject: [PATCH 1/5] libcli: Avoid a talloc/free

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/security_token.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libcli/security/security_token.c b/libcli/security/security_token.c
index 40f1382..95a09b3 100644
--- a/libcli/security/security_token.c
+++ b/libcli/security/security_token.c
@@ -84,12 +84,14 @@ bool security_token_is_sid(const struct security_token *token, const struct dom_
 bool security_token_is_sid_string(const struct security_token *token, const char *sid_string)
 {
 	bool ret;
-	struct dom_sid *sid = dom_sid_parse_talloc(NULL, sid_string);
-	if (!sid) return false;
+	struct dom_sid sid;
 
-	ret = security_token_is_sid(token, sid);
+	ret = dom_sid_parse(sid_string, &sid);
+	if (!ret) {
+		return false;
+	}
 
-	talloc_free(sid);
+	ret = security_token_is_sid(token, &sid);
 	return ret;
 }
 
-- 
1.8.1.2


From cc5333bb34d0918167e0b597516f0f85c74d1c4e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 26 May 2014 20:57:31 +0000
Subject: [PATCH 2/5] libcli: Avoid a talloc/free

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/security_token.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libcli/security/security_token.c b/libcli/security/security_token.c
index 95a09b3..6812d42 100644
--- a/libcli/security/security_token.c
+++ b/libcli/security/security_token.c
@@ -119,12 +119,14 @@ bool security_token_has_sid(const struct security_token *token, const struct dom
 bool security_token_has_sid_string(const struct security_token *token, const char *sid_string)
 {
 	bool ret;
-	struct dom_sid *sid = dom_sid_parse_talloc(NULL, sid_string);
-	if (!sid) return false;
+	struct dom_sid sid;
 
-	ret = security_token_has_sid(token, sid);
+	ret = dom_sid_parse(sid_string, &sid);
+	if (!ret) {
+		return false;
+	}
 
-	talloc_free(sid);
+	ret = security_token_has_sid(token, &sid);
 	return ret;
 }
 
-- 
1.8.1.2


From 90c266d200fd50f5108e6fdc1b03f202eb96b3de Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 26 May 2014 21:01:38 +0000
Subject: [PATCH 3/5] libcli: Avoid an explicit memset call

On x86 with -O3, this saves surprising 160 bytes .text

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/util_sid.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
index 9a24a4a..8e42826 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -225,10 +225,10 @@ void sid_copy(struct dom_sid *dst, const struct dom_sid *src)
 {
 	int i;
 
-	ZERO_STRUCTP(dst);
-
-	dst->sid_rev_num = src->sid_rev_num;
-	dst->num_auths = src->num_auths;
+	*dst = (struct dom_sid) {
+		.sid_rev_num = src->sid_rev_num,
+		.num_auths = src->num_auths,
+	};
 
 	memcpy(&dst->id_auth[0], &src->id_auth[0], sizeof(src->id_auth));
 
-- 
1.8.1.2


From fbd9336bdd901b0f1e51fa5afcd22f86dca00382 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 27 May 2014 07:03:18 +0000
Subject: [PATCH 4/5] libcli: Simplify desc_expand_generic()

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/create_descriptor.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/libcli/security/create_descriptor.c b/libcli/security/create_descriptor.c
index 23e7e9b..33c9b10 100644
--- a/libcli/security/create_descriptor.c
+++ b/libcli/security/create_descriptor.c
@@ -125,19 +125,15 @@ static bool desc_ace_has_generic(TALLOC_CTX *mem_ctx,
 
 /* creates an ace in which the generic information is expanded */
 
-static void desc_expand_generic(TALLOC_CTX *mem_ctx,
-				struct security_ace *new_ace,
+static void desc_expand_generic(struct security_ace *new_ace,
 				struct dom_sid *owner,
 				struct dom_sid *group)
 {
-	struct dom_sid *co, *cg;
-	co = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_OWNER);
-	cg = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_GROUP);
 	new_ace->access_mask = map_generic_rights_ds(new_ace->access_mask);
-	if (dom_sid_equal(&new_ace->trustee, co)) {
+	if (dom_sid_equal(&new_ace->trustee, &global_sid_Creator_Owner)) {
 		new_ace->trustee = *owner;
 	}
-	if (dom_sid_equal(&new_ace->trustee, cg)) {
+	if (dom_sid_equal(&new_ace->trustee, &global_sid_Creator_Group)) {
 		new_ace->trustee = *group;
 	}
 	new_ace->flags = 0x0;
@@ -222,8 +218,7 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 						    return NULL;
 					    }
 					    tmp_acl->aces[tmp_acl->num_aces] = *ace;
-					    desc_expand_generic(tmp_ctx,
-								&tmp_acl->aces[tmp_acl->num_aces],
+					    desc_expand_generic(&tmp_acl->aces[tmp_acl->num_aces],
 								owner,
 								group);
 					    tmp_acl->aces[tmp_acl->num_aces].flags = SEC_ACE_FLAG_INHERITED_ACE;
@@ -294,8 +289,7 @@ static struct security_acl *process_user_acl(TALLOC_CTX *mem_ctx,
 		 * and another one where these are translated */
 		if (desc_ace_has_generic(tmp_ctx, ace)) {
 			if (!(ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT)) {
-				desc_expand_generic(tmp_ctx,
-						    &tmp_acl->aces[tmp_acl->num_aces-1],
+				desc_expand_generic(&tmp_acl->aces[tmp_acl->num_aces-1],
 						    owner,
 						    group);
 			} else {
@@ -306,8 +300,7 @@ static struct security_acl *process_user_acl(TALLOC_CTX *mem_ctx,
 							       tmp_acl->num_aces+1);
 				/* add a new ACE with expanded generic info */
 				tmp_acl->aces[tmp_acl->num_aces] = *ace;
-				desc_expand_generic(tmp_ctx,
-						    &tmp_acl->aces[tmp_acl->num_aces],
+				desc_expand_generic(&tmp_acl->aces[tmp_acl->num_aces],
 						    owner,
 						    group);
 				tmp_acl->num_aces++;
-- 
1.8.1.2


From d9a9b8b1a61491b3dda45dd850f5f0e282ab883a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 27 May 2014 07:04:38 +0000
Subject: [PATCH 5/5] libcli: Simplify desc_ace_has_generic()

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/create_descriptor.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/libcli/security/create_descriptor.c b/libcli/security/create_descriptor.c
index 33c9b10..03c0658 100644
--- a/libcli/security/create_descriptor.c
+++ b/libcli/security/create_descriptor.c
@@ -107,17 +107,14 @@ static bool object_in_list(struct GUID *object_list, struct GUID *object)
 /* returns true if the ACE gontains generic information
  * that needs to be processed additionally */
  
-static bool desc_ace_has_generic(TALLOC_CTX *mem_ctx,
-			     struct security_ace *ace)
+static bool desc_ace_has_generic(struct security_ace *ace)
 {
-	struct dom_sid *co, *cg;
-	co = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_OWNER);
-	cg = dom_sid_parse_talloc(mem_ctx,  SID_CREATOR_GROUP);
 	if (ace->access_mask & SEC_GENERIC_ALL || ace->access_mask & SEC_GENERIC_READ ||
 	    ace->access_mask & SEC_GENERIC_WRITE || ace->access_mask & SEC_GENERIC_EXECUTE) {
 		return true;
 	}
-	if (dom_sid_equal(&ace->trustee, co) || dom_sid_equal(&ace->trustee, cg)) {
+	if (dom_sid_equal(&ace->trustee, &global_sid_Creator_Owner) ||
+	    dom_sid_equal(&ace->trustee, &global_sid_Creator_Group)) {
 		return true;
 	}
 	return false;
@@ -175,7 +172,7 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 			tmp_acl->aces[tmp_acl->num_aces].flags |= SEC_ACE_FLAG_INHERITED_ACE;
 			/* remove IO flag from the child's ace */
 			if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY &&
-			    !desc_ace_has_generic(tmp_ctx, ace)) {
+			    !desc_ace_has_generic(ace)) {
 				tmp_acl->aces[tmp_acl->num_aces].flags &= ~SEC_ACE_FLAG_INHERIT_ONLY;
 			}
 
@@ -208,7 +205,7 @@ static struct security_acl *calculate_inherited_from_parent(TALLOC_CTX *mem_ctx,
 			tmp_acl->num_aces++;
 			if (is_container) {
 				if (!(ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) &&
-				    (desc_ace_has_generic(tmp_ctx, ace))) {
+				    (desc_ace_has_generic(ace))) {
 					    tmp_acl->aces = talloc_realloc(tmp_acl,
 									   tmp_acl->aces,
 									   struct security_ace,
@@ -287,7 +284,7 @@ static struct security_acl *process_user_acl(TALLOC_CTX *mem_ctx,
 		/* if the ACE contains CO, CG, GA, GE, GR or GW, and is inheritable
 		 * it has to be expanded to two aces, the original as IO,
 		 * and another one where these are translated */
-		if (desc_ace_has_generic(tmp_ctx, ace)) {
+		if (desc_ace_has_generic(ace)) {
 			if (!(ace->flags & SEC_ACE_FLAG_CONTAINER_INHERIT)) {
 				desc_expand_generic(&tmp_acl->aces[tmp_acl->num_aces-1],
 						    owner,
-- 
1.8.1.2



More information about the samba-technical mailing list