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

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Sep 22, 2026 at 09:47:21PM -0700, Namhyung Kim wrote:
> 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.

Nah, I'll make a few edits and stuff it in a git tree somewhere.

Thanks!