Skip to content

Commit

Permalink
libcontainer: Bail on close(2) failures
Browse files Browse the repository at this point in the history
Don't ignore close(2) return code, rather bail if there is any
unexpected failures. By checking the close return code we make sure we
don't introduce the same bug (closing an already closed fd) I've fixed
in the previous patch.

As a side note, we are not handling in this patch when close(2) returns
EINTR and the go runtime, since go 1.14, sends SIGURG to preempt
goroutines. This should not happen here though, as nsenter is guaranteed
to be executed before the go runtime starts.

Signed-off-by: Rodrigo Campos <[email protected]>
  • Loading branch information
rata committed Jul 2, 2021
1 parent 7d479e6 commit 6289bab
Showing 1 changed file with 14 additions and 6 deletions.
20 changes: 14 additions & 6 deletions libcontainer/nsenter/nsexec.c
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,8 @@ void nsexec(void)
bail("unable to spawn stage-1");

syncfd = sync_child_pipe[1];
close(sync_child_pipe[0]);
if (close(sync_child_pipe[0]) < 0)
bail("Failed to close fd sync_child_pipe[0]");

/*
* State machine for synchronisation with the children. We only
Expand Down Expand Up @@ -839,7 +840,9 @@ void nsexec(void)

/* Now sync with grandchild. */
syncfd = sync_grandchild_pipe[1];
close(sync_grandchild_pipe[0]);
if (close(sync_grandchild_pipe[0]) < 0)
bail("Failed to close fd sync_grandchild_pipe[0]");

write_log(DEBUG, "-> stage-2 synchronisation loop");
stage2_complete = false;
while (!stage2_complete) {
Expand Down Expand Up @@ -885,7 +888,8 @@ void nsexec(void)

/* We're in a child and thus need to tell the parent if we die. */
syncfd = sync_child_pipe[0];
close(sync_child_pipe[1]);
if (close(sync_child_pipe[1]) < 0)
bail("failed to close sync_child_pipe[1]");

/* For debugging. */
prctl(PR_SET_NAME, (unsigned long)"runc:[1:CHILD]", 0, 0, 0);
Expand Down Expand Up @@ -1042,8 +1046,11 @@ void nsexec(void)

/* We're in a child and thus need to tell the parent if we die. */
syncfd = sync_grandchild_pipe[0];
close(sync_grandchild_pipe[1]);
close(sync_child_pipe[0]);
if (close(sync_grandchild_pipe[1]) < 0)
bail("failed to close sync_grandchild_pipe[1]");

if (close(sync_child_pipe[0]) < 0)
bail("failed to close sync_child_pipe[0]");

/* For debugging. */
prctl(PR_SET_NAME, (unsigned long)"runc:[2:INIT]", 0, 0, 0);
Expand Down Expand Up @@ -1094,7 +1101,8 @@ void nsexec(void)
bail("failed to sync with patent: write(SYNC_CHILD_FINISH)");

/* Close sync pipes. */
close(sync_grandchild_pipe[0]);
if (close(sync_grandchild_pipe[0]) < 0)
bail("failed to close sync_grandchild_pipe[0]");

/* Free netlink data. */
nl_free(&config);
Expand Down

0 comments on commit 6289bab

Please sign in to comment.