On Tue, 5 Apr 2022 at 15:30, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: > On Mon, 4 Apr 2022 at 13:12, Marco Elver <elver@xxxxxxxxxx> wrote: > > With SIGTRAP on perf events, we have encountered termination of > > processes due to user space attempting to block delivery of SIGTRAP. > > Consider this case: > > > > <set up SIGTRAP on a perf event> > > ... > > sigset_t s; > > sigemptyset(&s); > > sigaddset(&s, SIGTRAP | <and others>); > > sigprocmask(SIG_BLOCK, &s, ...); > > ... > > <perf event triggers> > > > > When the perf event triggers, while SIGTRAP is blocked, force_sig_perf() > > will force the signal, but revert back to the default handler, thus > > terminating the task. > > > > This makes sense for error conditions, but not so much for explicitly > > requested monitoring. However, the expectation is still that signals > > generated by perf events are synchronous, which will no longer be the > > case if the signal is blocked and delivered later. > > > > To give user space the ability to clearly distinguish synchronous from > > asynchronous signals, introduce siginfo_t::si_perf_flags and > > TRAP_PERF_FLAG_ASYNC (opted for flags in case more binary information is > > required in future). > > > > The resolution to the problem is then to (a) no longer force the signal > > (avoiding the terminations), but (b) tell user space via si_perf_flags > > if the signal was synchronous or not, so that such signals can be > > handled differently (e.g. let user space decide to ignore or consider > > the data imprecise). > > > > The alternative of making the kernel ignore SIGTRAP on perf events if > > the signal is blocked may work for some usecases, but likely causes > > issues in others that then have to revert back to interception of > > sigprocmask() (which we want to avoid). [ A concrete example: when using > > breakpoint perf events to track data-flow, in a region of code where > > signals are blocked, data-flow can no longer be tracked accurately. > > When a relevant asynchronous signal is received after unblocking the > > signal, the data-flow tracking logic needs to know its state is > > imprecise. ] > > > > Link: https://lore.kernel.org/all/Yjmn%2FkVblV3TdoAq@xxxxxxxxxxxxxxxx/ > > Fixes: 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") > > Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > Signed-off-by: Marco Elver <elver@xxxxxxxxxx> > > Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > I've tested delivery of SIGTRAPs when it's blocked with sigprocmask, > it does not kill the process now. > > And tested the case where previously I was getting infinite recursion > and stack overflow (SIGTRAP handler causes another SIGTRAP recursively > before being able to detect recursion and return). With this patch it > can be handled by blocking recursive SIGTRAPs (!SA_NODEFER). Thanks! Should there be any further comments, please shout. Thanks, -- Marco
