Skip to content

Commit 030a976

Browse files
author
Peter Zijlstra
committed
perf: Consider OS filter fail
Some PMUs (notably the traditional hardware kind) have boundary issues with the OS filter. Specifically, it is possible for perf_event_attr::exclude_kernel=1 events to trigger in-kernel due to SKID or errata. This can upset the sigtrap logic some and trigger the WARN. However, if this invalid sample is the first we must not loose the SIGTRAP, OTOH if it is the second, it must not override the pending_addr with a (possibly) invalid one. Fixes: ca6c213 ("perf: Fix missing SIGTRAPs") Reported-by: Pengfei Xu <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Reviewed-by: Marco Elver <[email protected]> Tested-by: Pengfei Xu <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent af169b7 commit 030a976

File tree

1 file changed

+22
-2
lines changed

1 file changed

+22
-2
lines changed

kernel/events/core.c

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9273,6 +9273,19 @@ int perf_event_account_interrupt(struct perf_event *event)
92739273
return __perf_event_account_interrupt(event, 1);
92749274
}
92759275

9276+
static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *regs)
9277+
{
9278+
/*
9279+
* Due to interrupt latency (AKA "skid"), we may enter the
9280+
* kernel before taking an overflow, even if the PMU is only
9281+
* counting user events.
9282+
*/
9283+
if (event->attr.exclude_kernel && !user_mode(regs))
9284+
return false;
9285+
9286+
return true;
9287+
}
9288+
92769289
/*
92779290
* Generic event overflow handling, sampling.
92789291
*/
@@ -9306,14 +9319,21 @@ static int __perf_event_overflow(struct perf_event *event,
93069319
}
93079320

93089321
if (event->attr.sigtrap) {
9322+
/*
9323+
* The desired behaviour of sigtrap vs invalid samples is a bit
9324+
* tricky; on the one hand, one should not loose the SIGTRAP if
9325+
* it is the first event, on the other hand, we should also not
9326+
* trigger the WARN or override the data address.
9327+
*/
9328+
bool valid_sample = sample_is_allowed(event, regs);
93099329
unsigned int pending_id = 1;
93109330

93119331
if (regs)
93129332
pending_id = hash32_ptr((void *)instruction_pointer(regs)) ?: 1;
93139333
if (!event->pending_sigtrap) {
93149334
event->pending_sigtrap = pending_id;
93159335
local_inc(&event->ctx->nr_pending);
9316-
} else if (event->attr.exclude_kernel) {
9336+
} else if (event->attr.exclude_kernel && valid_sample) {
93179337
/*
93189338
* Should not be able to return to user space without
93199339
* consuming pending_sigtrap; with exceptions:
@@ -9330,7 +9350,7 @@ static int __perf_event_overflow(struct perf_event *event,
93309350
}
93319351

93329352
event->pending_addr = 0;
9333-
if (data->sample_flags & PERF_SAMPLE_ADDR)
9353+
if (valid_sample && (data->sample_flags & PERF_SAMPLE_ADDR))
93349354
event->pending_addr = data->addr;
93359355
irq_work_queue(&event->pending_irq);
93369356
}

0 commit comments

Comments
 (0)