Fix race in ExceptionHandler::GenerateDump()
When writing a minidump on Linux, we called clone() in linux/handler/exception_handler.cc with the CLONE_FILES flag. If the parent process died while the child waited for the continuation signal, the write side of the pipe 'fdes' stayed open in the child. The child would not receive a SIGPIPE and would wait forever. To fix this, we clone without CLONE_FILES and then close the read-side of fdes in the master before the ptrace call. That way, if the master dies, the child will receive a SIGPIPE and will die, too. To test this I added a sleep() call before SendContinueSignalToChild() and then killed the master, manually observing that the child would die, too. Bug: 728 Change-Id: Ifd72de835a34e7d9852ae1a362e707fdc6c96c7e Reviewed-on: https://chromium-review.googlesource.com/464708 Reviewed-by: Mike Frysinger <vapier@chromium.org>
This commit is contained in:
parent
67649c6185
commit
54a54702a1
1 changed files with 9 additions and 3 deletions
|
@ -414,9 +414,14 @@ struct ThreadArgument {
|
||||||
int ExceptionHandler::ThreadEntry(void *arg) {
|
int ExceptionHandler::ThreadEntry(void *arg) {
|
||||||
const ThreadArgument *thread_arg = reinterpret_cast<ThreadArgument*>(arg);
|
const ThreadArgument *thread_arg = reinterpret_cast<ThreadArgument*>(arg);
|
||||||
|
|
||||||
|
// Close the write end of the pipe. This allows us to fail if the parent dies
|
||||||
|
// while waiting for the continue signal.
|
||||||
|
sys_close(thread_arg->handler->fdes[1]);
|
||||||
|
|
||||||
// Block here until the crashing process unblocks us when
|
// Block here until the crashing process unblocks us when
|
||||||
// we're allowed to use ptrace
|
// we're allowed to use ptrace
|
||||||
thread_arg->handler->WaitForContinueSignal();
|
thread_arg->handler->WaitForContinueSignal();
|
||||||
|
sys_close(thread_arg->handler->fdes[0]);
|
||||||
|
|
||||||
return thread_arg->handler->DoDump(thread_arg->pid, thread_arg->context,
|
return thread_arg->handler->DoDump(thread_arg->pid, thread_arg->context,
|
||||||
thread_arg->context_size) == false;
|
thread_arg->context_size) == false;
|
||||||
|
@ -523,21 +528,22 @@ bool ExceptionHandler::GenerateDump(CrashContext *context) {
|
||||||
}
|
}
|
||||||
|
|
||||||
const pid_t child = sys_clone(
|
const pid_t child = sys_clone(
|
||||||
ThreadEntry, stack, CLONE_FILES | CLONE_FS | CLONE_UNTRACED,
|
ThreadEntry, stack, CLONE_FS | CLONE_UNTRACED, &thread_arg, NULL, NULL,
|
||||||
&thread_arg, NULL, NULL, NULL);
|
NULL);
|
||||||
if (child == -1) {
|
if (child == -1) {
|
||||||
sys_close(fdes[0]);
|
sys_close(fdes[0]);
|
||||||
sys_close(fdes[1]);
|
sys_close(fdes[1]);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Close the read end of the pipe.
|
||||||
|
sys_close(fdes[0]);
|
||||||
// Allow the child to ptrace us
|
// Allow the child to ptrace us
|
||||||
sys_prctl(PR_SET_PTRACER, child, 0, 0, 0);
|
sys_prctl(PR_SET_PTRACER, child, 0, 0, 0);
|
||||||
SendContinueSignalToChild();
|
SendContinueSignalToChild();
|
||||||
int status;
|
int status;
|
||||||
const int r = HANDLE_EINTR(sys_waitpid(child, &status, __WALL));
|
const int r = HANDLE_EINTR(sys_waitpid(child, &status, __WALL));
|
||||||
|
|
||||||
sys_close(fdes[0]);
|
|
||||||
sys_close(fdes[1]);
|
sys_close(fdes[1]);
|
||||||
|
|
||||||
if (r == -1) {
|
if (r == -1) {
|
||||||
|
|
Loading…
Reference in a new issue