Skip to content

Commit 435b8b5

Browse files
authored
[sanitizer] Don't TestPTrace() if SPARC; don't give up if internal_fork() fails (#152072)
Fixes corner cases of #151406: - Don't run TestPTrace() on SPARC, because internal_fork() on SPARC actually calls __fork(). We can't safely __fork(), because it's possible seccomp has been configured to disallow fork() but allow clone(). - if internal_fork() fails for whatever reason, we shouldn't give up. It is strictly worse to give up early than to attempt StopTheWorld. Also updates some comments/TODOs.
1 parent b8eb61a commit 435b8b5

File tree

1 file changed

+29
-7
lines changed

1 file changed

+29
-7
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,21 @@ struct ScopedSetTracerPID {
405405

406406
// This detects whether ptrace is blocked (e.g., by seccomp), by forking and
407407
// then attempting ptrace.
408-
// This separate check is necessary because StopTheWorld() creates a child
409-
// process with a shared virtual address space and shared TLS, and therefore
408+
// This separate check is necessary because StopTheWorld() creates a thread
409+
// with a shared virtual address space and shared TLS, and therefore
410410
// cannot use waitpid() due to the shared errno.
411411
static void TestPTrace() {
412+
# if SANITIZER_SPARC
413+
// internal_fork() on SPARC actually calls __fork(). We can't safely fork,
414+
// because it's possible seccomp has been configured to disallow fork() but
415+
// allow clone().
416+
Report("WARNING: skipping TestPTrace() because this is SPARC\n");
417+
Report(
418+
"If seccomp blocks ptrace, LeakSanitizer may hang without further "
419+
"notice\n");
420+
Report(
421+
"If seccomp does not block ptrace, you can safely ignore this warning\n");
422+
# else
412423
// Heuristic: only check the first time this is called. This is not always
413424
// correct (e.g., user manually triggers leak detection, then updates
414425
// seccomp, then leak detection is triggered again).
@@ -417,35 +428,46 @@ static void TestPTrace() {
417428
return;
418429
checked = true;
419430

420-
// We hope that fork() is not too expensive, because of copy-on-write.
431+
// Hopefully internal_fork() is not too expensive, thanks to copy-on-write.
421432
// Besides, this is only called the first time.
433+
// Note that internal_fork() on non-SPARC Linux actually calls
434+
// SYSCALL(clone); thus, it is reasonable to use it because if seccomp kills
435+
// TestPTrace(), it would have killed StopTheWorld() anyway.
422436
int pid = internal_fork();
423437

424438
if (pid < 0) {
425439
int rverrno;
426-
if (internal_iserror(pid, &rverrno)) {
440+
if (internal_iserror(pid, &rverrno))
427441
Report("WARNING: TestPTrace() failed to fork (errno %d)\n", rverrno);
428-
}
429-
internal__exit(-1);
442+
443+
// We don't abort the sanitizer - it's still worth letting the sanitizer
444+
// try.
445+
return;
430446
}
431447

432448
if (pid == 0) {
433449
// Child subprocess
450+
451+
// TODO: consider checking return value of internal_ptrace, to handle
452+
// SCMP_ACT_ERRNO. However, be careful not to consume too many
453+
// resources performing a proper ptrace.
434454
internal_ptrace(PTRACE_ATTACH, 0, nullptr, nullptr);
435455
internal__exit(0);
436456
} else {
437457
int wstatus;
438458
internal_waitpid(pid, &wstatus, 0);
439459

460+
// Handle SCMP_ACT_KILL
440461
if (WIFSIGNALED(wstatus)) {
441462
VReport(0,
442-
"Warning: ptrace appears to be blocked (is seccomp enabled?). "
463+
"WARNING: ptrace appears to be blocked (is seccomp enabled?). "
443464
"LeakSanitizer may hang.\n");
444465
VReport(0, "Child exited with signal %d.\n", WTERMSIG(wstatus));
445466
// We don't abort the sanitizer - it's still worth letting the sanitizer
446467
// try.
447468
}
448469
}
470+
# endif
449471
}
450472

451473
void StopTheWorld(StopTheWorldCallback callback, void *argument) {

0 commit comments

Comments
 (0)