[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Tue Aug 27 01:17:03 UTC 2019
The branch, master has been updated
via 1521a22f436 ldb: Call TALLOC_FREE(filtered_msg->elements) on ldb_filter_attrs() failure
via 2117789c35f ldb: use TALLOC_FREE() over talloc_free() in ldb_filter_attrs()
via b1eec5b196e ldb: Correct Pigeonhole principle validation in ldb_filter_attrs()
via 41aaeaf1fe9 ldb tests: Fix ldb_lmdb_size_test
from 085e179dcb6 ctdb-tests: fix mem leak in ltdb_fetch
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 1521a22f4366c86ec955cb9d32b7a758315d8ce0
Author: Andrew Bartlett <abartlet at samba.org>
Date: Mon Aug 26 14:50:15 2019 +1200
ldb: Call TALLOC_FREE(filtered_msg->elements) on ldb_filter_attrs() failure
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Tue Aug 27 01:16:33 UTC 2019 on sn-devel-184
commit 2117789c35fbf6d0ed02f391f17593e11727ec3e
Author: Andrew Bartlett <abartlet at samba.org>
Date: Mon Aug 26 14:48:52 2019 +1200
ldb: use TALLOC_FREE() over talloc_free() in ldb_filter_attrs()
This is a macro that sets the pointer to NULL after the talloc_free()
and is part of our standard coding practices.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
commit b1eec5b196e3d5a5716a5c74cf669ceaa5c0301f
Author: Andrew Bartlett <abartlet at samba.org>
Date: Mon Aug 26 13:04:07 2019 +1200
ldb: Correct Pigeonhole principle validation in ldb_filter_attrs()
Thankfully this only fails if the DB is corrupt and has a duplicate record.
The test was at the wrong end of the loop, and was for the
wrong boundary condition. A write after the end of the array would
occour before the condition was hit.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13695
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
commit 41aaeaf1fe921311f183d5c08ff121a9ccc4c5b4
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Mon Aug 26 16:34:29 2019 +1200
ldb tests: Fix ldb_lmdb_size_test
Fix the lmdb size test which ensures that databases > 4GiB can be
written by the lmdb backend. This test is not run as part of the normal
CI run as it exhausts the available disk on the test runners.
It was broken by changes to LDB allowing the lmdb map size to be
specified, and requiring GUID indexing by default.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
-----------------------------------------------------------------------
Summary of changes:
lib/ldb/common/ldb_pack.c | 35 +-
lib/ldb/tests/ldb_filter_attrs_test.c | 988 ++++++++++++++++++++++++++++++++++
lib/ldb/tests/ldb_lmdb_size_test.c | 40 +-
lib/ldb/wscript | 5 +
4 files changed, 1056 insertions(+), 12 deletions(-)
create mode 100644 lib/ldb/tests/ldb_filter_attrs_test.c
Changeset truncated at 500 lines:
diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index 9d87a10b9f1..e7dd364008a 100644
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -1163,7 +1163,10 @@ int ldb_filter_attrs(struct ldb_context *ldb,
} else if (i == 0) {
return 0;
- /* Otherwise we are copying at most as many element as we have attributes */
+ /*
+ * Otherwise we are copying at most as many elements as we
+ * have attributes
+ */
} else {
elements_size = i;
}
@@ -1177,7 +1180,12 @@ int ldb_filter_attrs(struct ldb_context *ldb,
for (i = 0; i < msg->num_elements; i++) {
struct ldb_message_element *el = &msg->elements[i];
- struct ldb_message_element *el2 = &filtered_msg->elements[num_elements];
+
+ /*
+ * el2 is assigned after the Pigeonhole principle
+ * check below for clarity
+ */
+ struct ldb_message_element *el2 = NULL;
unsigned int j;
if (keep_all == false) {
@@ -1193,6 +1201,18 @@ int ldb_filter_attrs(struct ldb_context *ldb,
continue;
}
}
+
+ /*
+ * Pigeonhole principle: we can't have more elements
+ * than the number of attributes if they are unique in
+ * the DB.
+ */
+ if (num_elements >= elements_size) {
+ goto failed;
+ }
+
+ el2 = &filtered_msg->elements[num_elements];
+
*el2 = *el;
el2->name = talloc_strdup(filtered_msg->elements,
el->name);
@@ -1211,13 +1231,6 @@ int ldb_filter_attrs(struct ldb_context *ldb,
}
}
num_elements++;
-
- /* Pidginhole principle: we can't have more elements
- * than the number of attributes if they are unique in
- * the DB */
- if (num_elements > elements_size) {
- goto failed;
- }
}
filtered_msg->num_elements = num_elements;
@@ -1238,11 +1251,11 @@ int ldb_filter_attrs(struct ldb_context *ldb,
goto failed;
}
} else {
- talloc_free(filtered_msg->elements);
- filtered_msg->elements = NULL;
+ TALLOC_FREE(filtered_msg->elements);
}
return 0;
failed:
+ TALLOC_FREE(filtered_msg->elements);
return -1;
}
diff --git a/lib/ldb/tests/ldb_filter_attrs_test.c b/lib/ldb/tests/ldb_filter_attrs_test.c
new file mode 100644
index 00000000000..7d555e0da2e
--- /dev/null
+++ b/lib/ldb/tests/ldb_filter_attrs_test.c
@@ -0,0 +1,988 @@
+/*
+ * Tests exercising the ldb_filter_attrs().
+ *
+ *
+ * Copyright (C) Catalyst.NET Ltd 2017
+ * Copyright (C) Andrew Bartlett <abartlet at samba.org> 2019
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "../include/ldb.h"
+#include "../include/ldb_module.h"
+
+struct ldbtest_ctx {
+ struct tevent_context *ev;
+ struct ldb_context *ldb;
+};
+
+/*
+ * NOTE WELL:
+ *
+ * This test checks the current behaviour of the function, however
+ * this is not in a public ABI and many of the tested behaviours are
+ * not ideal. If the behaviour is deliberatly improved, this test
+ * should be updated without worry to the new better behaviour.
+ *
+ * In particular the test is particularly to ensure the current
+ * behaviour is memory-safe.
+ */
+
+static int setup(void **state)
+{
+ struct ldbtest_ctx *test_ctx;
+
+ test_ctx = talloc_zero(NULL, struct ldbtest_ctx);
+ assert_non_null(test_ctx);
+
+ test_ctx->ev = tevent_context_init(test_ctx);
+ assert_non_null(test_ctx->ev);
+
+ test_ctx->ldb = ldb_init(test_ctx, test_ctx->ev);
+ assert_non_null(test_ctx->ldb);
+
+ *state = test_ctx;
+ return 0;
+}
+
+static int teardown(void **state)
+{
+ talloc_free(*state);
+ return 0;
+}
+
+
+/*
+ * Test against a record with only one attribute, matching the one in
+ * the list
+ */
+static void test_filter_attrs_one_attr_matched(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ int ret;
+
+ struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+
+ const char *attrs[] = {"foo", NULL};
+
+ uint8_t value[] = "The value.......end";
+ struct ldb_val value_1 = {
+ .data = value,
+ .length = (sizeof(value))
+ };
+ struct ldb_message_element element_1 = {
+ .name = "foo",
+ .num_values = 1,
+ .values = &value_1
+ };
+ struct ldb_message in = {
+ .dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
+ .num_elements = 1,
+ .elements = &element_1,
+ };
+
+ assert_non_null(in.dn);
+
+ ret = ldb_filter_attrs(ctx->ldb,
+ &in,
+ attrs,
+ filtered_msg);
+ assert_int_equal(ret, LDB_SUCCESS);
+ assert_non_null(filtered_msg);
+
+ /*
+ * assert the ldb_filter_attrs does not read or modify
+ * filtered_msg.dn in this case
+ */
+ assert_null(filtered_msg->dn);
+ assert_int_equal(filtered_msg->num_elements, 1);
+ assert_string_equal(filtered_msg->elements[0].name, "foo");
+ assert_int_equal(filtered_msg->elements[0].num_values, 1);
+ assert_int_equal(filtered_msg->elements[0].values[0].length,
+ sizeof(value));
+ assert_memory_equal(filtered_msg->elements[0].values[0].data,
+ value, sizeof(value));
+}
+
+/*
+ * Test against a record with only one attribute, matching the one of
+ * the multiple attributes in the list
+ */
+static void test_filter_attrs_one_attr_matched_of_many(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ int ret;
+
+ struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+
+ const char *attrs[] = {"foo", "bar", "baz", NULL};
+
+ uint8_t value[] = "The value.......end";
+ struct ldb_val value_1 = {
+ .data = value,
+ .length = (sizeof(value))
+ };
+ struct ldb_message_element element_1 = {
+ .name = "foo",
+ .num_values = 1,
+ .values = &value_1
+ };
+ struct ldb_message in = {
+ .dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
+ .num_elements = 1,
+ .elements = &element_1,
+ };
+
+ assert_non_null(in.dn);
+
+ ret = ldb_filter_attrs(ctx->ldb,
+ &in,
+ attrs,
+ filtered_msg);
+ assert_int_equal(ret, LDB_SUCCESS);
+ assert_non_null(filtered_msg);
+
+ /*
+ * assert the ldb_filter_attrs does not read or modify
+ * filtered_msg.dn in this case
+ */
+ assert_null(filtered_msg->dn);
+ assert_int_equal(filtered_msg->num_elements, 1);
+ assert_string_equal(filtered_msg->elements[0].name, "foo");
+ assert_int_equal(filtered_msg->elements[0].num_values, 1);
+ assert_int_equal(filtered_msg->elements[0].values[0].length,
+ sizeof(value));
+ assert_memory_equal(filtered_msg->elements[0].values[0].data,
+ value, sizeof(value));
+}
+
+/*
+ * Test against a record with only one attribute, matching both
+ * attributes in the list
+ */
+static void test_filter_attrs_two_attr_matched_attrs(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ int ret;
+
+ struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+
+ /* deliberatly the other order */
+ const char *attrs[] = {"bar", "foo", NULL};
+
+ uint8_t value1[] = "The value.......end";
+ uint8_t value2[] = "The value..MUST.end";
+ struct ldb_val value_1 = {
+ .data = value1,
+ .length = (sizeof(value1))
+ };
+ struct ldb_val value_2 = {
+ .data = value2,
+ .length = (sizeof(value2))
+ };
+
+ /* foo and bar are the other order to in attrs */
+ struct ldb_message_element elements[] = {
+ {
+ .name = "foo",
+ .num_values = 1,
+ .values = &value_1
+ },
+ {
+ .name = "bar",
+ .num_values = 1,
+ .values = &value_2
+ }
+ };
+ struct ldb_message in = {
+ .dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
+ .num_elements = 2,
+ .elements = elements,
+ };
+
+ assert_non_null(in.dn);
+
+ ret = ldb_filter_attrs(ctx->ldb,
+ &in,
+ attrs,
+ filtered_msg);
+ assert_int_equal(ret, LDB_SUCCESS);
+ assert_non_null(filtered_msg);
+ assert_int_equal(filtered_msg->num_elements, 2);
+
+ /*
+ * assert the ldb_filter_attrs does not read or modify
+ * filtered_msg.dn in this case
+ */
+ assert_null(filtered_msg->dn);
+
+ /* Assert that DB order is preserved */
+ assert_string_equal(filtered_msg->elements[0].name, "foo");
+ assert_int_equal(filtered_msg->elements[0].num_values, 1);
+ assert_int_equal(filtered_msg->elements[0].values[0].length,
+ sizeof(value1));
+ assert_memory_equal(filtered_msg->elements[0].values[0].data,
+ value1, sizeof(value1));
+ assert_string_equal(filtered_msg->elements[1].name, "bar");
+ assert_int_equal(filtered_msg->elements[1].num_values, 1);
+ assert_int_equal(filtered_msg->elements[1].values[0].length,
+ sizeof(value2));
+ assert_memory_equal(filtered_msg->elements[1].values[0].data,
+ value2, sizeof(value2));
+}
+
+/*
+ * Test against a record with two attributes, only of which is in
+ * the list
+ */
+static void test_filter_attrs_two_attr_matched_one_attr(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ int ret;
+
+ struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+
+ /* deliberatly the other order */
+ const char *attrs[] = {"bar", NULL};
+
+ uint8_t value1[] = "The value.......end";
+ uint8_t value2[] = "The value..MUST.end";
+ struct ldb_val value_1 = {
+ .data = value1,
+ .length = (sizeof(value1))
+ };
+ struct ldb_val value_2 = {
+ .data = value2,
+ .length = (sizeof(value2))
+ };
+
+ /* foo and bar are the other order to in attrs */
+ struct ldb_message_element elements[] = {
+ {
+ .name = "foo",
+ .num_values = 1,
+ .values = &value_1
+ },
+ {
+ .name = "bar",
+ .num_values = 1,
+ .values = &value_2
+ }
+ };
+ struct ldb_message in = {
+ .dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
+ .num_elements = 2,
+ .elements = elements,
+ };
+
+ assert_non_null(in.dn);
+
+ ret = ldb_filter_attrs(ctx->ldb,
+ &in,
+ attrs,
+ filtered_msg);
+ assert_int_equal(ret, LDB_SUCCESS);
+ assert_non_null(filtered_msg);
+ assert_int_equal(filtered_msg->num_elements, 1);
+
+ /*
+ * assert the ldb_filter_attrs does not read or modify
+ * filtered_msg.dn in this case
+ */
+ assert_null(filtered_msg->dn);
+
+ /* Assert that DB order is preserved */
+ assert_string_equal(filtered_msg->elements[0].name, "bar");
+ assert_int_equal(filtered_msg->elements[0].num_values, 1);
+ assert_int_equal(filtered_msg->elements[0].values[0].length,
+ sizeof(value2));
+ assert_memory_equal(filtered_msg->elements[0].values[0].data,
+ value2, sizeof(value2));
+}
+
+/*
+ * Test against a record with two attributes, both matching the one
+ * specified attribute in the list (a corrupt record)
+ */
+static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ int ret;
+
+ struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+
+ /* deliberatly the other order */
+ const char *attrs[] = {"bar", NULL};
+
+ uint8_t value1[] = "The value.......end";
+ uint8_t value2[] = "The value..MUST.end";
+ struct ldb_val value_1 = {
+ .data = value1,
+ .length = (sizeof(value1))
+ };
+ struct ldb_val value_2 = {
+ .data = value2,
+ .length = (sizeof(value2))
+ };
+
+ /* foo and bar are the other order to in attrs */
+ struct ldb_message_element elements[] = {
+ {
+ .name = "bar",
+ .num_values = 1,
+ .values = &value_1
+ },
+ {
+ .name = "bar",
+ .num_values = 1,
+ .values = &value_2
+ }
+ };
+ struct ldb_message in = {
+ .dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
+ .num_elements = 2,
+ .elements = elements,
+ };
+
+ assert_non_null(in.dn);
+
+ ret = ldb_filter_attrs(ctx->ldb,
+ &in,
+ attrs,
+ filtered_msg);
+
+ /* This should fail the pidgenhole test */
+ assert_int_equal(ret, -1);
+ assert_null(filtered_msg->elements);
+}
+
+/*
+ * Test against a record with two attributes, both matching the one
+ * specified attribute in the list (a corrupt record)
+ */
+static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ int ret;
+
+ struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+
+ const char *attrs[] = {"bar", "bar", NULL};
+
+ uint8_t value1[] = "The value.......end";
+ uint8_t value2[] = "The value..MUST.end";
+ struct ldb_val value_1 = {
+ .data = value1,
+ .length = (sizeof(value1))
+ };
+ struct ldb_val value_2 = {
+ .data = value2,
+ .length = (sizeof(value2))
+ };
+
+ /* foo and bar are the other order to in attrs */
+ struct ldb_message_element elements[] = {
+ {
+ .name = "bar",
--
Samba Shared Repository
More information about the samba-cvs
mailing list