Ensure that parent is alive after installing PR_SET_PDEATHSIG #141

Merged
merged 2 commits into from Sep 14, 2016

Conversation

Projects
None yet
2 participants
Collaborator

zyga commented Sep 14, 2016

The child process used for mount namespace capture uses
prctl(PR_SET_PDEATHSIG, ...) to get a SIGINT signal when the parent
process dies. The problem is that by the time that code runs the parent
process may have already died and the signal will never arrive.

To avoid this ensure that the parent process is still alive with
kill(parent, 0). The parent is not interrogated with getppid() but
instead is inherited from the original parent across fork as the child
process is re-parented to the init process (or locally installed
subreaper) when that occurs.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

Ensure that parent is alive after installing PR_SET_PDEATHSIG
The child process used for mount namespace capture uses
prctl(PR_SET_PDEATHSIG, ...) to get a SIGINT signal when the parent
process dies. The problem is that by the time that code runs the parent
process may have already died and the signal will never arrive.

To avoid this ensure that the parent process is still alive with
kill(parent, 0). The parent is not interrogated with getppid() but
instead is inherited from the original parent across fork as the child
process is re-parented to the init process (or locally installed
subreaper) when that occurs.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
+ die("cannot ensure that parent process is still alive");
+ break;
+ }
+ }
@jdstrand

jdstrand Sep 14, 2016

Contributor

This is definitely better than no check, but this check is not perfect. The parent could die and the kernel could assign the parent pid to a new process. Perhaps it would be better to simply timeout. The parent is supposed to send that signal quite fast... if the child doesn't get that in say, 10 seconds, just debug("parent process did not signal with 10 seconds"); abort();

@zyga

zyga Sep 14, 2016

Collaborator

I have this in a separate branch and I agree we should do both.

@jdstrand

jdstrand Sep 14, 2016

Contributor

After talking on IRC, the intent of this is simply an early warning system if the parent dies right away and that an in progress PR will implement a timeout. In this light, this PR is fine to land as is-- in the normal case the pid won't be reused and we can bail fast but if the pid is reused, that process won't send the signal anyway and the timeout will expire and we can cleanup then.

+1 provided that a comment is added regarding pid reuse and that we'll use a timeout to handle that.

@zyga zyga merged commit 11d202c into master Sep 14, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@zyga zyga deleted the ensure-parent-alive branch Sep 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment