Re: [PATCH RESEND] perf/core: Fix a refcount leak in attach_perf_ctx_data()

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Sep 22, 2026 at 03:38:35PM +0200, Peter Zijlstra wrote:
> On Sun, Sep 20, 2026 at 04:16:39PM -0700, Namhyung Kim wrote:
> > The attach_perf_ctx_data() can race on global and !global cases.  The
> > global case is protected by global_ctx_data_rwsem and shares a single
> > reference count using perf_ctx_data.global field.
> > 
> > But when it races with !global case, it may miss to set the global field
> > and result in a reference count leak.
> > 
> >  CPU1                              CPU2
> >  ----------------------------------------------------------------
> >  attach_task_ctx_data(global=1)    attach_task_ctx_data(global=0)
> >    cd1 = alloc_perf_ctx_data()       cd2 = alloc_perf_ctx_data()
> >                                        try_cmpxchg() // ok
> >                                        // task->perf_ctx_data = cd2
> > 
> >      try_cmpxchg()            // fail; old = cd2; global = 0
> >      refcount_inc_not_zero()  // cd2->refcount++;
> >      free_perf_ctx_data()     // cd1
> 
> Urgh, took a good while to remember how all that worked. Also, I think
> it might have been clearer written like so:
> 
>   CPU1					CPU2
> 
>   attach_task_ctx_data(.global=1)	attach_task_ctx_data(.global=0)
>     cd1 = alloc_perf_ctx_data();	  cd2 = alloc_perf_ctx_data();
>     					  //	{ .global = 0, .refcount = 1 };
> 
>     					  try_cmpxchg(); // success, 
> 					  // task->perf_ctx_data = cd2
>     try_cmpxhg(); // fail; old = cd2
>     refcount_inc_not_zero(&old->refcount); // success
>       // old.refcount = 2
>     free_perf_ctx_data(cd1);

I see.  I'll do better next time.  Let me know if you want me to resend.

Thanks,
Namhyung

> 
> > Then later detach_global_ctx_data() will see the data but it's not
> > marked as global, so it won't call detach_task_ctx_data().
> > 
> > Fixes: 506e64e710ff ("perf: attach/detach PMU specific data")
> > Assisted-by: Sashiko.dev:Gemini-3.1-pro
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  kernel/events/core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index db7b76d6b68aa55d..e180134bad5e0ae3 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -5454,6 +5454,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 = true;
> >  			free_perf_ctx_data(cd); /* unused */
> >  			return 0;
> >  		}
> 
>