From aea52d0ab0ea320dfee33cfb830d75ba32bef991 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Sun, 26 Oct 2025 15:58:44 -0700 Subject: [PATCH] libct/nsenter: sprinkle missing sane_kill Add a few missing sane_kill calls where they make sense. Remove one useless sane_kill of stage2_pid, as during SYNC_USERMAP stage2 is not yet started. It is harmless yet it makes the code slightly harder to read. Set the child pid to -1 upon receiving SYNC_CHILD_FINISH to minimize the chances of killing an unrelated process. When a child sends SYNC_CHILD_FINISH it is about to exit (although theoretically it could be stuck during debug logging). Signed-off-by: Kir Kolyshkin --- libcontainer/nsenter/nsexec.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/libcontainer/nsenter/nsexec.c b/libcontainer/nsenter/nsexec.c index c923c9743..8283ebb4b 100644 --- a/libcontainer/nsenter/nsexec.c +++ b/libcontainer/nsenter/nsexec.c @@ -848,8 +848,10 @@ void nsexec(void) bail("unable to spawn stage-1"); syncfd = sync_child_pipe[1]; - if (close(sync_child_pipe[0]) < 0) + if (close(sync_child_pipe[0]) < 0) { + sane_kill(stage1_pid, SIGKILL); bail("failed to close sync_child_pipe[0] fd"); + } /* * State machine for synchronisation with the children. We only @@ -860,8 +862,11 @@ void nsexec(void) while (!stage1_complete) { enum sync_t s; - if (read(syncfd, &s, sizeof(s)) != sizeof(s)) + if (read(syncfd, &s, sizeof(s)) != sizeof(s)) { + sane_kill(stage1_pid, SIGKILL); + sane_kill(stage2_pid, SIGKILL); bail("failed to sync with stage-1: next state"); + } switch (s) { case SYNC_USERMAP_PLS: @@ -887,7 +892,6 @@ void nsexec(void) s = SYNC_USERMAP_ACK; if (write(syncfd, &s, sizeof(s)) != sizeof(s)) { sane_kill(stage1_pid, SIGKILL); - sane_kill(stage2_pid, SIGKILL); bail("failed to sync with stage-1: write(SYNC_USERMAP_ACK)"); } break; @@ -938,8 +942,11 @@ void nsexec(void) case SYNC_CHILD_FINISH: write_log(DEBUG, "stage-1 complete"); stage1_complete = true; + stage1_pid = -1; break; default: + sane_kill(stage1_pid, SIGKILL); + sane_kill(stage2_pid, SIGKILL); bailx("unexpected sync value: %u", s); } } @@ -947,8 +954,10 @@ void nsexec(void) /* Now sync with grandchild. */ syncfd = sync_grandchild_pipe[1]; - if (close(sync_grandchild_pipe[0]) < 0) + if (close(sync_grandchild_pipe[0]) < 0) { + sane_kill(stage2_pid, SIGKILL); bail("failed to close sync_grandchild_pipe[0] fd"); + } write_log(DEBUG, "-> stage-2 synchronisation loop"); stage2_complete = false; @@ -962,15 +971,19 @@ void nsexec(void) bail("failed to sync with child: write(SYNC_GRANDCHILD)"); } - if (read(syncfd, &s, sizeof(s)) != sizeof(s)) + if (read(syncfd, &s, sizeof(s)) != sizeof(s)) { + sane_kill(stage2_pid, SIGKILL); bail("failed to sync with child: next state"); + } switch (s) { case SYNC_CHILD_FINISH: write_log(DEBUG, "stage-2 complete"); stage2_complete = true; + stage2_pid = -1; break; default: + sane_kill(stage2_pid, SIGKILL); bailx("unexpected sync value: %u", s); } }