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

[Ian Rogers]

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);
>
>