Messages in this thread Patch in this message | | | Date | Fri, 11 Aug 2017 12:18:07 +0100 | From | Mark Rutland <> | Subject | Re: pmu::read() called erroneously in v4.13-rc{3,4} |
| |
On Thu, Aug 10, 2017 at 06:35:51PM +0100, Mark Rutland wrote: > Hi, > > While running Vince's perf fuzzer on arm64 v4.13-rc3, I found we call > pmu::read() for an event whose event::cpu != smp_processor_id(), and > event::oncpu == -1, violating the usual pmu::read() requirements.
It looks like I have an event that wasn't entirely detached from its group_leader in perf_group_detach().
The below diff seems to get rid of the problem, though I think this is masking some futher issues, noted below.
---- diff --git a/kernel/events/core.c b/kernel/events/core.c index 407dad6..cac84b6 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1724,6 +1724,7 @@ static void perf_group_detach(struct perf_event *event) if (event->group_leader != event) { list_del_init(&event->group_entry); event->group_leader->nr_siblings--; + event->group_leader = event; goto out; } ---- If perf_group_detach() iterates over siblings, it re-inits each of their group_leader entries, but doesn't do this if provided a sibling directly. Is that deliberate?
It looks like without the above, we could get into perf_output_read_group(), and follow a stale event->group_leader, read()ing that without checking its state. We check the state of siblings, so shouldn't we check the leader, too?
I'm also confused by perf_output_read_group() when event == leader. AFAICT, in that case we won't read() the event at all, and we'll only read() the siblings. Is that right?
Thanks, Mark.
| |