perf/core: Don't send SIGTRAP after exec removed the event
Subsystems (2)
PATCH v2 1/1 perf/core: Don't send SIGTRAP after exec removed the event
A sigtrap event must also set remove_on_exec, so that its SIGTRAP never reaches a program after exec. But the signal is sent from task work, which only runs on the way back to user space. If the event overflows shortly before execve(), the task work can still be pending when the task enters execve(), and then runs when execve() returns. By then perf_event_exec() has removed the event and the new program has default signal handlers, so the SIGTRAP kills it.
The exec_stress test in the remove_on_exec selftest catches this and fails about half the time in a VM. A process that opens a sigtrap event on itself and then calls execve() is killed by SIGTRAP in 15% to 50% of runs, both on an AMD machine running v7.2 and in a VM, with or without close-on-exec on the event fd.
perf_event_exit_event() sets PERF_EVENT_STATE_EXIT when exec removes the event. If the event fd is close-on-exec and was the last reference to the file, exec also queues the file release as task work. Task work runs newest first, so perf_release() runs before the SIGTRAP work and moves the event on to PERF_EVENT_STATE_DEAD. Exit is already caught by the PF_EXITING check in perf_sigtrap(). So don't send the signal when the event state is PERF_EVENT_STATE_EXIT or lower, which means the event has been removed. That also covers PERF_EVENT_STATE_REVOKED, where the PMU is gone.
---
Notes:
Changes in v2:
- Skip the signal for every state at or below PERF_EVENT_STATE_EXIT, not only EXIT. With a close-on-exec event fd the file release runs first and moves the event to PERF_EVENT_STATE_DEAD, so v1 still killed 360 of 2000 children. Reported by Sashiko.
- v1: https://lore.kernel.org/all/[email protected]/
Tested on v7.3-rc5 x86_64 under virtme-ng (KASAN, lockdep, 8 vCPUs on an AMD Zen 3 host):
- selftests/perf_events/remove_on_exec: exec_stress failed in 9 of 20 runs before, all 20 pass after.
- The exec_stress pattern in a loop (30 inheriting children, 50 rounds):
a child was killed by SIGTRAP in 17 rounds before, in none after.
- Each child opens its own sigtrap + remove_on_exec event and execs:
756 of 2000 children were killed before, none after. With
PERF_FLAG_FD_CLOEXEC: 836 of 2000 before, 360 with v1, none after. On the bare-metal host running v7.2.6 the same program kills 297 to 1033 of 2000 children across runs, and 1012 of 2000 with PERF_FLAG_FD_CLOEXEC.
- sigtrap_threads, watermark_signal and mmap pass before and after. No new kernel warnings.
v7.2 fails the same way in the VM. I did not test kernels older than v7.2. gcc W=1 and sparse show no new warnings.
kernel/events/core.c | 9 +++++++++ 1 file changed, 9 insertions(+)
kernel/events/core.c +9 -0
@@ -7631,6 +7631,15 @@ static void perf_sigtrap(struct perf_event *event)if (current->flags &PF_EXITING )return ; +/* +* The event was removed after this signal was queued, e.g. by exec() +* (remove_on_exec), which may also have closed its fd (close-on-exec). +* The new program has default signal handlers, so a SIGTRAP would +* kill it. +*/ +if (event->state <=PERF_EVENT_STATE_EXIT ) +return ; +/* * We'd expect this to only occur if the irq_work is delayed and either * ctx->task or current has changed in the meantime. This can be the
base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
Signature · 1 line
2.55.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Missing READ_ONCE() on concurrent lockless access to event->state --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing READ_ONCE() on concurrent lockless access to event->state --
commit a39b84d0fe4531dad91da56d867a84ee0e8c6cf6
Author: Danish Khateeb <[email protected]>
perf/core: Don't send SIGTRAP after exec removed the event
This commit prevents sending a SIGTRAP to a process after an exec() call has removed the perf event. It adds a state check in perf_sigtrap() to ensure signals are skipped if the event state indicates it has already been removed.
Quoted 4 lines
kernel/events/core.c +0
-0
[ ... ]
kernel/events/core.c static void perf_sigtrap(struct perf_event *event) · line 7640
Quoted 11 lines
kernel/events/core.c +7
-0@@ -7631,6 +7631,15 @@ static void perf_sigtrap(struct perf_event *event)if (current->flags &PF_EXITING )return ; +/* +* The event was removed after this signal was queued, e.g. by exec() +* (remove_on_exec), which may also have closed its fd (close-on-exec). +* The new program has default signal handlers, so a SIGTRAP would +* kill it. +*/ +if (event->state <=PERF_EVENT_STATE_EXIT )
[Severity: Low]
Does this lockless read of event->state need a READ_ONCE() annotation?
If another thread sharing the file descriptor calls close(), perf_release() can run concurrently on another CPU and modify event->state using WRITE_ONCE(). Under the kernel memory model, reading a variable that can be concurrently modified typically requires READ_ONCE() to prevent compiler optimizations like tearing or rematerialization, and to avoid data race warnings.
Quoted 5 lines
kernel/events/core.c +2
-0+return ; +/* * We'd expect this to only occur if the irq_work is delayed and either * ctx->task or current has changed in the meantime. This can be the
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Peter Zijlstra
maintainerQuoted 24 lines · Danish Khateeb
A sigtrap event must also set
remove_on_exec, so that its SIGTRAP never reaches a program after exec. But the signal is sent from task work, which only runs on the way back to user space. If the event overflows shortly beforeexecve(), the task work can still be pending when the task entersexecve(), and then runs whenexecve()returns. By thenperf_event_exec()has removed the event and the new program has default signal handlers, so the SIGTRAP kills it.The
exec_stresstest in theremove_on_execselftest catches this and fails about half the time in a VM. A process that opens a sigtrap event on itself and then callsexecve()is killed by SIGTRAP in 15% to 50% of runs, both on an AMD machine running v7.2 and in a VM, with or without close-on-exec on the event fd.
perf_event_exit_event()setsPERF_EVENT_STATE_EXITwhen exec removes the event. If the event fd is close-on-exec and was the last reference to the file, exec also queues the file release as task work. Task work runs newest first, soperf_release()runs before the SIGTRAP work and moves the event on toPERF_EVENT_STATE_DEAD. Exit is already caught by thePF_EXITINGcheck inperf_sigtrap(). So don't send the signal when the event state isPERF_EVENT_STATE_EXITor lower, which means the event has been removed. That also coversPERF_EVENT_STATE_REVOKED, where the PMU is gone.
Quoted 4 lines
Fixes 97ba62b27867 ("perf: Add support for SIGTRAP on perf events")Assisted-by LLMSigned-off-by Danish Khateeb <[email protected]>
Is this the same problem as this one?
From: Danish Khateeb <[email protected]>
Quoted 3 lines · Peter Zijlstra
Is this the same problem as this one?
No. That one is an exec into a non-dumpable binary, where perf_event_exit_task() sets TASK_TOMBSTONE and the WARN fires. This is an ordinary exec: ctx->task doesn't change and the SIGTRAP kills the new program. With Luo's patch on rc5 that still happens (567 of 2000 runs), with this one it doesn't, and Luo's WARN goes away too, since perf_event_exit_task() sets every event to PERF_EVENT_STATE_EXIT.
Thanks
Danish