Re: [tip: perf/core] perf: Fix event_function_call() locking

Hi Peter and Kim,

I tested this patch on top of 6.11.0-rc3-next-20240812. This issue can not be reproduced in syzkaller reproducer.

Best Regards, Thanks!

On 2024-08-13 at 23:02:09 +0200, Peter Zijlstra wrote:

On Tue, Aug 13, 2024 at 11:28:54AM -0700, Namhyung Kim wrote:

Duh, yeah.

...

---

diff --git a/kernel/events/core.c b/kernel/events/core.c index 9893ba5e98aa..85204c2376fa 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -298,13 +298,14 @@ static int event_function(void *info) static void event_function_call(struct perf_event *event, event_f func, void *data) { struct perf_event_context *ctx = event->ctx;

• struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);

• struct perf_cpu_context *cpuctx; struct task_struct *task = READ_ONCE(ctx->task); /* verified in event_function */ struct event_function_struct efs = { .event = event, .func = func, .data = data, };
• unsigned long flags;

if (!event->parent) { /* @@ -327,22 +328,27 @@ static void event_function_call(struct perf_event *event, event_f func, void *da if (!task_function_call(task, event_function, &efs)) return;

• local_irq_save(flags);

This can just be local_irq_disable() though, seeing how the fingered commit replaced raw_spin_lock_irq().

I'll queue the below...

---

Subject: perf: Really fix event_function_call() locking From: Namhyung Kim namhyung@kernel.org Date: Tue Aug 13 22:55:11 CEST 2024

Commit 558abc7e3f89 ("perf: Fix event_function_call() locking") lost IRQ disabling by mistake.

Fixes: 558abc7e3f89 ("perf: Fix event_function_call() locking") Reported-by: Pengfei Xu pengfei.xu@intel.com Reported-by: Naresh Kamboju naresh.kamboju@linaro.org Signed-off-by: Namhyung Kim namhyung@kernel.org Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org

---

kernel/events/core.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)

--- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -298,8 +298,8 @@ static int event_function(void *info) static void event_function_call(struct perf_event *event, event_f func, void *data) { struct perf_event_context *ctx = event->ctx;

• struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context); struct task_struct *task = READ_ONCE(ctx->task); /* verified in event_function */

• struct perf_cpu_context *cpuctx; struct event_function_struct efs = { .event = event, .func = func,

@@ -327,22 +327,25 @@ static void event_function_call(struct p if (!task_function_call(task, event_function, &efs)) return;

• local_irq_disable();
• cpuctx = this_cpu_ptr(&perf_cpu_context); perf_ctx_lock(cpuctx, ctx); /*
  • Reload the task pointer, it might have been changed by
  • a concurrent perf_event_context_sched_out().
  */ task = ctx->task;

• if (task == TASK_TOMBSTONE) {

• perf_ctx_unlock(cpuctx, ctx);
  

• return;
  

• }

• if (task == TASK_TOMBSTONE)

• goto unlock;
  

  if (ctx->is_active) { perf_ctx_unlock(cpuctx, ctx);

• local_irq_enable();
  

  goto again; } func(event, NULL, ctx, data);

+unlock: perf_ctx_unlock(cpuctx, ctx);

• local_irq_enable();

} /*