The branch, master has been updated
via ba33d90 ldb: Ensure we can open a new LDB after a fork()
via f891b8d ldb: Add tests for ldb_tdb use after a fork()
via 2136664 ldb_tdb: Allow use of a TDB for ldb_tdb after as fork()
via 3b06915 ldb: Reset errno before checking it in ltdb_connect()
via daf79e5 ldb/tests: add tests for
transaction_{start,commit}/lock_read across forks
via 1174b52 ldb_tdb: Prevent ldb_tdb reuse after a fork()
from 4e78aee s3: VFS: Fix memory leak in vfs_ceph.
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit ba33d90ed67b787a11b26bc69f9553d0d52dfe45
Author: Andrew Bartlett <abart...@samba.org>
Date: Wed May 9 12:53:53 2018 +1200
ldb: Ensure we can open a new LDB after a fork()
Based on work for an mdb-specific test by Gary Lockyer
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Wed May 9 07:27:24 CEST 2018 on sn-devel-144
commit f891b8dc32d24eea81ee5fe6ace02c1cf7129443
Author: Andrew Bartlett <abart...@samba.org>
Date: Mon May 7 12:59:00 2018 +1200
ldb: Add tests for ldb_tdb use after a fork()
We need to show that despite the internal cache of TDB pointers that it
is safe to open a ldb_tdb after a fork()
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 21366649410c29904a463b57e7d8688ce6e11381
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri May 4 22:22:26 2018 +1200
ldb_tdb: Allow use of a TDB for ldb_tdb after as fork()
Otherwise we rely on the caller doing tdb_reopen_all() which should
not be their job.
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 3b069156639e4d53f8bd392fe6a21c36ff0caa99
Author: Andrew Bartlett <abart...@samba.org>
Date: Mon May 7 12:59:49 2018 +1200
ldb: Reset errno before checking it in ltdb_connect()
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit daf79e5b35b354200b26b2b0fac3487287ffa720
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Wed May 9 11:02:41 2018 +1200
ldb/tests: add tests for transaction_{start,commit}/lock_read across forks
(Split from a larger commit by Andrew Bartlett)
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 1174b52b91de045b5c111dff97c49488ac963bfa
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri May 4 14:35:14 2018 +1200
ldb_tdb: Prevent ldb_tdb reuse after a fork()
We may relax this restriction in the future, but for now do not assume
that the caller has done a tdb_reopen_all() at the right time.
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
-----------------------------------------------------------------------
Summary of changes:
lib/ldb/ldb_tdb/ldb_tdb.c | 112 +++++++++++-
lib/ldb/ldb_tdb/ldb_tdb.h | 7 +
lib/ldb/ldb_tdb/ldb_tdb_wrap.c | 18 ++
lib/ldb/tests/ldb_mod_op_test.c | 280 +++++++++++++++++++++++++++++
lib/ldb/tests/ldb_tdb_test.c | 387 ++++++++++++++++++++++++++++++++++++++++
lib/ldb/wscript | 8 +-
6 files changed, 802 insertions(+), 10 deletions(-)
create mode 100644 lib/ldb/tests/ldb_tdb_test.c
Changeset truncated at 500 lines:
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 0833a4f..f9bc35c 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -100,6 +100,17 @@ static int ltdb_lock_read(struct ldb_module *module)
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
int tdb_ret = 0;
int ret;
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
if (tdb_transaction_active(ltdb->tdb) == false &&
ltdb->read_lock_count == 0) {
@@ -128,6 +139,17 @@ static int ltdb_unlock_read(struct ldb_module *module)
{
void *data = ldb_module_get_private(module);
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
if (!tdb_transaction_active(ltdb->tdb) && ltdb->read_lock_count == 1) {
tdb_unlockall_read(ltdb->tdb);
ltdb->read_lock_count--;
@@ -1447,21 +1469,69 @@ static int ltdb_rename(struct ltdb_context *ctx)
static int ltdb_tdb_transaction_start(struct ltdb_private *ltdb)
{
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(ltdb->module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
+
return tdb_transaction_start(ltdb->tdb);
}
static int ltdb_tdb_transaction_cancel(struct ltdb_private *ltdb)
{
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(ltdb->module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
+
return tdb_transaction_cancel(ltdb->tdb);
}
static int ltdb_tdb_transaction_prepare_commit(struct ltdb_private *ltdb)
{
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(ltdb->module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
+
return tdb_transaction_prepare_commit(ltdb->tdb);
}
static int ltdb_tdb_transaction_commit(struct ltdb_private *ltdb)
{
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(ltdb->module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
+
return tdb_transaction_commit(ltdb->tdb);
}
@@ -1470,6 +1540,18 @@ static int ltdb_start_trans(struct ldb_module *module)
void *data = ldb_module_get_private(module);
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(ltdb->module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
+
/* Do not take out the transaction lock on a read-only DB */
if (ltdb->read_only) {
return LDB_ERR_UNWILLING_TO_PERFORM;
@@ -1498,6 +1580,17 @@ static int ltdb_prepare_commit(struct ldb_module *module)
int ret;
void *data = ldb_module_get_private(module);
struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
+ pid_t pid = getpid();
+
+ if (ltdb->pid != pid) {
+ ldb_asprintf_errstring(
+ ldb_module_get_ctx(module),
+ __location__": Reusing ldb opend by pid %d in "
+ "process %d\n",
+ ltdb->pid,
+ pid);
+ return LDB_ERR_PROTOCOL_ERROR;
+ }
if (!ltdb->kv_ops->transaction_active(ltdb)) {
ldb_set_errstring(ldb_module_get_ctx(module),
@@ -2138,8 +2231,6 @@ int init_store(struct ltdb_private *ltdb,
const char *options[],
struct ldb_module **_module)
{
- struct ldb_module *module;
-
if (getenv("LDB_WARN_UNINDEXED")) {
ltdb->warn_unindexed = true;
}
@@ -2150,23 +2241,25 @@ int init_store(struct ltdb_private *ltdb,
ltdb->sequence_number = 0;
- module = ldb_module_new(ldb, ldb, name, <db_ops);
- if (!module) {
+ ltdb->pid = getpid();
+
+ ltdb->module = ldb_module_new(ldb, ldb, name, <db_ops);
+ if (!ltdb->module) {
ldb_oom(ldb);
talloc_free(ltdb);
return LDB_ERR_OPERATIONS_ERROR;
}
- ldb_module_set_private(module, ltdb);
- talloc_steal(module, ltdb);
+ ldb_module_set_private(ltdb->module, ltdb);
+ talloc_steal(ltdb->module, ltdb);
- if (ltdb_cache_load(module) != 0) {
+ if (ltdb_cache_load(ltdb->module) != 0) {
ldb_asprintf_errstring(ldb, "Unable to load ltdb cache "
"records for backend '%s'", name);
- talloc_free(module);
+ talloc_free(ltdb->module);
return LDB_ERR_OPERATIONS_ERROR;
}
- *_module = module;
+ *_module = ltdb->module;
/*
* Set or override the maximum key length
*
@@ -2262,6 +2355,7 @@ int ltdb_connect(struct ldb_context *ldb, const char *url,
ltdb->kv_ops = &key_value_ops;
+ errno = 0;
/* note that we use quite a large default hash size */
ltdb->tdb = ltdb_wrap_open(ltdb, path, 10000,
tdb_flags, open_flags,
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.h b/lib/ldb/ldb_tdb/ldb_tdb.h
index 4d53120..caaa67a 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.h
+++ b/lib/ldb/ldb_tdb/ldb_tdb.h
@@ -36,6 +36,7 @@ struct kv_db_ops {
ldb_context */
struct ltdb_private {
const struct kv_db_ops *kv_ops;
+ struct ldb_module *module;
TDB_CONTEXT *tdb;
unsigned int connect_flags;
@@ -75,6 +76,12 @@ struct ltdb_private {
* greater than this length will be rejected.
*/
unsigned max_key_length;
+
+ /*
+ * The PID that opened this database so we don't work in a
+ * fork()ed child.
+ */
+ pid_t pid;
};
struct ltdb_context {
diff --git a/lib/ldb/ldb_tdb/ldb_tdb_wrap.c b/lib/ldb/ldb_tdb/ldb_tdb_wrap.c
index eb16809..bc702a2 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb_wrap.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb_wrap.c
@@ -74,6 +74,7 @@ struct ltdb_wrap {
struct tdb_context *tdb;
dev_t device;
ino_t inode;
+ pid_t pid;
};
static struct ltdb_wrap *tdb_list;
@@ -105,9 +106,25 @@ struct tdb_context *ltdb_wrap_open(TALLOC_CTX *mem_ctx,
if (stat(path, &st) == 0) {
for (w=tdb_list;w;w=w->next) {
if (st.st_dev == w->device && st.st_ino == w->inode) {
+ pid_t pid = getpid();
+ int ret;
if (!talloc_reference(mem_ctx, w)) {
return NULL;
}
+ if (w->pid != pid) {
+ ret = tdb_reopen(w->tdb);
+ if (ret != 0) {
+ /*
+ * Avoid use-after-free:
+ * on fail the TDB
+ * is closed!
+ */
+ DLIST_REMOVE(tdb_list,
+ w);
+ return NULL;
+ }
+ w->pid = pid;
+ }
return w->tdb;
}
}
@@ -135,6 +152,7 @@ struct tdb_context *ltdb_wrap_open(TALLOC_CTX *mem_ctx,
w->device = st.st_dev;
w->inode = st.st_ino;
+ w->pid = getpid();
talloc_set_destructor(w, ltdb_wrap_destructor);
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 1340f5e..67ac024 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -3820,6 +3820,270 @@ static void
test_ldb_talloc_destructor_transaction_cleanup(void **state)
}
}
+static void test_transaction_start_across_fork(void **state)
+{
+ struct ldb_context *ldb1 = NULL;
+ int ret;
+ struct ldbtest_ctx *test_ctx = NULL;
+ int pipes[2];
+ char buf[2];
+ int wstatus;
+ pid_t pid, child_pid;
+
+ test_ctx = talloc_get_type_abort(*state, struct ldbtest_ctx);
+
+ /*
+ * Open the database
+ */
+ ldb1 = ldb_init(test_ctx, test_ctx->ev);
+ ret = ldb_connect(ldb1, test_ctx->dbpath, 0, NULL);
+ assert_int_equal(ret, 0);
+
+ ret = pipe(pipes);
+ assert_int_equal(ret, 0);
+
+ child_pid = fork();
+ if (child_pid == 0) {
+ close(pipes[0]);
+ ret = ldb_transaction_start(ldb1);
+ if (ret != LDB_ERR_PROTOCOL_ERROR) {
+ print_error(__location__": ldb_transaction_start "
+ "returned (%d) %s\n",
+ ret,
+ ldb1->err_string);
+ exit(LDB_ERR_OTHER);
+ }
+
+ ret = write(pipes[1], "GO", 2);
+ if (ret != 2) {
+ print_error(__location__
+ " write returned (%d)",
+ ret);
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+ exit(LDB_SUCCESS);
+ }
+ close(pipes[1]);
+ ret = read(pipes[0], buf, 2);
+ assert_int_equal(ret, 2);
+
+ pid = waitpid(child_pid, &wstatus, 0);
+ assert_int_equal(pid, child_pid);
+
+ assert_true(WIFEXITED(wstatus));
+
+ assert_int_equal(WEXITSTATUS(wstatus), 0);
+}
+
+static void test_transaction_commit_across_fork(void **state)
+{
+ struct ldb_context *ldb1 = NULL;
+ int ret;
+ struct ldbtest_ctx *test_ctx = NULL;
+ int pipes[2];
+ char buf[2];
+ int wstatus;
+ pid_t pid, child_pid;
+
+ test_ctx = talloc_get_type_abort(*state, struct ldbtest_ctx);
+
+ /*
+ * Open the database
+ */
+ ldb1 = ldb_init(test_ctx, test_ctx->ev);
+ ret = ldb_connect(ldb1, test_ctx->dbpath, 0, NULL);
+ assert_int_equal(ret, 0);
+
+ ret = ldb_transaction_start(ldb1);
+ assert_int_equal(ret, 0);
+
+ ret = pipe(pipes);
+ assert_int_equal(ret, 0);
+
+ child_pid = fork();
+ if (child_pid == 0) {
+ close(pipes[0]);
+ ret = ldb_transaction_commit(ldb1);
+
+ if (ret != LDB_ERR_PROTOCOL_ERROR) {
+ print_error(__location__": ldb_transaction_commit "
+ "returned (%d) %s\n",
+ ret,
+ ldb1->err_string);
+ exit(LDB_ERR_OTHER);
+ }
+
+ ret = write(pipes[1], "GO", 2);
+ if (ret != 2) {
+ print_error(__location__
+ " write returned (%d)",
+ ret);
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+ exit(LDB_SUCCESS);
+ }
+ close(pipes[1]);
+ ret = read(pipes[0], buf, 2);
+ assert_int_equal(ret, 2);
+
+ pid = waitpid(child_pid, &wstatus, 0);
+ assert_int_equal(pid, child_pid);
+
+ assert_true(WIFEXITED(wstatus));
+
+ assert_int_equal(WEXITSTATUS(wstatus), 0);
+}
+
+static void test_lock_read_across_fork(void **state)
+{
+ struct ldb_context *ldb1 = NULL;
+ int ret;
+ struct ldbtest_ctx *test_ctx = NULL;
+ int pipes[2];
+ char buf[2];
+ int wstatus;
+ pid_t pid, child_pid;
+
+ test_ctx = talloc_get_type_abort(*state, struct ldbtest_ctx);
+
+ /*
+ * Open the database
+ */
+ ldb1 = ldb_init(test_ctx, test_ctx->ev);
+ ret = ldb_connect(ldb1, test_ctx->dbpath, 0, NULL);
+ assert_int_equal(ret, 0);
+
+ ret = pipe(pipes);
+ assert_int_equal(ret, 0);
+
+ child_pid = fork();
+ if (child_pid == 0) {
+ struct ldb_dn *basedn;
+ struct ldb_result *result = NULL;
+
+ close(pipes[0]);
+
+ basedn = ldb_dn_new_fmt(test_ctx, test_ctx->ldb, "dc=test");
+ assert_non_null(basedn);
+
+ ret = ldb_search(test_ctx->ldb,
+ test_ctx,
+ &result,
+ basedn,
+ LDB_SCOPE_BASE,
+ NULL,
+ NULL);
+ if (ret != LDB_ERR_PROTOCOL_ERROR) {
+ print_error(__location__": ldb_search "
+ "returned (%d) %s\n",
+ ret,
+ ldb1->err_string);
+ exit(LDB_ERR_OTHER);
+ }
+
+ ret = write(pipes[1], "GO", 2);
+ if (ret != 2) {
+ print_error(__location__
+ " write returned (%d)",
+ ret);
+ exit(LDB_ERR_OPERATIONS_ERROR);
+ }
+ exit(LDB_SUCCESS);
+ }
+ close(pipes[1]);
+ ret = read(pipes[0], buf, 2);
+ assert_int_equal(ret, 2);
+
+ pid = waitpid(child_pid, &wstatus, 0);
+ assert_int_equal(pid, child_pid);
+
+ assert_true(WIFEXITED(wstatus));
+
+ assert_int_equal(WEXITSTATUS(wstatus), 0);
+
+ {
+ /*
+ * Ensure that the search actually succeeds on the opening
+ * pid
+ */
+ struct ldb_dn *basedn;
+ struct ldb_result *result = NULL;
+
+ close(pipes[0]);
+
+ basedn = ldb_dn_new_fmt(test_ctx, test_ctx->ldb, "dc=test");
+ assert_non_null(basedn);
+
+ ret = ldb_search(test_ctx->ldb,
+ test_ctx,
+ &result,
+ basedn,
+ LDB_SCOPE_BASE,
+ NULL,
+ NULL);
+ assert_int_equal(0, ret);
+ }
+}
+
+static void test_multiple_opens_across_fork(void **state)
+{
+ struct ldb_context *ldb1 = NULL;
+ struct ldb_context *ldb2 = NULL;
+ int ret;
+ struct ldbtest_ctx *test_ctx = NULL;
+ int pipes[2];
+ char buf[2];
+ int wstatus;
+ pid_t pid, child_pid;
+
+ test_ctx = talloc_get_type_abort(*state, struct ldbtest_ctx);
+
+ /*
+ * Open the database again
+ */
+ ldb1 = ldb_init(test_ctx, test_ctx->ev);
+ ret = ldb_connect(ldb1, test_ctx->dbpath, LDB_FLG_RDONLY, NULL);
+ assert_int_equal(ret, 0);
--
Samba Shared Repository
