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

[MOHAMED AYMAN]

Hi Leo, Sebastian,

Thank you both for the deep architectural insights.

Leo, your point about the parent device unbinding and tearing down the
data while the PM notifier holds a raw pointer makes perfect sense.
Trading a "scheduling while atomic" panic for a potential
Use-After-Free is certainly not the goal. I clearly see why the raw
spinlock isn't enough to protect the underlying memory lifetime
against a concurrent driver unbind.

I am closely following your discussion regarding the
`device_link_add()` approach. Once you reach a consensus on the best
lifetime model for the active session, please let me know how you
would like to proceed.

I would be more than happy to help implement the new design and
prepare the necessary patches, or I can gladly step back if you prefer
to handle this broader architectural refactoring yourself, Leo.

Thanks again for your time and guidance on this!

Best regards,
Mohamed Ayman

On Fri, Jul 17, 2026 at 7:12 PM Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
>
> 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