Re: [GIT PULL] Performance events changes for v7.1

Hello,

It seems sashio removes receipients in a reply.  Added PeterZ.

On Mon, Apr 13, 2026 at 09:41:09AM +0000, [email protected] wrote:
> Sashiko AI review found 2 potential issue(s):
> - [Critical] Pre-existing Stack Buffer Overflow in AMD IBS NMI handler on CPUs supporting `IBS_CAPS_OPDATA4`.
> - [High] Reference count leak of `perf_ctx_data` in `attach_global_ctx_data()` when racing with a concurrent non-global context creation.
> --
> 
> commit 167b4d230f2ffe8284d5afdbd849ef2f4f05ef9a
> Author: Ingo Molnar <[email protected]>
> 
> Performance events changes for v7.1
> 
> This commit pulls in various performance event changes for v7.1. It includes
> an optimization to allocate task_ctx_data quickly on large systems, updates
> and fixes for the AMD PMU driver IBS support, Intel PEBS constraint
> validation, and MSR driver changes.
> 
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -5487,6 +5487,12 @@ attach_global_ctx_data(struct kmem_cache *ctx_cache)
> >  					cd = NULL;
> >  			}
> >  			if (!cd) {
> > +				/*
> > +				 * Try to allocate context quickly before
> > +				 * traversing the whole thread list again.
> > +				 */
> > +				if (!attach_task_ctx_data(p, ctx_cache, true, GFP_NOWAIT))
> > +					continue;
> >  				get_task_struct(p);
> >  				goto alloc;
> >  			}
> 
> Could the new GFP_NOWAIT fast-path in attach_global_ctx_data() leak the
> perf_ctx_data reference if it races with a concurrent non-global context
> creation?
> 
> If attach_task_ctx_data() races with a per-task event attaching a non-global
> context, it will find an existing context where old is not NULL. It increments
> old->refcount and returns 0, but intentionally ignores the global argument
> and does not set old->global to 1.
> 
> Because it returns success, attach_global_ctx_data() executes continue and
> skips to the next thread. The context for the thread is left with an
> extra reference but global remains 0.

Right, it looks like an existing problem.  It should set the old->global
like below:

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 95d7a3e57268b6b4..af773bc750495706 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5447,6 +5447,8 @@ attach_task_ctx_data(struct task_struct *task, struct kmem_cache *ctx_cache,
                }
 
                if (refcount_inc_not_zero(&old->refcount)) {
+                       if (global)
+                               old->global = 1;
                        free_perf_ctx_data(cd); /* unused */
                        return 0;
                }

> 
> Later, when the global context is detached, __detach_global_ctx_data()
> iterates over all threads and only calls detach_task_ctx_data(p) if
> cd and cd->global are true. Since cd->global is 0, the context is skipped,
> permanently leaking the perf_ctx_data allocation.
> 
> > @@ -5497,7 +5503,7 @@ attach_global_ctx_data(struct kmem_cache *ctx_cache)
> >  
> >  	return 0;
> >  alloc:
> > -	ret = attach_task_ctx_data(p, ctx_cache, true);
> > +	ret = attach_task_ctx_data(p, ctx_cache, true, GFP_KERNEL);
> >  	put_task_struct(p);
> >  	if (ret) {
> >  		__detach_global_ctx_data();
> 
> Can this result in a double-increment leak? If the same race occurs,
> attach_task_ctx_data() bumps the refcount without setting global to 1 and
> returns success. Then goto again restarts the loop.
> 
> When the loop reaches the thread again, it evaluates:
> 
> 	cd = rcu_dereference(p->perf_ctx_data);
> 	if (cd && !cd->global) {
> 		cd->global = 1;
> 		refcount_inc_not_zero(&cd->refcount);
> 	}
> 
> This bumps the refcount a second time. Since __detach_global_ctx_data()
> only decrements once, the extra reference is leaked.

Looks correct and should be fixed by the above change as well.  I'll
send a formal patch if you're ok.

Thanks,
Namhyung