Re: [PATCH v3] coresight: Fix scheduling while atomic in coresight_cpu_pm_notify()

[Sebastian Andrzej Siewior]

On 2026-07-17 16:58:48 [+0100], Leo Yan wrote:
> >  static struct coresight_path *coresight_cpu_get_active_path(enum cs_mode mode)
> >  {
> >  	struct coresight_device *source;
> > -	bool is_active = false;
> > +	struct coresight_path *path = NULL;
> >  
> > -	source = coresight_get_percpu_source_ref(smp_processor_id());
> > -	if (!source)
> > -		return NULL;
> > -
> > -	if (coresight_get_mode(source) & mode)
> > -		is_active = true;
> > +	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> >  
> > -	coresight_put_percpu_source_ref(source);
> > +	source = per_cpu(csdev_source, smp_processor_id());
> > +	if (source && (coresight_get_mode(source) & mode))
> > +		path = source->path;
> 
> I agree the get_device()/put_device() pair in
> coresight_cpu_get_active_path() is not a good fit for CPU PM notifier,
> because the put_device() can become the final put while IRQ is disabled.
> 
> However, my understanding is this patch might cause UAF issue that the
> existing code is intended to prevent.
> 
> The raw spinlock (coresight_dev_lock) serializes access to the per-CPU
> csdev_source pointer. It does not guarantee the lifetime of the source
> or its _parent_ device. coresight_unregister() is not only reached from
> module unload; it can also be called when a driver is unbind, for
> example DT overlay removal or device hotplug/unplug.

But doesn't coresight_unregister() block on the coresight_dev_lock here?

> This is why the UAF issue Sashiko mentioned in patch 03 of [1]. A built
> CoreSight path currently grabs references for the path components, which
> keeps module alive, but that is not the same as preventing the parent
> device/driver from being unbound and tearing down CoreSight device data
> while an active session still has raw pointers.
> 
> There are also similar race window before the path is built: for
> example etm_setup_aux() has to look up source/sink state before
> coresight_build_path() establishes the path, so it might access
> released source/sink data if device is unbound.
> 
> I think a proper fix needs a clearer lifetime model for an active
> session. E.g., we could consider to call device_link_add() to prevent
> device unbind / unregister, and unlink device when the session is
> finished. Once that is in place, the CPU PM notifier can safely use the
> active path without get_device()/put_device() pair anymore.

Right. I am also not sure about lifetime of coresight_device::path.

> Hope this is clear and makes sense.
> 
> Thanks,
> Leo

Sebastian