libcli/security: handle node initialisation in one spot in insert_in_object_tree()

This removes special-case for initalising the children array in
insert_in_object_tree().  talloc_realloc() handles the intial allocate
case perfectly well, so there is no need to have this duplicated.

This also restores having just one place were the rest of the elements
are intialised, to ensure uniform behaviour.

To do this, we have to rework insert_in_object_tree to have only one
output variable, both because having both root and new_node as output
variables was too confusing, and because otherwise the two pointers
were being allowed to point at the same memory.

Andrew Bartlett

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/libcli/security/access_check.h b/libcli/security/access_check.h
index 84b2e5fee99b1c29dbdb9b7c5a2195f0963c00ed..952589dacb7794fbf8d934dcb8638d6177d3fce9 100644 (file)
--- a/libcli/security/access_check.h
+++ b/libcli/security/access_check.h
@@ -77,10 +77,10 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
                             struct dom_sid *replace_sid);
 
 bool insert_in_object_tree(TALLOC_CTX *mem_ctx,
-                         const struct GUID *guid,
-                         uint32_t init_access,
-                         struct object_tree **root,
-                          struct object_tree **new_node);
+                          const struct GUID *guid,
+                          uint32_t init_access,
+                          struct object_tree *root,
+                          struct object_tree **new_node_out);
 
 /* search by GUID */
 struct object_tree *get_object_tree_by_GUID(struct object_tree *root,

diff --git a/libcli/security/object_tree.c b/libcli/security/object_tree.c
index dcbd310baeaad7e7c12a89445fe12a1c125bc9a8..a629177936dbb6cb4410e7441e5b305b0bb50cae 100644 (file)
--- a/libcli/security/object_tree.c
+++ b/libcli/security/object_tree.c
@@ -38,52 +38,51 @@
  */
 
 bool insert_in_object_tree(TALLOC_CTX *mem_ctx,
-                         const struct GUID *guid,
-                         uint32_t init_access,
-                         struct object_tree **root,
-                         struct object_tree **new_node)
+                          const struct GUID *guid,
+                          uint32_t init_access,
+                          struct object_tree *root,
+                          struct object_tree **new_node_out)
 {
+       struct object_tree *new_node;
+
        if (!guid || GUID_all_zero(guid)){
                return true;
        }
 
-       if (!*root){
-               *root = talloc_zero(mem_ctx, struct object_tree);
-               if (!*root) {
+       if (!root) {
+               root = talloc_zero(mem_ctx, struct object_tree);
+               if (!root) {
                        return false;
                }
-               (*root)->guid = *guid;
-               (*root)->remaining_access = init_access;
-               *new_node = *root;
-               return true;
-       }
-
-       if (!(*root)->children) {
-               (*root)->children = talloc_array(mem_ctx, struct object_tree, 1);
-               (*root)->children[0].guid = *guid;
-               (*root)->children[0].num_of_children = 0;
-               (*root)->children[0].children = NULL;
-               (*root)->num_of_children++;
-               (*root)->children[0].remaining_access = init_access;
-               *new_node = &((*root)->children[0]);
-               return true;
-       }
-       else {
+               new_node = root;
+       } else {
                int i;
-               for (i = 0; i < (*root)->num_of_children; i++) {
-                       if (GUID_equal(&((*root)->children[i].guid), guid)) {
-                               *new_node = &((*root)->children[i]);
+
+               for (i = 0; i < root->num_of_children; i++) {
+                       if (GUID_equal(&root->children[i].guid, guid)) {
+                               new_node = &root->children[i];
+                               *new_node_out = new_node;
                                return true;
                        }
                }
-               (*root)->children = talloc_realloc(mem_ctx, (*root)->children, struct object_tree,
-                                                  (*root)->num_of_children +1);
-               (*root)->children[(*root)->num_of_children].guid = *guid;
-               (*root)->children[(*root)->num_of_children].remaining_access = init_access;
-               *new_node = &((*root)->children[(*root)->num_of_children]);
-               (*root)->num_of_children++;
-               return true;
+
+               root->children = talloc_realloc(mem_ctx, root->children,
+                                               struct object_tree,
+                                               root->num_of_children + 1);
+               if (!root->children) {
+                       return false;
+               }
+               new_node = &root->children[root->num_of_children];
+               root->num_of_children++;
        }
+
+       new_node->children = NULL;
+       new_node->guid = *guid;
+       new_node->remaining_access = init_access;
+       new_node->num_of_children = 0;
+
+       *new_node_out = new_node;
+       return true;
 }
 
 /* search by GUID */

diff --git a/source4/dsdb/common/dsdb_access.c b/source4/dsdb/common/dsdb_access.c
index fd75e77541e4644c6586ffefaefc0849ccfd0739..6af5c3ae879ad927b6592fbcbbfae25c557ea198 100644 (file)
--- a/source4/dsdb/common/dsdb_access.c
+++ b/source4/dsdb/common/dsdb_access.c
@@ -93,7 +93,6 @@ int dsdb_check_access_on_dn_internal(struct ldb_context *ldb,
        struct security_descriptor *sd = NULL;
        struct dom_sid *sid = NULL;
        struct object_tree *root = NULL;
-       struct object_tree *new_node = NULL;
        NTSTATUS status;
        uint32_t access_granted;
        int ret;
@@ -108,8 +107,8 @@ int dsdb_check_access_on_dn_internal(struct ldb_context *ldb,
        }
        sid = samdb_result_dom_sid(mem_ctx, acl_res->msgs[0], "objectSid");
        if (guid) {
-               if (!insert_in_object_tree(mem_ctx, guid, access_mask, &root,
-                                          &new_node)) {
+               if (!insert_in_object_tree(mem_ctx, guid, access_mask, NULL,
+                                          &root)) {
                        return ldb_operr(ldb);
                }
        }

diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
index 09ca201d949fb7a8d37a3b2e2a2778e5b21cd2c3..795a39cba4d7c75ac3251ebac26313b76b0070ac 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
@@ -109,16 +109,17 @@ int acl_check_access_on_attribute(struct ldb_module *module,
 
        if (!insert_in_object_tree(tmp_ctx,
                                   &objectclass->schemaIDGUID,
-                                  access_mask, &root,
-                                  &new_node)) {
+                                  access_mask, NULL,
+                                  &root)) {
                DEBUG(10, ("acl_search: cannot add to object tree class schemaIDGUID\n"));
                goto fail;
        }
+       new_node = root;
 
        if (!GUID_all_zero(&attr->attributeSecurityGUID)) {
                if (!insert_in_object_tree(tmp_ctx,
                                           &attr->attributeSecurityGUID,
-                                          access_mask, &new_node,
+                                          access_mask, new_node,
                                           &new_node)) {
                        DEBUG(10, ("acl_search: cannot add to object tree securityGUID\n"));
                        goto fail;
@@ -127,7 +128,7 @@ int acl_check_access_on_attribute(struct ldb_module *module,
 
        if (!insert_in_object_tree(tmp_ctx,
                                   &attr->schemaIDGUID,
-                                  access_mask, &new_node,
+                                  access_mask, new_node,
                                   &new_node)) {
                DEBUG(10, ("acl_search: cannot add to object tree attributeGUID\n"));
                goto fail;
@@ -162,14 +163,13 @@ int acl_check_access_on_objectclass(struct ldb_module *module,
        NTSTATUS status;
        uint32_t access_granted;
        struct object_tree *root = NULL;
-       struct object_tree *new_node = NULL;
        TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
        struct security_token *token = acl_user_token(module);
 
        if (!insert_in_object_tree(tmp_ctx,
                                   &objectclass->schemaIDGUID,
-                                  access_mask, &root,
-                                  &new_node)) {
+                                  access_mask, NULL,
+                                  &root)) {
                DEBUG(10, ("acl_search: cannot add to object tree class schemaIDGUID\n"));
                goto fail;
        }
@@ -209,7 +209,7 @@ int acl_check_extended_right(TALLOC_CTX *mem_ctx,
        GUID_from_string(ext_right, &right);
 
        if (!insert_in_object_tree(tmp_ctx, &right, right_type,
-                                  &root, &new_node)) {
+                                  NULL, &root)) {
                DEBUG(10, ("acl_ext_right: cannot add to object tree\n"));
                talloc_free(tmp_ctx);
                return LDB_ERR_OPERATIONS_ERROR;