[PATCH] Fix a bug introduced with serverid.tdb removal
Volker Lendecke
Volker.Lendecke at SerNet.DE
Tue Dec 12 08:05:43 UTC 2017
Hi!
cleanupd would spin 100%. Attached find a fix.
Review appreciated!
Thanks, Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From b52cd24cc5dbabc186f7b3ca80a0648ab339eb7e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 8 Dec 2017 17:18:33 +0100
Subject: [PATCH 1/3] messaging: Don't do self-sends in messaging_send_all
This leads to cleanupd doing endless MSG_SMB_UNLOCK calls, as it triggers
itself in the send_all. This worked correctly before the serverid.tdb removal
because cleanupd did not register in serverid.tdb (which was a bug, but it
helped us there).
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index a0a3f9fb1ba..561616df6e4 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -857,6 +857,11 @@ static int send_all_fn(pid_t pid, void *private_data)
struct send_all_state *state = private_data;
NTSTATUS status;
+ if (pid == getpid()) {
+ DBG_DEBUG("Skip ourselves in messaging_send_all\n");
+ return 0;
+ }
+
status = messaging_send_buf(state->msg_ctx, pid_to_procid(pid),
state->msg_type, state->buf, state->len);
if (!NT_STATUS_IS_OK(status)) {
--
2.11.0
From b02608487499cd8e287135405492d8573d3f58b1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 8 Dec 2017 17:21:37 +0100
Subject: [PATCH 2/3] messaging: Ignore messages from ourselves
For non-clustered messaging this should have never gone through the socket, we
should have caught it before in messaging_send_iov_from.
It can come in on a socket from ctdb when broadcasting in clustered mode. There
ctdb does the broadcasting.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 561616df6e4..464233fda2c 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -399,6 +399,11 @@ static void messaging_recv_cb(struct tevent_context *ev,
(unsigned)rec.msg_type, rec.buf.length, num_fds,
server_id_str_buf(rec.src, &idbuf));
+ if (server_id_same_process(&rec.src, &msg_ctx->id)) {
+ DBG_DEBUG("Ignoring self-send\n");
+ goto close_fail;
+ }
+
messaging_dispatch_rec(msg_ctx, ev, &rec);
return;
--
2.11.0
From 19afd6a3bad8c44704bf0f9b5a82467324cd47fb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Dec 2017 15:58:26 +0100
Subject: [PATCH 3/3] torture: Check messaging_send_all
We must make sure not to receive our own broadcast
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/selftest/tests.py | 1 +
source3/torture/proto.h | 1 +
source3/torture/test_messaging_send_all.c | 279 ++++++++++++++++++++++++++++++
source3/torture/torture.c | 1 +
source3/wscript_build | 1 +
5 files changed, 283 insertions(+)
create mode 100644 source3/torture/test_messaging_send_all.c
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 1b3576827a3..f8d2a4de9ba 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -147,6 +147,7 @@ local_tests = [
"LOCAL-MESSAGING-FDPASS2",
"LOCAL-MESSAGING-FDPASS2a",
"LOCAL-MESSAGING-FDPASS2b",
+ "LOCAL-MESSAGING-SEND-ALL",
"LOCAL-PTHREADPOOL-TEVENT",
"LOCAL-CANONICALIZE-PATH",
"LOCAL-DBWRAP-WATCH1",
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 83e0c7494d8..12c76a72d62 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -124,6 +124,7 @@ bool run_messaging_fdpass1(int dummy);
bool run_messaging_fdpass2(int dummy);
bool run_messaging_fdpass2a(int dummy);
bool run_messaging_fdpass2b(int dummy);
+bool run_messaging_send_all(int dummy);
bool run_oplock_cancel(int dummy);
bool run_pthreadpool_tevent(int dummy);
bool run_g_lock1(int dummy);
diff --git a/source3/torture/test_messaging_send_all.c b/source3/torture/test_messaging_send_all.c
new file mode 100644
index 00000000000..8512fa4bffd
--- /dev/null
+++ b/source3/torture/test_messaging_send_all.c
@@ -0,0 +1,279 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Test for a messaging_send_all bug
+ * Copyright (C) Volker Lendecke 2017
+ *
+ * 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/>.
+ */
+
+#include "includes.h"
+#include "torture/proto.h"
+#include "lib/util/tevent_unix.h"
+#include "messages.h"
+#include "lib/async_req/async_sock.h"
+#include "lib/util/sys_rw.h"
+
+static pid_t fork_responder(struct messaging_context *msg_ctx,
+ int exit_pipe[2])
+{
+ struct tevent_context *ev = messaging_tevent_context(msg_ctx);
+ struct tevent_req *req;
+ pid_t child_pid;
+ int ready_pipe[2];
+ char c = 0;
+ bool ok;
+ int ret, err;
+ NTSTATUS status;
+ ssize_t nwritten;
+
+ ret = pipe(ready_pipe);
+ if (ret == -1) {
+ perror("pipe failed");
+ return -1;
+ }
+
+ child_pid = fork();
+ if (child_pid == -1) {
+ perror("fork failed");
+ close(ready_pipe[0]);
+ close(ready_pipe[1]);
+ return -1;
+ }
+
+ if (child_pid != 0) {
+ ssize_t nread;
+ close(ready_pipe[1]);
+ nread = read(ready_pipe[0], &c, 1);
+ close(ready_pipe[0]);
+ if (nread != 1) {
+ perror("read failed");
+ return -1;
+ }
+ return child_pid;
+ }
+
+ close(ready_pipe[0]);
+ close(exit_pipe[1]);
+
+ status = messaging_reinit(msg_ctx);
+ if (!NT_STATUS_IS_OK(status)) {
+ fprintf(stderr, "messaging_reinit failed: %s\n",
+ nt_errstr(status));
+ close(ready_pipe[1]);
+ exit(1);
+ }
+
+ nwritten = sys_write(ready_pipe[1], &c, 1);
+ if (nwritten != 1) {
+ fprintf(stderr, "write failed: %s\n", strerror(errno));
+ exit(1);
+ }
+
+ close(ready_pipe[1]);
+
+ req = wait_for_read_send(ev, ev, exit_pipe[0], false);
+ if (req == NULL) {
+ fprintf(stderr, "wait_for_read_send failed\n");
+ exit(1);
+ }
+
+ ok = tevent_req_poll_unix(req, ev, &err);
+ if (!ok) {
+ fprintf(stderr, "tevent_req_poll_unix failed: %s\n",
+ strerror(err));
+ exit(1);
+ }
+
+ exit(0);
+}
+
+struct messaging_send_all_state {
+ struct tevent_context *ev;
+ struct messaging_context *msg;
+ pid_t *senders;
+ size_t num_received;
+};
+
+static void collect_pong_received(struct tevent_req *subreq);
+
+static struct tevent_req *collect_pong_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct messaging_context *msg,
+ const pid_t *senders,
+ size_t num_senders)
+{
+ struct tevent_req *req, *subreq;
+ struct messaging_send_all_state *state;
+
+ req = tevent_req_create(mem_ctx, &state,
+ struct messaging_send_all_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ state->senders = talloc_memdup(
+ state, senders, num_senders * sizeof(pid_t));
+ if (tevent_req_nomem(state->senders, req)) {
+ return tevent_req_post(req, ev);
+ }
+ state->ev = ev;
+ state->msg = msg;
+
+ subreq = messaging_read_send(state, state->ev, state->msg, MSG_PONG);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, collect_pong_received, req);
+ return req;
+}
+
+static void collect_pong_received(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct messaging_send_all_state *state = tevent_req_data(
+ req, struct messaging_send_all_state);
+ size_t num_senders = talloc_array_length(state->senders);
+ size_t i;
+ struct messaging_rec *rec;
+ int ret;
+
+ ret = messaging_read_recv(subreq, state, &rec);
+ TALLOC_FREE(subreq);
+ if (tevent_req_error(req, ret)) {
+ return;
+ }
+
+ /*
+ * We need to make sure we don't receive our own broadcast!
+ */
+
+ if (rec->src.pid == (uint64_t)getpid()) {
+ fprintf(stderr, "Received my own broadcast!\n");
+ tevent_req_error(req, EMULTIHOP);
+ return;
+ }
+
+ for (i=0; i<num_senders; i++) {
+ if (state->senders[i] == (pid_t)rec->src.pid) {
+ printf("got message from %"PRIu64"\n", rec->src.pid);
+ state->senders[i] = 0;
+ state->num_received += 1;
+ break;
+ }
+ }
+
+ if (state->num_received == num_senders) {
+ printf("done\n");
+ tevent_req_done(req);
+ return;
+ }
+
+ subreq = messaging_read_send(state, state->ev, state->msg, MSG_PONG);
+ if (tevent_req_nomem(subreq, req)) {
+ return;
+ }
+ tevent_req_set_callback(subreq, collect_pong_received, req);
+}
+
+static int collect_pong_recv(struct tevent_req *req)
+{
+ return tevent_req_simple_recv_unix(req);
+}
+
+extern int torture_nprocs;
+
+bool run_messaging_send_all(int dummy)
+{
+ struct tevent_context *ev = NULL;
+ struct messaging_context *msg_ctx = NULL;
+ int exit_pipe[2];
+ pid_t children[MAX(5, torture_nprocs)];
+ struct tevent_req *req;
+ size_t i;
+ bool ok;
+ int ret, err;
+
+ ev = samba_tevent_context_init(talloc_tos());
+ if (ev == NULL) {
+ fprintf(stderr, "tevent_context_init failed\n");
+ return false;
+ }
+ msg_ctx = messaging_init(ev, ev);
+ if (msg_ctx == NULL) {
+ fprintf(stderr, "messaging_init failed\n");
+ return false;
+ }
+ ret = pipe(exit_pipe);
+ if (ret != 0) {
+ perror("parent: pipe failed for exit_pipe");
+ return false;
+ }
+
+ for (i=0; i<ARRAY_SIZE(children); i++) {
+ children[i] = fork_responder(msg_ctx, exit_pipe);
+ if (children[i] == -1) {
+ fprintf(stderr, "fork_responder(%zu) failed\n", i);
+ return false;
+ }
+ }
+
+ req = collect_pong_send(ev, ev, msg_ctx, children,
+ ARRAY_SIZE(children));
+ if (req == NULL) {
+ perror("collect_pong failed");
+ return false;
+ }
+
+ ok = tevent_req_set_endtime(req, ev,
+ tevent_timeval_current_ofs(10, 0));
+ if (!ok) {
+ perror("tevent_req_set_endtime failed");
+ return false;
+ }
+
+ messaging_send_all(msg_ctx, MSG_PING, NULL, 0);
+
+ ok = tevent_req_poll_unix(req, ev, &err);
+ if (!ok) {
+ perror("tevent_req_poll_unix failed");
+ return false;
+ }
+
+ ret = collect_pong_recv(req);
+ TALLOC_FREE(req);
+
+ if (ret != 0) {
+ fprintf(stderr, "collect_pong_send returned %s\n",
+ strerror(ret));
+ return false;
+ }
+
+ close(exit_pipe[1]);
+
+ for (i=0; i<ARRAY_SIZE(children); i++) {
+ pid_t child;
+ int status;
+
+ do {
+ child = waitpid(children[i], &status, 0);
+ } while ((child == -1) && (errno == EINTR));
+
+ if (child != children[i]) {
+ printf("waitpid(%d) failed\n", children[i]);
+ return false;
+ }
+ }
+
+ return true;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 87276c09cb3..232e8ff2bbb 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11559,6 +11559,7 @@ static struct {
{ "LOCAL-MESSAGING-FDPASS2", run_messaging_fdpass2, 0 },
{ "LOCAL-MESSAGING-FDPASS2a", run_messaging_fdpass2a, 0 },
{ "LOCAL-MESSAGING-FDPASS2b", run_messaging_fdpass2b, 0 },
+ { "LOCAL-MESSAGING-SEND-ALL", run_messaging_send_all, 0 },
{ "LOCAL-BASE64", run_local_base64, 0},
{ "LOCAL-RBTREE", run_local_rbtree, 0},
{ "LOCAL-MEMCACHE", run_local_memcache, 0},
diff --git a/source3/wscript_build b/source3/wscript_build
index 8b46caee02c..d4bc3cb1700 100644
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -1169,6 +1169,7 @@ bld.SAMBA3_BINARY('smbtorture' + bld.env.suffix3,
torture/test_buffersize.c
torture/test_messaging_read.c
torture/test_messaging_fd_passing.c
+ torture/test_messaging_send_all.c
torture/test_oplock_cancel.c
torture/test_pthreadpool_tevent.c
torture/bench_pthreadpool.c
--
2.11.0
More information about the samba-technical
mailing list