Re: [tip: perf/urgent] perf/core: Detach event groups during remove_on_exec

[Taeyang Lee]

Hi Ian,

I agree that the prctl enable/disable path has a real lock inversion:
perf_event_task_enable()/disable() takes owner->perf_event_mutex before
event->ctx->mutex, while perf_event_remove_on_exec() takes ctx->mutex
before perf_remove_from_owner(), which then takes owner->perf_event_mutex.

This appears pre-existing from the remove_on_exec path rather than being
introduced by 037a3c43edfb; the recent patch changes the detach flags after
perf_remove_from_owner() and does not change this lock ordering. I agree
this should be investigated/fixed separately.

I don't think close(fd) has the same ordering issue, since
perf_event_release_kernel() drops owner->perf_event_mutex before taking
ctx->mutex, but the prctl path is enough for the AB-BA.

Thanks,

On Fri, Jul 10, 2026 at 2:51 AM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> On Fri, Jul 3, 2026 at 3:02 AM tip-bot2 for Taeyang Lee
> <tip-bot2@xxxxxxxxxxxxx> wrote:
> >
> > The following commit has been merged into the perf/urgent branch of tip:
> >
> > Commit-ID:     037a3c43edfb597665dd34457cd22b14692f2ba3
> > Gitweb:        https://git.kernel.org/tip/037a3c43edfb597665dd34457cd22b14692f2ba3
> > Author:        Taeyang Lee <0wn@xxxxxxxxx>
> > AuthorDate:    Sun, 14 Jun 2026 23:22:18 +09:00
> > Committer:     Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > CommitterDate: Thu, 02 Jul 2026 13:21:48 +02:00
> >
> > perf/core: Detach event groups during remove_on_exec
> >
> > perf_event_remove_on_exec() removes events by calling
> > perf_event_exit_event(). For top-level events, this removes the event from
> > the context with DETACH_EXIT only.
> >
> > This can leave inconsistent group state when a removed event is a group
> > leader and the group contains siblings without remove_on_exec. If the group
> > was active, the surviving siblings can remain active and attached to the
> > removed leader's sibling list, but are no longer represented by a valid
> > group leader on the PMU context active lists.
> >
> > A later close of the removed leader uses DETACH_GROUP and can promote the
> > still-active siblings from this stale group state. The next schedule-in can
> > then add an already-linked active_list entry again, corrupting the PMU
> > context active list.
> >
> > With DEBUG_LIST enabled, this is caught as a list_add double-add in
> > merge_sched_in().
> >
> > Fix this by detaching group relationships when remove_on_exec removes an
> > event. This preserves the existing task-exit and revoke behavior, while
> > ensuring surviving siblings are ungrouped before the removed event leaves
> > the context.
> >
> > Fixes: 2e498d0a74e5 ("perf: Add support for event removal on exec")
> > Signed-off-by: Taeyang Lee <0wn@xxxxxxxxx>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> > Link: https://patch.msgid.link/ai65GgZcC0LAlWLG@Taeyangs-MacBook-Pro.local
>
> Hi Peter, Taeyang and Marco,
>
> AI code review has flagged this code as introducing, or at least being
> adjacent, to an existing deadlock. An example of the problem is with
> an event being enabled at the same time as perf_event_remove_on_exec
> is running due to the user code doing an exec. It seems possible for 1
> CPU to hold the perf_event_mutex and be trying to get the
> perf_event_context->mutex while the other CPU has the
> perf_event_context->mutex and is trying to get the perf_event_mutex. A
> fuller AI-generated analysis is below, and it seems correct but I may
> be missing something. The fixes for this look somewhat large so I
> thought I'd flag the issue before trying anything.
>
> Thanks,
> Ian
>
> ## Executive Summary
>
> The reported potential AB-BA deadlock between
> `owner->perf_event_mutex` (class `&task_struct->perf_event_mutex`) and
> `ctx->mutex` (class `&perf_event_context->mutex`) is **PRESENT** in
> the current tip-of-tree code in `torvalds/master`.
>
> The lock inversion was introduced in 2021 by commit `2e498d0a74e5`
> ("perf: Add support for event removal on exec") and remains in the
> code after the recent fix in commit `037a3c43edfb` ("perf/core: Detach
> event groups during remove_on_exec").
>
> The use of `SINGLE_DEPTH_NESTING` in `perf_remove_from_owner()`
> suppresses the Lockdep warning for this specific inversion but **fails
> to prevent the actual CPU deadlock at runtime**.
>
> ## Lock Inversion Paths
>
> The deadlock can be triggered by Process A (Owner) and Process B
> (Monitored Task) executing concurrent system calls.
>
> ### Process A Paths (Normal Lock Order: `owner->perf_event_mutex` ->
> `ctx->mutex`)
>
> #### Path A1: `prctl(PR_TASK_PERF_EVENTS_ENABLE / DISABLE)`
> Process A invokes `prctl` to enable or disable events it owns.
> 1. **`perf_event_task_enable()`** (or `disable`):
>    - Acquires `current->perf_event_mutex` (which is **`A->perf_event_mutex`**).
>    - Iterates over `A->perf_event_list`.
>    - For an event monitoring Process B, calls **`perf_event_ctx_lock(event)`**.
> 2. **`perf_event_ctx_lock_nested()`**:
>    - Acquires **`event->ctx->mutex`** (which is **`CtxB->mutex`**).
>
> **Result:** `A->perf_event_mutex` → `CtxB->mutex`.
>
> #### Path A2: `close(event_fd)`
> Process A (or any task holding a reference to the event fd) closes the
> event file descriptor.
> 1. **`perf_release()`** -> **`perf_event_release_kernel()`**:
>    - Calls **`perf_remove_from_owner(event)`**.
> 2. **`perf_remove_from_owner()`**:
>    - Reads `event->owner` (which is Process A).
>    - Acquires **`owner->perf_event_mutex`** (which is
> **`A->perf_event_mutex`**).
> 3. **`perf_event_release_kernel()`** continues:
>    - Calls **`perf_event_ctx_lock(event)`**.
> 4. **`perf_event_ctx_lock_nested()`**:
>    - Acquires **`event->ctx->mutex`** (which is **`CtxB->mutex`**).
>
> **Result:** `A->perf_event_mutex` → `CtxB->mutex`.
>
> ### Process B Path (Inverted Lock Order: `ctx->mutex` ->
> `owner->perf_event_mutex`)
>
> #### Path B1: `execve()`
> Process B invokes `execve` to execute a new program image. The events
> monitoring Process B that have the `attr.remove_on_exec = 1` bit set
> are removed.
> 1. **`begin_new_exec()`** -> **`perf_event_exec()`** ->
> **`perf_event_remove_on_exec()`**:
>    - Acquires **`ctx->mutex`** (which is **`CtxB->mutex`**).
>    - Iterates over `CtxB->event_list`.
>    - Finds an event with `attr.remove_on_exec = 1` (owned by Process A).
>    - Calls **`perf_remove_from_owner(event)`**.
> 2. **`perf_remove_from_owner()`**:
>    - Reads `event->owner` (which is Process A).
>    - Acquires **`owner->perf_event_mutex`** (which is
> **`A->perf_event_mutex`**) via
> `mutex_lock_nested(&owner->perf_event_mutex, SINGLE_DEPTH_NESTING)`.
>
> **Result:** `CtxB->mutex` → `A->perf_event_mutex`.
>
> ## Concrete Deadlock Interleaving
>
> If Process A and Process B execute Path A1 (or A2) and Path B1
> concurrently on different CPUs, the following interleaving causes a
> CPU deadlock:
>
> | CPU 0 (Process A) | CPU 1 (Process B) |
> | :--- | :--- |
> | Enters `perf_event_task_enable()` | Enters `perf_event_remove_on_exec()` |
> | Acquires **`A->perf_event_mutex`** | Acquires **`CtxB->mutex`** |
> | Calls `perf_event_ctx_lock(event)` | Calls `perf_remove_from_owner(event)` |
> | Tries to acquire **`CtxB->mutex`**<br>**(BLOCKS on CPU 1)** | Tries
> to acquire **`A->perf_event_mutex`**<br>**(BLOCKS on CPU 0)** |
>
> Both processes are now blocked waiting for each other, resulting in a
> permanent system hang (or soft lockup) on the affected cores.
>
> ## Analysis of `SINGLE_DEPTH_NESTING`
>
> In `perf_remove_from_owner()`, the lock is acquired as:
> ```c
> mutex_lock_nested(&owner->perf_event_mutex, SINGLE_DEPTH_NESTING);
> ```
> Where `SINGLE_DEPTH_NESTING` is defined as `1`.
>
> This annotation instructs Lockdep to treat this acquisition as being
> at subclass 1. Since Path A acquires `A->perf_event_mutex` at the
> default subclass 0, Lockdep's dependency graph observes the sequence
> as:
> - Path A: `Class X (Subclass 0)` → `Class Y (Subclass 0)`
> - Path B: `Class Y (Subclass 0)` → `Class X (Subclass 1)`
>
> Because `X_0` and `X_1` are treated as distinct nodes for cycle
> detection in Lockdep to permit parent-child hierarchical locking, this
> annotation **suppresses the Lockdep "possible circular locking
> dependency detected" warning**.
>
> However, the hardware and the actual runtime mutex implementation
> (`kernel/locking/mutex.c`) strip the subclass argument in non-Lockdep
> builds, and even in Lockdep builds, mutual exclusion is enforced
> purely based on the physical memory address of the `struct mutex`.
> Since both paths target the EXACT SAME memory address for
> `A->perf_event_mutex` and `CtxB->mutex`, the CPU-level deadlock
> remains unaffected by the annotation.
>
> The accompanying comment in `perf_remove_from_owner()`:
> ```c
> /*
>  * If we're here through perf_event_exit_task() we're already
>  * holding ctx->mutex which would be an inversion wrt. the
>  * normal lock order.
>  *
>  * However we can safely take this lock because its the child
>  * ctx->mutex.
>  */
> ```
> is a historical artifact (dating back to commit `f63a8daa5812` in
> 2015) and does not hold true for the `remove_on_exec` path introduced
> in 2021 by commit `2e498d0a74e5`, as Process B is the target profilee
> and not necessarily a child process of the owner Task A, and no
> hierarchical relationship protects these specific lock instances.
>
> ## Conclusion
>
> The AB-BA deadlock between `owner->perf_event_mutex` and `ctx->mutex`
> is **VERIFIED** to be present in the current tip-of-tree kernel.
>
> > ---
> >  kernel/events/core.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 954c36e..d7f3e2c 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -4729,7 +4729,7 @@ static void perf_remove_from_owner(struct perf_event *event);
> >  static void perf_event_exit_event(struct perf_event *event,
> >                                   struct perf_event_context *ctx,
> >                                   struct task_struct *task,
> > -                                 bool revoke);
> > +                                 unsigned long detach_flags);
> >
> >  /*
> >   * Removes all events from the current task that have been marked
> > @@ -4756,7 +4756,7 @@ static void perf_event_remove_on_exec(struct perf_event_context *ctx)
> >
> >                 modified = true;
> >
> > -               perf_event_exit_event(event, ctx, ctx->task, false);
> > +               perf_event_exit_event(event, ctx, ctx->task, DETACH_GROUP);
> >         }
> >
> >         raw_spin_lock_irqsave(&ctx->lock, flags);
> > @@ -12937,7 +12937,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event,
> >         /*
> >          * De-schedule the event and mark it REVOKED.
> >          */
> > -       perf_event_exit_event(event, ctx, ctx->task, true);
> > +       perf_event_exit_event(event, ctx, ctx->task, DETACH_REVOKE);
> >
> >         /*
> >          * All _free_event() bits that rely on event->pmu:
> > @@ -14525,12 +14525,13 @@ static void
> >  perf_event_exit_event(struct perf_event *event,
> >                       struct perf_event_context *ctx,
> >                       struct task_struct *task,
> > -                     bool revoke)
> > +                     unsigned long detach_flags)
> >  {
> >         struct perf_event *parent_event = event->parent;
> > -       unsigned long detach_flags = DETACH_EXIT;
> >         unsigned int attach_state;
> >
> > +       detach_flags |= DETACH_EXIT;
> > +
> >         if (parent_event) {
> >                 /*
> >                  * Do not destroy the 'original' grouping; because of the
> > @@ -14553,8 +14554,8 @@ perf_event_exit_event(struct perf_event *event,
> >                         sync_child_event(event, task);
> >         }
> >
> > -       if (revoke)
> > -               detach_flags |= DETACH_GROUP | DETACH_REVOKE;
> > +       if (detach_flags & DETACH_REVOKE)
> > +               detach_flags |= DETACH_GROUP;
> >
> >         perf_remove_from_context(event, detach_flags);
> >         /*
> > @@ -14642,7 +14643,7 @@ static void perf_event_exit_task_context(struct task_struct *task, bool exit)
> >                 perf_event_task(task, ctx, 0);
> >
> >         list_for_each_entry_safe(child_event, next, &ctx->event_list, event_entry)
> > -               perf_event_exit_event(child_event, ctx, exit ? task : NULL, false);
> > +               perf_event_exit_event(child_event, ctx, exit ? task : NULL, 0);
> >
> >         mutex_unlock(&ctx->mutex);
> >
> >



-- 
___

Taeyang Lee Researcher
Theori, Inc. / Xint Code
Website. www.theori.io
Email. 0wn@xxxxxxxxx