The Samba-Bugzilla – Attachment 13570 Details for
Bug 13012
Implementation of process-exists control is wrong
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-7
BZ13012-v4-7.patch (text/plain), 14.20 KB, created by
Amitay Isaacs
on 2017-09-11 03:43:32 UTC
(
hide
)
Description:
Patches for v4-7
Filename:
MIME Type:
Creator:
Amitay Isaacs
Created:
2017-09-11 03:43:32 UTC
Size:
14.20 KB
patch
obsolete
>From 614be9967261449ff1bf8601b7df60e867f2d6ca Mon Sep 17 00:00:00 2001 >From: Amitay Isaacs <amitay@gmail.com> >Date: Fri, 25 Aug 2017 15:00:59 +1000 >Subject: [PATCH 1/4] ctdb-daemon: Fix implementation of process_exists control > >Only check processes that are CTDB clients. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 > >Signed-off-by: Amitay Isaacs <amitay@gmail.com> >Reviewed-by: Martin Schwenke <martin@meltin.net> >(cherry picked from commit d0a20baf43834c7290dfd8f256d9521724202f0c) >--- > ctdb/server/ctdb_daemon.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > >diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c >index b5cee615e91..7f088f037e8 100644 >--- a/ctdb/server/ctdb_daemon.c >+++ b/ctdb/server/ctdb_daemon.c >@@ -1796,12 +1796,16 @@ int32_t ctdb_control_process_exists(struct ctdb_context *ctdb, pid_t pid) > { > struct ctdb_client *client; > >- if (ctdb->nodes[ctdb->pnn]->flags & (NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) { >- client = ctdb_find_client_by_pid(ctdb, pid); >- if (client != NULL) { >- DEBUG(DEBUG_NOTICE,(__location__ " Killing client with pid:%d on banned/stopped node\n", (int)pid)); >- talloc_free(client); >- } >+ client = ctdb_find_client_by_pid(ctdb, pid); >+ if (client == NULL) { >+ return -1; >+ } >+ >+ if (ctdb->nodes[ctdb->pnn]->flags & NODE_FLAGS_INACTIVE) { >+ DEBUG(DEBUG_NOTICE, >+ ("Killing client with pid:%d on banned/stopped node\n", >+ (int)pid)); >+ talloc_free(client); > return -1; > } > >-- >2.13.5 > > >From afba2eb9294e3556d0a2d9a8d10360f846899a67 Mon Sep 17 00:00:00 2001 >From: Amitay Isaacs <amitay@gmail.com> >Date: Fri, 25 Aug 2017 16:54:47 +1000 >Subject: [PATCH 2/4] ctdb-tests: Fix the implementation of process-exists in > fake daemon > >Keep track of clients and their pids. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 > >Signed-off-by: Amitay Isaacs <amitay@gmail.com> >Reviewed-by: Martin Schwenke <martin@meltin.net> >(cherry picked from commit 7dec80a7c042d83f9d48c75a8717c3d1b59b1fbf) >--- > ctdb/tests/src/fake_ctdbd.c | 81 +++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 79 insertions(+), 2 deletions(-) > >diff --git a/ctdb/tests/src/fake_ctdbd.c b/ctdb/tests/src/fake_ctdbd.c >index 2bb9b1e37e3..c79c28da0a2 100644 >--- a/ctdb/tests/src/fake_ctdbd.c >+++ b/ctdb/tests/src/fake_ctdbd.c >@@ -108,6 +108,13 @@ struct fake_control_failure { > const char *comment; > }; > >+struct ctdb_client { >+ struct ctdb_client *prev, *next; >+ struct ctdbd_context *ctdb; >+ pid_t pid; >+ void *state; >+}; >+ > struct ctdbd_context { > struct node_map *node_map; > struct interface_map *iface_map; >@@ -126,6 +133,7 @@ struct ctdbd_context { > char *reclock; > struct ctdb_public_ip_list *known_ips; > struct fake_control_failure *control_failures; >+ struct ctdb_client *client_list; > }; > > /* >@@ -825,6 +833,48 @@ fail: > } > > /* >+ * Manage clients >+ */ >+ >+static int ctdb_client_destructor(struct ctdb_client *client) >+{ >+ DLIST_REMOVE(client->ctdb->client_list, client); >+ return 0; >+} >+ >+static int client_add(struct ctdbd_context *ctdb, pid_t client_pid, >+ void *client_state) >+{ >+ struct ctdb_client *client; >+ >+ client = talloc_zero(client_state, struct ctdb_client); >+ if (client == NULL) { >+ return ENOMEM; >+ } >+ >+ client->ctdb = ctdb; >+ client->pid = client_pid; >+ client->state = client_state; >+ >+ DLIST_ADD(ctdb->client_list, client); >+ talloc_set_destructor(client, ctdb_client_destructor); >+ return 0; >+} >+ >+static void *client_find(struct ctdbd_context *ctdb, pid_t client_pid) >+{ >+ struct ctdb_client *client; >+ >+ for (client=ctdb->client_list; client != NULL; client=client->next) { >+ if (client->pid == client_pid) { >+ return client->state; >+ } >+ } >+ >+ return NULL; >+} >+ >+/* > * CTDB context setup > */ > >@@ -1148,6 +1198,7 @@ struct client_state { > int fd; > struct ctdbd_context *ctdb; > int pnn; >+ pid_t pid; > struct comm_context *comm; > struct srvid_register_state *rstate; > int status; >@@ -1261,11 +1312,22 @@ static void control_process_exists(TALLOC_CTX *mem_ctx, > struct ctdb_req_header *header, > struct ctdb_req_control *request) > { >+ struct client_state *state = tevent_req_data( >+ req, struct client_state); >+ struct ctdbd_context *ctdb = state->ctdb; >+ struct client_state *cstate; > struct ctdb_reply_control reply; > > reply.rdata.opcode = request->opcode; >- reply.status = kill(request->rdata.data.pid, 0); >- reply.errmsg = NULL; >+ >+ cstate = client_find(ctdb, request->rdata.data.pid); >+ if (cstate == NULL) { >+ reply.status = -1; >+ reply.errmsg = "No client for PID"; >+ } else { >+ reply.status = kill(request->rdata.data.pid, 0); >+ reply.errmsg = NULL; >+ } > > client_send_control(req, header, &reply); > } >@@ -2980,6 +3042,8 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx, > { > struct tevent_req *req; > struct client_state *state; >+ struct ucred cr; >+ socklen_t crl = sizeof(struct ucred); > int ret; > > req = tevent_req_create(mem_ctx, &state, struct client_state); >@@ -2992,6 +3056,13 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx, > state->ctdb = ctdb; > state->pnn = pnn; > >+ ret = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cr, &crl); >+ if (ret != 0) { >+ tevent_req_error(req, ret); >+ return tevent_req_post(req, ev); >+ } >+ state->pid = cr.pid; >+ > ret = comm_setup(state, ev, fd, client_read_handler, req, > client_dead_handler, req, &state->comm); > if (ret != 0) { >@@ -2999,6 +3070,12 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx, > return tevent_req_post(req, ev); > } > >+ ret = client_add(ctdb, state->pid, state); >+ if (ret != 0) { >+ tevent_req_error(req, ret); >+ return tevent_req_post(req, ev); >+ } >+ > DEBUG(DEBUG_INFO, ("New client fd=%d\n", fd)); > > return req; >-- >2.13.5 > > >From 807daa87b99a8db734b88f51ec546a2fc04680e0 Mon Sep 17 00:00:00 2001 >From: Amitay Isaacs <amitay@gmail.com> >Date: Wed, 30 Aug 2017 13:05:32 +1000 >Subject: [PATCH 3/4] ctdb-tests: Add a dummy ctdb client for testing > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 > >Signed-off-by: Amitay Isaacs <amitay@gmail.com> >Reviewed-by: Martin Schwenke <martin@meltin.net> >(cherry picked from commit 3067db5b50162fdae288aaad8e75beb924fc9494) >--- > ctdb/tests/src/dummy_client.c | 148 ++++++++++++++++++++++++++++++++++++++++++ > ctdb/wscript | 3 +- > 2 files changed, 150 insertions(+), 1 deletion(-) > create mode 100644 ctdb/tests/src/dummy_client.c > >diff --git a/ctdb/tests/src/dummy_client.c b/ctdb/tests/src/dummy_client.c >new file mode 100644 >index 00000000000..6af41f3b1f9 >--- /dev/null >+++ b/ctdb/tests/src/dummy_client.c >@@ -0,0 +1,148 @@ >+/* >+ Dummy CTDB client for testing >+ >+ Copyright (C) Amitay Isaacs 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 "replace.h" >+#include "system/network.h" >+ >+#include <popt.h> >+#include <talloc.h> >+#include <tevent.h> >+ >+#include "common/logging.h" >+ >+#include "client/client.h" >+ >+static struct { >+ const char *sockpath; >+ const char *debuglevel; >+ int timelimit; >+ const char *srvidstr; >+} options; >+ >+static struct poptOption cmdline_options[] = { >+ POPT_AUTOHELP >+ { "socket", 's', POPT_ARG_STRING, &options.sockpath, 0, >+ "Unix domain socket path", "filename" }, >+ { "debug", 'd', POPT_ARG_STRING, &options.debuglevel, 0, >+ "debug level", "ERR|WARNING|NOTICE|INFO|DEBUG" } , >+ { "timelimit", 't', POPT_ARG_INT, &options.timelimit, 0, >+ "time limit", "seconds" }, >+ { "srvid", 'S', POPT_ARG_STRING, &options.srvidstr, 0, >+ "srvid to register", "srvid" }, >+ POPT_TABLEEND >+}; >+ >+static void dummy_handler(uint64_t srvid, TDB_DATA data, void *private_data) >+{ >+ bool *done = (bool *)private_data; >+ >+ *done = true; >+} >+ >+int main(int argc, const char *argv[]) >+{ >+ TALLOC_CTX *mem_ctx; >+ struct tevent_context *ev; >+ struct ctdb_client_context *client; >+ const char *ctdb_socket; >+ poptContext pc; >+ int opt, ret; >+ int log_level; >+ bool status, done; >+ >+ /* Set default options */ >+ options.sockpath = CTDB_SOCKET; >+ options.debuglevel = "ERR"; >+ options.timelimit = 60; >+ options.srvidstr = NULL; >+ >+ ctdb_socket = getenv("CTDB_SOCKET"); >+ if (ctdb_socket != NULL) { >+ options.sockpath = ctdb_socket; >+ } >+ >+ pc = poptGetContext(argv[0], argc, argv, cmdline_options, >+ POPT_CONTEXT_KEEP_FIRST); >+ while ((opt = poptGetNextOpt(pc)) != -1) { >+ fprintf(stderr, "Invalid option %s\n", poptBadOption(pc, 0)); >+ exit(1); >+ } >+ >+ if (options.sockpath == NULL) { >+ fprintf(stderr, "Please specify socket path\n"); >+ poptPrintHelp(pc, stdout, 0); >+ exit(1); >+ } >+ >+ mem_ctx = talloc_new(NULL); >+ if (mem_ctx == NULL) { >+ fprintf(stderr, "Memory allocation error\n"); >+ exit(1); >+ } >+ >+ ev = tevent_context_init(mem_ctx); >+ if (ev == NULL) { >+ fprintf(stderr, "Memory allocation error\n"); >+ exit(1); >+ } >+ >+ status = debug_level_parse(options.debuglevel, &log_level); >+ if (! status) { >+ fprintf(stderr, "Invalid debug level\n"); >+ poptPrintHelp(pc, stdout, 0); >+ exit(1); >+ } >+ >+ setup_logging("dummy_client", DEBUG_STDERR); >+ DEBUGLEVEL = log_level; >+ >+ ret = ctdb_client_init(mem_ctx, ev, options.sockpath, &client); >+ if (ret != 0) { >+ D_ERR("Failed to initialize client, ret=%d\n", ret); >+ exit(1); >+ } >+ >+ done = false; >+ if (options.srvidstr != NULL) { >+ uint64_t srvid; >+ >+ srvid = strtoull(options.srvidstr, NULL, 0); >+ >+ ret = ctdb_client_set_message_handler(ev, client, srvid, >+ dummy_handler, &done); >+ if (ret != 0) { >+ D_ERR("Failed to register srvid, ret=%d\n", ret); >+ talloc_free(client); >+ exit(1); >+ } >+ >+ D_INFO("Registered SRVID 0x%"PRIx64"\n", srvid); >+ } >+ >+ ret = ctdb_client_wait_timeout(ev, &done, >+ tevent_timeval_current_ofs(options.timelimit, 0)); >+ if (ret != 0 && ret == ETIME) { >+ D_ERR("client_wait_timeout() failed, ret=%d\n", ret); >+ talloc_free(client); >+ exit(1); >+ } >+ >+ talloc_free(client); >+ exit(0); >+} >diff --git a/ctdb/wscript b/ctdb/wscript >index 7197b2a7c9a..c678228f0c7 100644 >--- a/ctdb/wscript >+++ b/ctdb/wscript >@@ -790,7 +790,8 @@ def build(bld): > 'transaction_loop', > 'update_record', > 'update_record_persistent', >- 'lock_tdb' >+ 'lock_tdb', >+ 'dummy_client' > ] > > for target in ctdb_tests: >-- >2.13.5 > > >From 344f21712aab9c04f40557e1655c32428c9a6c68 Mon Sep 17 00:00:00 2001 >From: Amitay Isaacs <amitay@gmail.com> >Date: Fri, 25 Aug 2017 16:55:34 +1000 >Subject: [PATCH 4/4] ctdb-tests: Fix ctdb process-exist tests > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 > >Signed-off-by: Amitay Isaacs <amitay@gmail.com> >Reviewed-by: Martin Schwenke <martin@meltin.net> > >Autobuild-User(master): Amitay Isaacs <amitay@samba.org> >Autobuild-Date(master): Sat Sep 9 14:44:57 CEST 2017 on sn-devel-144 > >(cherry picked from commit 87f7d32a906799e83cb9b023978e689a630de017) >--- > ctdb/tests/simple/07_ctdb_process_exists.sh | 34 ++++++++++++++++++----------- > ctdb/tests/tool/ctdb.process-exists.001.sh | 12 +++++----- > 2 files changed, 28 insertions(+), 18 deletions(-) > >diff --git a/ctdb/tests/simple/07_ctdb_process_exists.sh b/ctdb/tests/simple/07_ctdb_process_exists.sh >index b7492a84f19..c44924b0fe6 100755 >--- a/ctdb/tests/simple/07_ctdb_process_exists.sh >+++ b/ctdb/tests/simple/07_ctdb_process_exists.sh >@@ -15,11 +15,10 @@ Prerequisites: > Steps: > > 1. Verify that the status on all of the ctdb nodes is 'OK'. >-2. On one of the cluster nodes, get the PID of an existing process >- (using ps wax). >+2. On one of the cluster nodes, get the PID of a ctdb client. > 3. Run 'ctdb process-exists <pid>' on the node and verify that the > correct output is shown. >-4. Run 'ctdb process-exists <pid>' with a pid of a non-existent >+4. Run 'ctdb process-exists <pid>' with a pid of ctdb daemon > process and verify that the correct output is shown. > > Expected results: >@@ -38,15 +37,25 @@ cluster_is_healthy > > test_node=1 > >-# Create a background process on $test_node that will last for 60 seconds. >+# Execute a ctdb client on $test_node that will last for 60 seconds. > # It should still be there when we check. >-try_command_on_node $test_node 'sleep 60 >/dev/null 2>&1 & echo $!' >-pid="$out" >+try_command_on_node -v $test_node \ >+ "$CTDB_TEST_WRAPPER exec dummy_client >/dev/null 2>&1 & echo \$!" >+client_pid="$out" > >-echo "Checking for PID $pid on node $test_node" >-# set -e is good, but avoid it here >+cleanup () >+{ >+ if [ -n "$client_pid" ] ; then >+ onnode $test_node kill -9 "$client_pid" >+ fi >+} >+ >+ctdb_test_exit_hook_add cleanup >+ >+echo "Checking for PID $client_pid on node $test_node" > status=0 >-try_command_on_node $test_node "$CTDB process-exists ${pid}" || status=$? >+try_command_on_node $test_node \ >+ "$CTDB process-exists ${client_pid}" || status=$? > echo "$out" > > if [ $status -eq 0 ] ; then >@@ -56,10 +65,9 @@ else > testfailures=1 > fi > >-# Now just echo the PID of the shell from the onnode process on node >-# 2. This PID will disappear and PIDs shouldn't roll around fast >-# enough to trick the test... but there is a chance that will happen! >-try_command_on_node $test_node 'echo $$' >+# Now just echo the PID of the ctdb daemon on test node. >+# This is not a ctdb client and process-exists should return error. >+try_command_on_node $test_node "ctdb getpid" > pid="$out" > > echo "Checking for PID $pid on node $test_node" >diff --git a/ctdb/tests/tool/ctdb.process-exists.001.sh b/ctdb/tests/tool/ctdb.process-exists.001.sh >index b153da1e1fe..2339344fec5 100755 >--- a/ctdb/tests/tool/ctdb.process-exists.001.sh >+++ b/ctdb/tests/tool/ctdb.process-exists.001.sh >@@ -11,12 +11,14 @@ NODEMAP > 2 192.168.20.43 0x0 > EOF > >-pid=$(ctdbd_getpid) >+dummy_client -s $ctdbd_socket & >+pid=$! > > ok "PID $pid exists" > simple_test "$pid" > >-# Use a PID that is probably impossible. It must fit into 32 bits but >-# should be larger than most settings for pid_max. >-required_result 1 "PID 99999999 does not exist" >-simple_test "99999999" >+kill -9 $pid >+ >+pid=$(ctdbd_getpid) >+required_result 1 "PID $pid does not exist" >+simple_test "$pid" >-- >2.13.5 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
martins
:
review+
Actions:
View
Attachments on
bug 13012
:
13569
|
13570
|
13585
|
13586