ldb_tdb: Avoid allocation of a DN between the GUID index and the DB lookup
authorAndrew Bartlett <abartlet@samba.org>
Fri, 1 Sep 2017 08:06:15 +0000 (20:06 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Fri, 22 Sep 2017 19:20:24 +0000 (21:20 +0200)
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
lib/ldb/ldb_tdb/ldb_index.c
lib/ldb/ldb_tdb/ldb_search.c
lib/ldb/ldb_tdb/ldb_tdb.c
lib/ldb/ldb_tdb/ldb_tdb.h

index 3ba0a949da1f032c8f7228ad086bee0c61be9ac7..1fc0a082757d0c011180a1d694e62b0588ee4033 100644 (file)
@@ -362,12 +362,15 @@ int ltdb_key_dn_from_idx(struct ldb_module *module,
                                       "values (%u > 1)",
                                       ltdb->cache->GUID_index_attribute,
                                       dn_str, list->count);
+               TALLOC_FREE(list);
                return LDB_ERR_CONSTRAINT_VIOLATION;
        }
 
-       *tdb_key = ltdb_guid_to_key(module, ltdb,
-                                   mem_ctx, &list->dn[0]);
-       if (tdb_key->dptr == NULL) {
+       /* The tdb_key memory is allocated by the caller */
+       ret = ltdb_guid_to_key(module, ltdb,
+                              &list->dn[0], tdb_key);
+
+       if (ret != LDB_SUCCESS) {
                return LDB_ERR_OPERATIONS_ERROR;
        }
 
@@ -1335,7 +1338,11 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
        ldb = ldb_module_get_ctx(ac->module);
 
        for (i = 0; i < dn_list->count; i++) {
-               TDB_DATA tdb_key;
+               uint8_t guid_key[LTDB_GUID_KEY_SIZE];
+               TDB_DATA tdb_key = {
+                       .dptr = guid_key,
+                       .dsize = sizeof(guid_key)
+               };
                int ret;
                bool matched;
 
@@ -1344,17 +1351,20 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
                        return LDB_ERR_OPERATIONS_ERROR;
                }
 
-               tdb_key = ltdb_idx_to_key(ac->module, ltdb,
-                                         ac, &dn_list->dn[i]);
-               if (tdb_key.dptr == NULL) {
-                       return LDB_ERR_OPERATIONS_ERROR;
+               ret = ltdb_idx_to_key(ac->module, ltdb,
+                                     ac, &dn_list->dn[i],
+                                     &tdb_key);
+               if (ret != LDB_SUCCESS) {
+                       return ret;
                }
 
                ret = ltdb_search_key(ac->module, ltdb,
                                      tdb_key, msg,
                                      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
                                      LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
-               TALLOC_FREE(tdb_key.dptr);
+               if (tdb_key.dptr != guid_key) {
+                       TALLOC_FREE(tdb_key.dptr);
+               }
                if (ret == LDB_ERR_NO_SUCH_OBJECT) {
                        /* the record has disappeared? yes, this can happen */
                        talloc_free(msg);
index 07757f86f3b9cf3b6d90e771954a0df444bb8430..0af230f219bc17772818d77969a4cc1fcc92680b 100644 (file)
@@ -285,14 +285,19 @@ int ltdb_search_dn1(struct ldb_module *module, struct ldb_dn *dn, struct ldb_mes
        void *data = ldb_module_get_private(module);
        struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
        int ret;
-       TDB_DATA tdb_key;
-
-       TALLOC_CTX *tdb_key_ctx = talloc_new(msg);
-       if (!tdb_key_ctx) {
-               return ldb_module_oom(module);
-       }
+       uint8_t guid_key[LTDB_GUID_KEY_SIZE];
+       TDB_DATA tdb_key = {
+               .dptr = guid_key,
+               .dsize = sizeof(guid_key)
+       };
+       TALLOC_CTX *tdb_key_ctx = NULL;
 
        if (ltdb->cache->GUID_index_attribute == NULL) {
+               tdb_key_ctx = talloc_new(msg);
+               if (!tdb_key_ctx) {
+                       return ldb_module_oom(module);
+               }
+
                /* form the key */
                tdb_key = ltdb_key_dn(module, tdb_key_ctx, dn);
                if (!tdb_key.dptr) {
@@ -300,6 +305,11 @@ int ltdb_search_dn1(struct ldb_module *module, struct ldb_dn *dn, struct ldb_mes
                        return LDB_ERR_OPERATIONS_ERROR;
                }
        } else if (ldb_dn_is_special(dn)) {
+               tdb_key_ctx = talloc_new(msg);
+               if (!tdb_key_ctx) {
+                       return ldb_module_oom(module);
+               }
+
                /* form the key */
                tdb_key = ltdb_key_dn(module, tdb_key_ctx, dn);
                if (!tdb_key.dptr) {
@@ -307,11 +317,17 @@ int ltdb_search_dn1(struct ldb_module *module, struct ldb_dn *dn, struct ldb_mes
                        return LDB_ERR_OPERATIONS_ERROR;
                }
        } else {
-               /* Look in the index to find the key for this DN */
-               ret = ltdb_key_dn_from_idx(module, ltdb, tdb_key_ctx,
+               /*
+                * Look in the index to find the key for this DN.
+                *
+                * the tdb_key memory is allocated above, msg is just
+                * used for internal memory.
+                *
+                */
+               ret = ltdb_key_dn_from_idx(module, ltdb,
+                                          msg,
                                           dn, &tdb_key);
                if (ret != LDB_SUCCESS) {
-                       TALLOC_FREE(tdb_key_ctx);
                        return ret;
                }
        }
index d2d8ee8b151965faaa988e5f888dda7e992bd875..dc3aa46bfb0a38f55d05fd0cb65ec255aefe8402 100644 (file)
@@ -221,62 +221,58 @@ failed:
        return key;
 }
 
-TDB_DATA ltdb_guid_to_key(struct ldb_module *module,
-                         struct ltdb_private *ltdb,
-                         TALLOC_CTX *mem_ctx,
-                         const struct ldb_val *GUID_val)
+/* The caller is to provide a correctly sized key */
+int ltdb_guid_to_key(struct ldb_module *module,
+                    struct ltdb_private *ltdb,
+                    const struct ldb_val *GUID_val,
+                    TDB_DATA *key)
 {
-       TDB_DATA key;
-       const char *GUID_prefix = "GUID=";
-       const int GUID_prefix_len = strlen(GUID_prefix);
-
-       key.dptr = talloc_size(mem_ctx,
-                              GUID_val->length+GUID_prefix_len);
+       const char *GUID_prefix = LTDB_GUID_KEY_PREFIX;
+       const int GUID_prefix_len = sizeof(LTDB_GUID_KEY_PREFIX) - 1;
 
-       if (key.dptr == NULL) {
-               errno = ENOMEM;
-               key.dptr = NULL;
-               key.dsize = 0;
-               return key;
+       if (key->dsize != (GUID_val->length+GUID_prefix_len)) {
+               return LDB_ERR_OPERATIONS_ERROR;
        }
-       memcpy(key.dptr, "GUID=", GUID_prefix_len);
-       memcpy(&key.dptr[GUID_prefix_len],
-              GUID_val->data, GUID_val->length);
 
-       key.dsize = talloc_get_size(key.dptr);
-       return key;
+       memcpy(key->dptr, GUID_prefix, GUID_prefix_len);
+       memcpy(&key->dptr[GUID_prefix_len],
+              GUID_val->data, GUID_val->length);
+       return LDB_SUCCESS;
 }
 
-TDB_DATA ltdb_idx_to_key(struct ldb_module *module,
-                        struct ltdb_private *ltdb,
-                        TALLOC_CTX *mem_ctx,
-                        const struct ldb_val *idx_val)
+/*
+ * The caller is to provide a correctly sized key, used only in
+ * the GUID index mode
+ */
+int ltdb_idx_to_key(struct ldb_module *module,
+                   struct ltdb_private *ltdb,
+                   TALLOC_CTX *mem_ctx,
+                   const struct ldb_val *idx_val,
+                   TDB_DATA *key)
 {
-       TDB_DATA key;
        struct ldb_context *ldb = ldb_module_get_ctx(module);
        struct ldb_dn *dn;
 
        if (ltdb->cache->GUID_index_attribute != NULL) {
-               return ltdb_guid_to_key(module, ltdb, mem_ctx, idx_val);
+               return ltdb_guid_to_key(module, ltdb,
+                                       idx_val, key);
        }
 
        dn = ldb_dn_from_ldb_val(mem_ctx, ldb, idx_val);
        if (dn == NULL) {
-               errno = EINVAL;
-               key.dptr = NULL;
-               key.dsize = 0;
-               return key;
+               /*
+                * LDB_ERR_INVALID_DN_SYNTAX would just be confusing
+                * to the caller, as this in an invalid index value
+                */
+               return LDB_ERR_OPERATIONS_ERROR;
        }
        /* form the key */
-       key = ltdb_key_dn(module, mem_ctx, dn);
+       *key = ltdb_key_dn(module, mem_ctx, dn);
        TALLOC_FREE(dn);
-       if (!key.dptr) {
-               errno = ENOMEM;
-               key.dptr = NULL;
-               key.dsize = 0;
-               return key;
+       if (!key->dptr) {
+               return ldb_module_oom(module);
        }
-       return key;
+       return LDB_SUCCESS;
 }
 
 /*
@@ -294,6 +290,7 @@ TDB_DATA ltdb_key_msg(struct ldb_module *module, TALLOC_CTX *mem_ctx,
        struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
        TDB_DATA key;
        const struct ldb_val *guid_val;
+       int ret;
 
        if (ltdb->cache->GUID_index_attribute == NULL) {
                return ltdb_key_dn(module, mem_ctx, msg->dn);
@@ -312,8 +309,25 @@ TDB_DATA ltdb_key_msg(struct ldb_module *module, TALLOC_CTX *mem_ctx,
                return key;
        }
 
-       return ltdb_guid_to_key(module, ltdb, mem_ctx, guid_val);
+       /* In this case, allocate with talloc */
+       key.dptr = talloc_size(mem_ctx, LTDB_GUID_KEY_SIZE);
+       if (key.dptr == NULL) {
+               errno = ENOMEM;
+               key.dptr = NULL;
+               key.dsize = 0;
+               return key;
+       }
+       key.dsize = talloc_get_size(key.dptr);
 
+       ret = ltdb_guid_to_key(module, ltdb, guid_val, &key);
+
+       if (ret != LDB_SUCCESS) {
+               errno = EINVAL;
+               key.dptr = NULL;
+               key.dsize = 0;
+               return key;
+       }
+       return key;
 }
 
 /*
index e8fafd263900541a837474546912a360c764b105..7e182495928c7f91ee59443ce2963b0da2fe1111 100644 (file)
@@ -82,6 +82,7 @@ struct ltdb_context {
 /* DB keys */
 #define LTDB_GUID_KEY_PREFIX "GUID="
 #define LTDB_GUID_SIZE 16
+#define LTDB_GUID_KEY_SIZE (LTDB_GUID_SIZE + sizeof(LTDB_GUID_KEY_PREFIX) - 1)
 
 /* The following definitions come from lib/ldb/ldb_tdb/ldb_cache.c  */
 
@@ -153,14 +154,15 @@ TDB_DATA ltdb_key_dn(struct ldb_module *module, TALLOC_CTX *mem_ctx,
                     struct ldb_dn *dn);
 TDB_DATA ltdb_key_msg(struct ldb_module *module, TALLOC_CTX *mem_ctx,
                      const struct ldb_message *msg);
-TDB_DATA ltdb_guid_to_key(struct ldb_module *module,
-                         struct ltdb_private *ltdb,
-                         TALLOC_CTX *mem_ctx,
-                         const struct ldb_val *guid_val);
-TDB_DATA ltdb_idx_to_key(struct ldb_module *module,
-                        struct ltdb_private *ltdb,
-                        TALLOC_CTX *mem_ctx,
-                        const struct ldb_val *idx_val);
+int ltdb_guid_to_key(struct ldb_module *module,
+                    struct ltdb_private *ltdb,
+                    const struct ldb_val *GUID_val,
+                    TDB_DATA *key);
+int ltdb_idx_to_key(struct ldb_module *module,
+                   struct ltdb_private *ltdb,
+                   TALLOC_CTX *mem_ctx,
+                   const struct ldb_val *idx_val,
+                   TDB_DATA *key);
 int ltdb_store(struct ldb_module *module, const struct ldb_message *msg, int flgs);
 int ltdb_modify_internal(struct ldb_module *module, const struct ldb_message *msg, struct ldb_request *req);
 int ltdb_delete_noindex(struct ldb_module *module,