Commit 78ed93d7 authored by Marco Elver's avatar Marco Elver Committed by Peter Zijlstra

signal: Deliver SIGTRAP on perf event asynchronously if blocked

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. ]

Fixes: 97ba62b2 ("perf: Add support for SIGTRAP on perf events")
Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarMarco Elver <elver@google.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: default avatarGeert Uytterhoeven <geert@linux-m68k.org>
Tested-by: default avatarDmitry Vyukov <dvyukov@google.com>
Link: https://lore.kernel.org/r/20220404111204.935357-1-elver@google.com
parent 7bebfe9d
...@@ -708,6 +708,7 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x18); ...@@ -708,6 +708,7 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x18);
static_assert(offsetof(siginfo_t, si_pkey) == 0x14); static_assert(offsetof(siginfo_t, si_pkey) == 0x14);
static_assert(offsetof(siginfo_t, si_perf_data) == 0x10); static_assert(offsetof(siginfo_t, si_perf_data) == 0x10);
static_assert(offsetof(siginfo_t, si_perf_type) == 0x14); static_assert(offsetof(siginfo_t, si_perf_type) == 0x14);
static_assert(offsetof(siginfo_t, si_perf_flags) == 0x18);
static_assert(offsetof(siginfo_t, si_band) == 0x0c); static_assert(offsetof(siginfo_t, si_band) == 0x0c);
static_assert(offsetof(siginfo_t, si_fd) == 0x10); static_assert(offsetof(siginfo_t, si_fd) == 0x10);
static_assert(offsetof(siginfo_t, si_call_addr) == 0x0c); static_assert(offsetof(siginfo_t, si_call_addr) == 0x0c);
......
...@@ -1011,6 +1011,7 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x28); ...@@ -1011,6 +1011,7 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x28);
static_assert(offsetof(siginfo_t, si_pkey) == 0x20); static_assert(offsetof(siginfo_t, si_pkey) == 0x20);
static_assert(offsetof(siginfo_t, si_perf_data) == 0x18); static_assert(offsetof(siginfo_t, si_perf_data) == 0x18);
static_assert(offsetof(siginfo_t, si_perf_type) == 0x20); static_assert(offsetof(siginfo_t, si_perf_type) == 0x20);
static_assert(offsetof(siginfo_t, si_perf_flags) == 0x24);
static_assert(offsetof(siginfo_t, si_band) == 0x10); static_assert(offsetof(siginfo_t, si_band) == 0x10);
static_assert(offsetof(siginfo_t, si_fd) == 0x18); static_assert(offsetof(siginfo_t, si_fd) == 0x18);
static_assert(offsetof(siginfo_t, si_call_addr) == 0x10); static_assert(offsetof(siginfo_t, si_call_addr) == 0x10);
......
...@@ -487,6 +487,7 @@ static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18); ...@@ -487,6 +487,7 @@ static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14); static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_perf_data) == 0x10); static_assert(offsetof(compat_siginfo_t, si_perf_data) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_perf_type) == 0x14); static_assert(offsetof(compat_siginfo_t, si_perf_type) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_perf_flags) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c); static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c);
static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10); static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_call_addr) == 0x0c); static_assert(offsetof(compat_siginfo_t, si_call_addr) == 0x0c);
......
...@@ -625,6 +625,7 @@ static inline void siginfo_build_tests(void) ...@@ -625,6 +625,7 @@ static inline void siginfo_build_tests(void)
/* _sigfault._perf */ /* _sigfault._perf */
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_data) != 0x10); BUILD_BUG_ON(offsetof(siginfo_t, si_perf_data) != 0x10);
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_type) != 0x14); BUILD_BUG_ON(offsetof(siginfo_t, si_perf_type) != 0x14);
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_flags) != 0x18);
/* _sigpoll */ /* _sigpoll */
BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c); BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c);
......
...@@ -779,5 +779,6 @@ static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18); ...@@ -779,5 +779,6 @@ static_assert(offsetof(compat_siginfo_t, si_upper) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14); static_assert(offsetof(compat_siginfo_t, si_pkey) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_perf_data) == 0x10); static_assert(offsetof(compat_siginfo_t, si_perf_data) == 0x10);
static_assert(offsetof(compat_siginfo_t, si_perf_type) == 0x14); static_assert(offsetof(compat_siginfo_t, si_perf_type) == 0x14);
static_assert(offsetof(compat_siginfo_t, si_perf_flags) == 0x18);
static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c); static_assert(offsetof(compat_siginfo_t, si_band) == 0x0c);
static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10); static_assert(offsetof(compat_siginfo_t, si_fd) == 0x10);
...@@ -590,5 +590,6 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x28); ...@@ -590,5 +590,6 @@ static_assert(offsetof(siginfo_t, si_upper) == 0x28);
static_assert(offsetof(siginfo_t, si_pkey) == 0x20); static_assert(offsetof(siginfo_t, si_pkey) == 0x20);
static_assert(offsetof(siginfo_t, si_perf_data) == 0x18); static_assert(offsetof(siginfo_t, si_perf_data) == 0x18);
static_assert(offsetof(siginfo_t, si_perf_type) == 0x20); static_assert(offsetof(siginfo_t, si_perf_type) == 0x20);
static_assert(offsetof(siginfo_t, si_perf_flags) == 0x24);
static_assert(offsetof(siginfo_t, si_band) == 0x10); static_assert(offsetof(siginfo_t, si_band) == 0x10);
static_assert(offsetof(siginfo_t, si_fd) == 0x14); static_assert(offsetof(siginfo_t, si_fd) == 0x14);
...@@ -149,8 +149,10 @@ static inline void signal_compat_build_tests(void) ...@@ -149,8 +149,10 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_data) != 0x18); BUILD_BUG_ON(offsetof(siginfo_t, si_perf_data) != 0x18);
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_type) != 0x20); BUILD_BUG_ON(offsetof(siginfo_t, si_perf_type) != 0x20);
BUILD_BUG_ON(offsetof(siginfo_t, si_perf_flags) != 0x24);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_data) != 0x10); BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_data) != 0x10);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_type) != 0x14); BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_type) != 0x14);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf_flags) != 0x18);
CHECK_CSI_OFFSET(_sigpoll); CHECK_CSI_OFFSET(_sigpoll);
CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int)); CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int));
......
...@@ -235,6 +235,7 @@ typedef struct compat_siginfo { ...@@ -235,6 +235,7 @@ typedef struct compat_siginfo {
struct { struct {
compat_ulong_t _data; compat_ulong_t _data;
u32 _type; u32 _type;
u32 _flags;
} _perf; } _perf;
}; };
} _sigfault; } _sigfault;
......
...@@ -320,7 +320,7 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *); ...@@ -320,7 +320,7 @@ int send_sig_mceerr(int code, void __user *, short, struct task_struct *);
int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper); int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper);
int force_sig_pkuerr(void __user *addr, u32 pkey); int force_sig_pkuerr(void __user *addr, u32 pkey);
int force_sig_perf(void __user *addr, u32 type, u64 sig_data); int send_sig_perf(void __user *addr, u32 type, u64 sig_data);
int force_sig_ptrace_errno_trap(int errno, void __user *addr); int force_sig_ptrace_errno_trap(int errno, void __user *addr);
int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno); int force_sig_fault_trapno(int sig, int code, void __user *addr, int trapno);
......
...@@ -99,6 +99,7 @@ union __sifields { ...@@ -99,6 +99,7 @@ union __sifields {
struct { struct {
unsigned long _data; unsigned long _data;
__u32 _type; __u32 _type;
__u32 _flags;
} _perf; } _perf;
}; };
} _sigfault; } _sigfault;
...@@ -164,6 +165,7 @@ typedef struct siginfo { ...@@ -164,6 +165,7 @@ typedef struct siginfo {
#define si_pkey _sifields._sigfault._addr_pkey._pkey #define si_pkey _sifields._sigfault._addr_pkey._pkey
#define si_perf_data _sifields._sigfault._perf._data #define si_perf_data _sifields._sigfault._perf._data
#define si_perf_type _sifields._sigfault._perf._type #define si_perf_type _sifields._sigfault._perf._type
#define si_perf_flags _sifields._sigfault._perf._flags
#define si_band _sifields._sigpoll._band #define si_band _sifields._sigpoll._band
#define si_fd _sifields._sigpoll._fd #define si_fd _sifields._sigpoll._fd
#define si_call_addr _sifields._sigsys._call_addr #define si_call_addr _sifields._sigsys._call_addr
...@@ -270,6 +272,11 @@ typedef struct siginfo { ...@@ -270,6 +272,11 @@ typedef struct siginfo {
* that are of the form: ((PTRACE_EVENT_XXX << 8) | SIGTRAP) * that are of the form: ((PTRACE_EVENT_XXX << 8) | SIGTRAP)
*/ */
/*
* Flags for si_perf_flags if SIGTRAP si_code is TRAP_PERF.
*/
#define TRAP_PERF_FLAG_ASYNC (1u << 0)
/* /*
* SIGCHLD si_codes * SIGCHLD si_codes
*/ */
......
...@@ -6533,7 +6533,7 @@ static void perf_sigtrap(struct perf_event *event) ...@@ -6533,7 +6533,7 @@ static void perf_sigtrap(struct perf_event *event)
if (current->flags & PF_EXITING) if (current->flags & PF_EXITING)
return; return;
force_sig_perf((void __user *)event->pending_addr, send_sig_perf((void __user *)event->pending_addr,
event->attr.type, event->attr.sig_data); event->attr.type, event->attr.sig_data);
} }
......
...@@ -1805,7 +1805,7 @@ int force_sig_pkuerr(void __user *addr, u32 pkey) ...@@ -1805,7 +1805,7 @@ int force_sig_pkuerr(void __user *addr, u32 pkey)
} }
#endif #endif
int force_sig_perf(void __user *addr, u32 type, u64 sig_data) int send_sig_perf(void __user *addr, u32 type, u64 sig_data)
{ {
struct kernel_siginfo info; struct kernel_siginfo info;
...@@ -1817,7 +1817,18 @@ int force_sig_perf(void __user *addr, u32 type, u64 sig_data) ...@@ -1817,7 +1817,18 @@ int force_sig_perf(void __user *addr, u32 type, u64 sig_data)
info.si_perf_data = sig_data; info.si_perf_data = sig_data;
info.si_perf_type = type; info.si_perf_type = type;
return force_sig_info(&info); /*
* Signals generated by perf events should not terminate the whole
* process if SIGTRAP is blocked, however, delivering the signal
* asynchronously is better than not delivering at all. But tell user
* space if the signal was asynchronous, so it can clearly be
* distinguished from normal synchronous ones.
*/
info.si_perf_flags = sigismember(&current->blocked, info.si_signo) ?
TRAP_PERF_FLAG_ASYNC :
0;
return send_sig_info(info.si_signo, &info, current);
} }
/** /**
...@@ -3432,6 +3443,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, ...@@ -3432,6 +3443,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
to->si_addr = ptr_to_compat(from->si_addr); to->si_addr = ptr_to_compat(from->si_addr);
to->si_perf_data = from->si_perf_data; to->si_perf_data = from->si_perf_data;
to->si_perf_type = from->si_perf_type; to->si_perf_type = from->si_perf_type;
to->si_perf_flags = from->si_perf_flags;
break; break;
case SIL_CHLD: case SIL_CHLD:
to->si_pid = from->si_pid; to->si_pid = from->si_pid;
...@@ -3509,6 +3521,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, ...@@ -3509,6 +3521,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
to->si_addr = compat_ptr(from->si_addr); to->si_addr = compat_ptr(from->si_addr);
to->si_perf_data = from->si_perf_data; to->si_perf_data = from->si_perf_data;
to->si_perf_type = from->si_perf_type; to->si_perf_type = from->si_perf_type;
to->si_perf_flags = from->si_perf_flags;
break; break;
case SIL_CHLD: case SIL_CHLD:
to->si_pid = from->si_pid; to->si_pid = from->si_pid;
...@@ -4722,6 +4735,7 @@ static inline void siginfo_buildtime_checks(void) ...@@ -4722,6 +4735,7 @@ static inline void siginfo_buildtime_checks(void)
CHECK_OFFSET(si_pkey); CHECK_OFFSET(si_pkey);
CHECK_OFFSET(si_perf_data); CHECK_OFFSET(si_perf_data);
CHECK_OFFSET(si_perf_type); CHECK_OFFSET(si_perf_type);
CHECK_OFFSET(si_perf_flags);
/* sigpoll */ /* sigpoll */
CHECK_OFFSET(si_band); CHECK_OFFSET(si_band);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment