[SCM] Samba Shared Repository - branch master updated
Martin Schwenke
martins at samba.org
Thu Feb 16 08:22:02 UTC 2017
The branch, master has been updated
via 5e7ae1b ctdb-scripts: Initialise CTDB_NFS_CALLOUT in statd-callout
via 024a2c2 ctdb-tests: Add more comm tests
via 9db7785 ctdb-common: Fix use-after-free error in comm_fd_handler()
from 07bbd7f docs/vfs_ceph: document user_id parameter
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 5e7ae1b1e2fa8137aaa6a2a2f446156ae61f4c84
Author: Martin Schwenke <martin at meltin.net>
Date: Tue Feb 14 09:04:41 2017 +1100
ctdb-scripts: Initialise CTDB_NFS_CALLOUT in statd-callout
Some configurations may set CTDB_NFS_CALLOUT to the empty string.
They may do this if they allow a choice of NFS implementations. In
this case the default call-out for Linux kernel NFS should be used.
However, statd-callout does not call nfs_callout_init() to set the
default. Therefore, statd-callout is unable to restart the lock
manager, so the grace period is never entered.
statd-callout must call nfs_callout_init() before trying to restart
the lock manager.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12589
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
Autobuild-User(master): Martin Schwenke <martins at samba.org>
Autobuild-Date(master): Thu Feb 16 09:21:03 CET 2017 on sn-devel-144
commit 024a2c20d2bcdbcc43d16d492c7cd2d09b93c8f0
Author: Amitay Isaacs <amitay at gmail.com>
Date: Tue Feb 7 15:18:02 2017 +1100
ctdb-tests: Add more comm tests
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12580
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
commit 9db7785fc6ffbaad434ee189c0f46c488358aab5
Author: Amitay Isaacs <amitay at gmail.com>
Date: Mon Feb 6 15:54:55 2017 +1100
ctdb-common: Fix use-after-free error in comm_fd_handler()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12580
comm_write_send() creates a new tevent_req and adds it to the queue
of requests to be processed. If this tevent_req is freed, then the
queue entry is not removed causing use-after-free error.
If the tevent_req returned by comm_write_send() is freed, then that
request should be removed from the queue and any pending actions based
on that request should also be removed.
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
-----------------------------------------------------------------------
Summary of changes:
ctdb/common/comm.c | 46 +++++-
ctdb/config/statd-callout | 1 +
ctdb/tests/cunit/comm_test_001.sh | 10 +-
ctdb/tests/src/comm_test.c | 309 +++++++++++++++++++++++++++++++++-----
4 files changed, 324 insertions(+), 42 deletions(-)
Changeset truncated at 500 lines:
diff --git a/ctdb/common/comm.c b/ctdb/common/comm.c
index 7f370da..12f4970 100644
--- a/ctdb/common/comm.c
+++ b/ctdb/common/comm.c
@@ -251,14 +251,22 @@ static void comm_read_failed(struct tevent_req *req)
* Write packets
*/
+struct comm_write_entry {
+ struct comm_context *comm;
+ struct tevent_queue_entry *qentry;
+ struct tevent_req *req;
+};
+
struct comm_write_state {
struct tevent_context *ev;
struct comm_context *comm;
+ struct comm_write_entry *entry;
struct tevent_req *subreq;
uint8_t *buf;
size_t buflen, nwritten;
};
+static int comm_write_entry_destructor(struct comm_write_entry *entry);
static void comm_write_trigger(struct tevent_req *req, void *private_data);
static void comm_write_done(struct tevent_req *subreq);
@@ -269,6 +277,7 @@ struct tevent_req *comm_write_send(TALLOC_CTX *mem_ctx,
{
struct tevent_req *req;
struct comm_write_state *state;
+ struct comm_write_entry *entry;
req = tevent_req_create(mem_ctx, &state, struct comm_write_state);
if (req == NULL) {
@@ -280,15 +289,38 @@ struct tevent_req *comm_write_send(TALLOC_CTX *mem_ctx,
state->buf = buf;
state->buflen = buflen;
- if (!tevent_queue_add_entry(comm->queue, ev, req,
- comm_write_trigger, NULL)) {
- talloc_free(req);
- return NULL;
+ entry = talloc_zero(state, struct comm_write_entry);
+ if (tevent_req_nomem(entry, req)) {
+ return tevent_req_post(req, ev);
}
+ entry->comm = comm;
+ entry->req = req;
+ entry->qentry = tevent_queue_add_entry(comm->queue, ev, req,
+ comm_write_trigger, NULL);
+ if (tevent_req_nomem(entry->qentry, req)) {
+ return tevent_req_post(req, ev);
+ }
+
+ state->entry = entry;
+ talloc_set_destructor(entry, comm_write_entry_destructor);
+
return req;
}
+static int comm_write_entry_destructor(struct comm_write_entry *entry)
+{
+ struct comm_context *comm = entry->comm;
+
+ if (comm->write_req == entry->req) {
+ comm->write_req = NULL;
+ TEVENT_FD_NOT_WRITEABLE(comm->fde);
+ }
+
+ TALLOC_FREE(entry->qentry);
+ return 0;
+}
+
static void comm_write_trigger(struct tevent_req *req, void *private_data)
{
struct comm_write_state *state = tevent_req_data(
@@ -333,6 +365,8 @@ static void comm_write_done(struct tevent_req *subreq)
}
state->nwritten = nwritten;
+ state->entry->qentry = NULL;
+ TALLOC_FREE(state->entry);
tevent_req_done(req);
}
@@ -382,8 +416,8 @@ static void comm_fd_handler(struct tevent_context *ev,
struct comm_write_state *write_state;
if (comm->write_req == NULL) {
- /* This should never happen */
- abort();
+ TEVENT_FD_NOT_WRITEABLE(comm->fde);
+ return;
}
write_state = tevent_req_data(comm->write_req,
diff --git a/ctdb/config/statd-callout b/ctdb/config/statd-callout
index 3f2dd39..38f847b 100755
--- a/ctdb/config/statd-callout
+++ b/ctdb/config/statd-callout
@@ -128,6 +128,7 @@ case "$1" in
# where the lock manager will respond "strangely" immediately
# after restarting it, which causes clients to fail to reclaim
# their locks.
+ nfs_callout_init
"$CTDB_NFS_CALLOUT" "stop" "nlockmgr" >/dev/null 2>&1
sleep 2
"$CTDB_NFS_CALLOUT" "start" "nlockmgr" >/dev/null 2>&1
diff --git a/ctdb/tests/cunit/comm_test_001.sh b/ctdb/tests/cunit/comm_test_001.sh
index 5d20db2..ac09f5c 100755
--- a/ctdb/tests/cunit/comm_test_001.sh
+++ b/ctdb/tests/cunit/comm_test_001.sh
@@ -2,6 +2,12 @@
. "${TEST_SCRIPTS_DIR}/unit.sh"
-ok "100 2048 500 4096 1024 8192 200 16384 300 32768 400 65536 1048576 "
-unit_test comm_test
+ok_null
+unit_test comm_test 1
+
+ok_null
+unit_test comm_test 2
+
+ok "100 2048 500 4096 1024 8192 200 16384 300 32768 400 65536 1048576 "
+unit_test comm_test 3
diff --git a/ctdb/tests/src/comm_test.c b/ctdb/tests/src/comm_test.c
index 2189435..5e1d694 100644
--- a/ctdb/tests/src/comm_test.c
+++ b/ctdb/tests/src/comm_test.c
@@ -26,7 +26,218 @@
#include "common/pkt_write.c"
#include "common/comm.c"
-static void dead_handler(void *private_data)
+/*
+ * Test read_handler and dead_handler
+ */
+
+static void test1_read_handler(uint8_t *buf, size_t buflen,
+ void *private_data)
+{
+ int *result = (int *)private_data;
+
+ *result = -1;
+}
+
+static void test1_dead_handler(void *private_data)
+{
+ int *result = (int *)private_data;
+
+ *result = 1;
+}
+
+static void test1(void)
+{
+ TALLOC_CTX *mem_ctx;
+ struct tevent_context *ev;
+ struct comm_context *comm;
+ int fd[2];
+ int result = 0;
+ uint32_t data[2];
+ int ret;
+ ssize_t n;
+
+ mem_ctx = talloc_new(NULL);
+ assert(mem_ctx != NULL);
+
+ ev = tevent_context_init(mem_ctx);
+ assert(ev != NULL);
+
+ ret = pipe(fd);
+ assert(ret == 0);
+
+ ret = comm_setup(ev, ev, fd[0], test1_read_handler, &result,
+ test1_dead_handler, &result, &comm);
+ assert(ret == 0);
+
+ data[0] = 2 * sizeof(uint32_t);
+ data[1] = 0;
+
+ n = write(fd[1], (void *)&data, data[0]);
+ assert(n == data[0]);
+
+ while (result == 0) {
+ tevent_loop_once(ev);
+ }
+
+ assert(result == -1);
+
+ result = 0;
+ close(fd[1]);
+
+ while (result == 0) {
+ tevent_loop_once(ev);
+ }
+
+ assert(result == 1);
+
+ talloc_free(mem_ctx);
+}
+
+/*
+ * Test that the tevent_req returned by comm_write_send() can be free'd.
+ */
+
+struct test2_state {
+ TALLOC_CTX *mem_ctx;
+ bool done;
+};
+
+static void test2_read_handler(uint8_t *buf, size_t buflen,
+ void *private_data)
+{
+ struct test2_state *state = (struct test2_state *)private_data;
+
+ TALLOC_FREE(state->mem_ctx);
+}
+
+static void test2_dead_handler(void *private_data)
+{
+ abort();
+}
+
+struct test2_write_state {
+ int count;
+};
+
+static void test2_write_done(struct tevent_req *subreq);
+
+static struct tevent_req *test2_write_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct comm_context *comm,
+ uint8_t *buf, size_t buflen)
+{
+ struct tevent_req *req, *subreq;
+ struct test2_write_state *state;
+ int i;
+
+ req = tevent_req_create(mem_ctx, &state, struct test2_write_state);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ state->count = 0;
+
+ for (i=0; i<10; i++) {
+ subreq = comm_write_send(state, ev, comm, buf, buflen);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, test2_write_done, req);
+ }
+
+ return req;
+}
+
+static void test2_write_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct test2_write_state *state = tevent_req_data(
+ req, struct test2_write_state);
+ bool status;
+ int ret;
+
+ status = comm_write_recv(subreq, &ret);
+ TALLOC_FREE(subreq);
+ if (! status) {
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ state->count += 1;
+
+ if (state->count == 10) {
+ tevent_req_done(req);
+ }
+}
+
+static void test2_timer_handler(struct tevent_context *ev,
+ struct tevent_timer *te,
+ struct timeval cur_time,
+ void *private_data)
+{
+ struct test2_state *state = (struct test2_state *)private_data;
+
+ state->done = true;
+}
+
+static void test2(void)
+{
+ TALLOC_CTX *mem_ctx;
+ struct tevent_context *ev;
+ struct comm_context *comm_reader, *comm_writer;
+ struct test2_state test2_state;
+ struct tevent_req *req;
+ struct tevent_timer *te;
+ int fd[2];
+ uint32_t data[2];
+ int ret;
+
+ mem_ctx = talloc_new(NULL);
+ assert(mem_ctx != NULL);
+
+ test2_state.mem_ctx = talloc_new(mem_ctx);
+ assert(test2_state.mem_ctx != NULL);
+
+ test2_state.done = false;
+
+ ev = tevent_context_init(mem_ctx);
+ assert(ev != NULL);
+
+ ret = pipe(fd);
+ assert(ret == 0);
+
+ ret = comm_setup(ev, ev, fd[0], test2_read_handler, &test2_state,
+ test2_dead_handler, NULL, &comm_reader);
+ assert(ret == 0);
+
+ ret = comm_setup(ev, ev, fd[1], NULL, NULL, test2_dead_handler, NULL,
+ &comm_writer);
+ assert(ret == 0);
+
+ data[0] = 2 * sizeof(uint32_t);
+ data[1] = 0;
+
+ req = test2_write_send(test2_state.mem_ctx, ev, comm_writer,
+ (uint8_t *)data, data[0]);
+ assert(req != NULL);
+
+ te = tevent_add_timer(ev, ev, tevent_timeval_current_ofs(5,0),
+ test2_timer_handler, &test2_state);
+ assert(te != NULL);
+
+ while (! test2_state.done) {
+ tevent_loop_once(ev);
+ }
+
+ talloc_free(mem_ctx);
+}
+
+/*
+ * Test that data is written and read correctly.
+ */
+
+static void test3_dead_handler(void *private_data)
{
int dead_data = *(int *)private_data;
@@ -34,14 +245,14 @@ static void dead_handler(void *private_data)
if (dead_data == 1) {
/* reader */
- printf("writer closed pipe\n");
+ fprintf(stderr, "writer closed pipe\n");
} else {
/* writer */
- printf("reader closed pipe\n");
+ fprintf(stderr, "reader closed pipe\n");
}
}
-struct writer_state {
+struct test3_writer_state {
struct tevent_context *ev;
struct comm_context *comm;
uint8_t *buf;
@@ -49,15 +260,15 @@ struct writer_state {
int count, id;
};
-static void writer_next(struct tevent_req *subreq);
+static void test3_writer_next(struct tevent_req *subreq);
-static struct tevent_req *writer_send(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev,
- struct comm_context *comm,
- size_t *pkt_size, int count)
+static struct tevent_req *test3_writer_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct comm_context *comm,
+ size_t *pkt_size, int count)
{
struct tevent_req *req, *subreq;
- struct writer_state *state;
+ struct test3_writer_state *state;
size_t max_size = 0, buflen;
int i;
@@ -67,7 +278,7 @@ static struct tevent_req *writer_send(TALLOC_CTX *mem_ctx,
}
}
- req = tevent_req_create(mem_ctx, &state, struct writer_state);
+ req = tevent_req_create(mem_ctx, &state, struct test3_writer_state);
if (req == NULL) {
return NULL;
}
@@ -95,16 +306,16 @@ static struct tevent_req *writer_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- tevent_req_set_callback(subreq, writer_next, req);
+ tevent_req_set_callback(subreq, test3_writer_next, req);
return req;
}
-static void writer_next(struct tevent_req *subreq)
+static void test3_writer_next(struct tevent_req *subreq)
{
struct tevent_req *req = tevent_req_callback_data(
subreq, struct tevent_req);
- struct writer_state *state = tevent_req_data(
- req, struct writer_state);
+ struct test3_writer_state *state = tevent_req_data(
+ req, struct test3_writer_state);
bool ret;
int err;
size_t buflen;
@@ -130,10 +341,10 @@ static void writer_next(struct tevent_req *subreq)
return;
}
- tevent_req_set_callback(subreq, writer_next, req);
+ tevent_req_set_callback(subreq, test3_writer_next, req);
}
-static void writer_recv(struct tevent_req *req, int *perr)
+static void test3_writer_recv(struct tevent_req *req, int *perr)
{
if (tevent_req_is_unix_error(req, perr)) {
return;
@@ -141,7 +352,7 @@ static void writer_recv(struct tevent_req *req, int *perr)
*perr = 0;
}
-static void writer(int fd, size_t *pkt_size, int count)
+static void test3_writer(int fd, size_t *pkt_size, int count)
{
TALLOC_CTX *mem_ctx;
struct tevent_context *ev;
@@ -157,31 +368,32 @@ static void writer(int fd, size_t *pkt_size, int count)
assert(ev != NULL);
err = comm_setup(mem_ctx, ev, fd, NULL, NULL,
- dead_handler, &dead_data, &comm);
+ test3_dead_handler, &dead_data, &comm);
assert(err == 0);
assert(comm != NULL);
- req = writer_send(mem_ctx, ev, comm, pkt_size, count);
+ req = test3_writer_send(mem_ctx, ev, comm, pkt_size, count);
assert(req != NULL);
tevent_req_poll(req, ev);
- writer_recv(req, &err);
+ test3_writer_recv(req, &err);
assert(err == 0);
talloc_free(mem_ctx);
}
-struct reader_state {
+struct test3_reader_state {
size_t *pkt_size;
int count, received;
bool done;
};
-static void reader_handler(uint8_t *buf, size_t buflen, void *private_data)
+static void test3_reader_handler(uint8_t *buf, size_t buflen,
+ void *private_data)
{
- struct reader_state *state = talloc_get_type_abort(
- private_data, struct reader_state);
+ struct test3_reader_state *state = talloc_get_type_abort(
+ private_data, struct test3_reader_state);
assert(buflen == state->pkt_size[state->received]);
printf("%zi ", buflen);
@@ -193,12 +405,12 @@ static void reader_handler(uint8_t *buf, size_t buflen, void *private_data)
}
}
-static void reader(int fd, size_t *pkt_size, int count)
+static void test3_reader(int fd, size_t *pkt_size, int count)
{
TALLOC_CTX *mem_ctx;
struct tevent_context *ev;
struct comm_context *comm;
- struct reader_state *state;
+ struct test3_reader_state *state;
int dead_data = 1;
int err;
--
Samba Shared Repository
More information about the samba-cvs
mailing list