Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] The patch silently alters the behavior of perf_event_read_local() by explicitly returning -EINVAL for revoked or dead events. Pre-existing issues: - [High] TOCTOU race in `perf_event_read_local()` allows NULL pointer dereference or Use-After-Free of `event->pmu` despite the newly added state check. -- commit 3c330c9904a35189d22ecaa756a6d8b5458cfb9c Author: Vinay Belgaumkar <[email protected]> perf: Fix null pointer access in is_include_guest_event() A kernel panic is observed when an application has a perf connection open and the driver managing the pmu event unbinds. The issue occurs when perf_pmu_unregister() is called while there is an active perf session. The null pointer error is seen when the event is freed through __free_event() and mediated_pmu_unaccount_event is called. There is a missing null check inside is_include_guest_event(). Also call mediated_pmu_unaccount_event() inside __pmu_detach_event() similar to __free_event(). > diff --git a/kernel/events/core.c b/kernel/events/core.c > index a6c8e38a31104..9809fc10c4f51 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -4917,6 +4917,11 @@ int perf_event_read_local(struct perf_event *event, > u64 *value, > goto out; > } > > + if (event->state <= PERF_EVENT_STATE_REVOKED) { > + ret = -EINVAL; > + goto out; > + } > + [Severity: Medium] Does this explicitly change the return value for revoked or dead events to -EINVAL? The commit message describes fixing a null pointer access in is_include_guest_event(), but does not mention altering the behavior of perf_event_read_local() for revoked events. Could this be documented in the commit message? [Severity: High] This is a pre-existing issue, but does this lockless state check leave a time-of-check to time-of-use race window that can still lead to a null pointer dereference of event->pmu? If CPU A calls perf_event_read_local(), disables interrupts, and locklessly checks event->state, it can pass if the event state is PERF_EVENT_STATE_INACTIVE. Concurrently, if CPU B calls perf_pmu_unregister() which leads to __pmu_detach_event(), it will not send an IPI to synchronize with CPU A since the event is inactive. CPU B then sets event->state to PERF_EVENT_STATE_REVOKED and clears event->pmu to NULL. When CPU A continues and calls __perf_event_read_cpu(event, event->cpu), if the event has PERF_EV_CAP_READ_SCOPE, it will dereference event->pmu->scope, which was just set to NULL by CPU B, causing a panic. Can this sequence be synchronized safely? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
