From 8a030cf174ad7afa88531753763053f134f162dc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Sep 2017 12:45:24 +0200 Subject: [PATCH 1/4] lib/util/run_cmd: prevent zombies in samba_runcmd_send on timeout Ensure the state desctructor calls tfork_destroy to reap the waiter and worker processes. Otherwise we leave the waiter process as a zombie behind us as we never call waitpid on it in case of a timeout or talloc_free() from the caller. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13062 Pair-programmed-with: Stefan Metzmacher Signed-off-by: Stefan Metzmacher Signed-off-by: Ralph Boehme (cherry picked from commit 9a8eeabd95afca2e88666b3e8f2af954dbf23ba9) --- lib/util/util_runcmd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c index 6077fdd..ef3402a 100644 --- a/lib/util/util_runcmd.c +++ b/lib/util/util_runcmd.c @@ -34,11 +34,10 @@ static int samba_runcmd_state_destructor(struct samba_runcmd_state *state) { - if (state->pid > 0) { - kill(state->pid, SIGKILL); - waitpid(state->pid, NULL, 0); - state->pid = -1; + if (state->tfork != NULL) { + tfork_destroy(&state->tfork); } + state->pid = -1; if (state->fd_stdin != -1) { close(state->fd_stdin); @@ -275,6 +274,7 @@ static void samba_runcmd_io_handler(struct tevent_context *ev, tevent_req_error(req, errno); return; } + state->pid = -1; TALLOC_FREE(fde); if (WIFEXITED(status)) { -- 1.9.1 From 54ff46cd7fa59ad907058fed63837123bd1f7d05 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Sep 2017 13:06:08 +0200 Subject: [PATCH 2/4] lib/util/run_cmd: ensure fd_stdin gets set to -1 in the destructor Bug: https://bugzilla.samba.org/show_bug.cgi?id=13062 Pair-programmed-with: Stefan Metzmacher Signed-off-by: Stefan Metzmacher Signed-off-by: Ralph Boehme (cherry picked from commit 4aaf072d1fd732abf2cbea135d508260cdafa4eb) --- lib/util/util_runcmd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c index ef3402a..9c6cb30 100644 --- a/lib/util/util_runcmd.c +++ b/lib/util/util_runcmd.c @@ -41,6 +41,7 @@ static int samba_runcmd_state_destructor(struct samba_runcmd_state *state) if (state->fd_stdin != -1) { close(state->fd_stdin); + state->fd_stdin = -1; } return 0; } -- 1.9.1 From 73013a57dbc410ebbd29bc3da513a39b46345e05 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Sep 2017 13:07:26 +0200 Subject: [PATCH 3/4] lib/util/run_cmd: remove a printf Bug: https://bugzilla.samba.org/show_bug.cgi?id=13062 Pair-programmed-with: Stefan Metzmacher Signed-off-by: Stefan Metzmacher Signed-off-by: Ralph Boehme (cherry picked from commit 94a8331e5425b735f9e2c0121afc2fb108bec891) --- lib/util/util_runcmd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c index 9c6cb30..b2f0d9f 100644 --- a/lib/util/util_runcmd.c +++ b/lib/util/util_runcmd.c @@ -110,7 +110,6 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx, state->tfork = tfork_create(); if (state->tfork == NULL) { - printf("state->tfork == NULL\n"); close(p1[0]); close(p1[1]); close(p2[0]); -- 1.9.1 From 5db6049f725674b395b6c00026c908f73778a0ca Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 29 Sep 2017 13:07:53 +0200 Subject: [PATCH 4/4] lib/util/run_cmd: use a cleanup function instead of a destructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: https://bugzilla.samba.org/show_bug.cgi?id=13062 Pair-programmed-with: Stefan Metzmacher Signed-off-by: Stefan Metzmacher Signed-off-by: Ralph Boehme Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Sat Sep 30 12:14:57 CEST 2017 on sn-devel-144 (cherry picked from commit 6539cc8a24204697b20506896c401e7b40eee928) --- lib/util/util_runcmd.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c index b2f0d9f..42d84a8 100644 --- a/lib/util/util_runcmd.c +++ b/lib/util/util_runcmd.c @@ -32,8 +32,12 @@ #include "../lib/util/tfork.h" #include "../lib/util/sys_rw.h" -static int samba_runcmd_state_destructor(struct samba_runcmd_state *state) +static void samba_runcmd_cleanup_fn(struct tevent_req *req, + enum tevent_req_state req_state) { + struct samba_runcmd_state *state = tevent_req_data( + req, struct samba_runcmd_state); + if (state->tfork != NULL) { tfork_destroy(&state->tfork); } @@ -43,7 +47,6 @@ static int samba_runcmd_state_destructor(struct samba_runcmd_state *state) close(state->fd_stdin); state->fd_stdin = -1; } - return 0; } static void samba_runcmd_io_handler(struct tevent_context *ev, @@ -140,7 +143,7 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx, smb_set_close_on_exec(state->fd_stderr); smb_set_close_on_exec(state->fd_status); - talloc_set_destructor(state, samba_runcmd_state_destructor); + tevent_req_set_cleanup_fn(req, samba_runcmd_cleanup_fn); state->fde_stdout = tevent_add_fd(ev, state, state->fd_stdout, -- 1.9.1