Dropping the last reference to a coresight_device can trigger a kernel
panic on PREEMPT_RT builds due to a "scheduling while atomic" violation.

When the CPU enters an idle state, coresight_cpu_pm_notify() is invoked
with local interrupts disabled. It calls coresight_cpu_get_active_path(),
which currently uses coresight_get_percpu_source_ref() to get a kobject
reference, and then immediately drops it with coresight_put_percpu_source_ref().

If this put_device() call drops the very last reference (e.g., due to a
concurrent device unregistration), it synchronously triggers the release
cascade. On PREEMPT_RT, free_percpu() takes a sleeping rt-mutex.
Furthermore, any parent device in the release chain might also acquire
sleeping locks, causing a system crash in the atomic PM context.

Fix this by eliminating the get/put dance entirely in the PM notifier path.
Since coresight_cpu_pm_notify() runs with IRQs disabled, it is safe to
read the per-cpu source pointer directly under the coresight_dev_lock,
check the mode, and return the path without unnecessarily manipulating the
kobject refcount.

Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 22 ++++++--------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 6d65c43d5..713bcff53 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1850,25 +1850,15 @@ static void coresight_release_device_list(void)
 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;
 
-	/*
-	 * It is expected to run in atomic context or with the CPU lock held for
-	 * sysfs mode, so it cannot be preempted to disable the path. Here
-	 * returns the active path pointer without concern that its state may
-	 * change. Since the build path has taken a reference on the component,
-	 * the path can be safely used by the caller.
-	 */
-	return is_active ? source->path : NULL;
+	return path;
 }
 
 /* Return: 1 if PM is required, 0 if skip, or a negative error */
-- 
2.34.1

On Fri, Jul 17, 2026 at 12:41:54AM +0300, Mohamed Ayman 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;
>  
> -	/*
> -	 * It is expected to run in atomic context or with the CPU lock held for
> -	 * sysfs mode, so it cannot be preempted to disable the path. Here
> -	 * returns the active path pointer without concern that its state may
> -	 * change. Since the build path has taken a reference on the component,
> -	 * the path can be safely used by the caller.
> -	 */

Please keep the comment, as it helps explain why the path pointer can be
returned and safely used by the caller.

> -	return is_active ? source->path : NULL;
> +	return path;

With above update:

Reviewed-by: Leo Yan <leo.yan@arm.com>

Just a thought: we could view this in two stages.

1) The first stage is building the CoreSight path, where we need to
   ensure the involved modules remain bound while the path is being
   established.

2) Once the path has been built and the device mode is enabled, we
   enter the runtime stage. From that point on, observing the device
   mode as enabled guarantees that the associated data structures
   can be accessed safely.

I would leave this to maintainers for a call in case any concerns on
lifetime management.

On Fri, Jul 17, 2026 at 12:41:54AM +0300, Mohamed Ayman 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.

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.

Hope this is clear and makes sense.

Thanks,
Leo

[1] https://sashiko.dev/#/patchset/20260405-arm_coresight_path_power_management_improvement-v10-0-13e94754a8be%40arm.com

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

On Fri, Jul 17, 2026 at 06:12:01PM +0200, Sebastian Andrzej Siewior wrote:

[...]

> > 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?

Fair point.

coresight_cpu_get_active_path() can safely access the source while
holding coresight_dev_lock. Unregistration will be blocked until the
access from the CPU PM notifier has finished.

> > 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.

coresight_device::path is a runtime pointer that is valid only while
the device is enabled.

Since both the path and the device mode are updated atomically on the
local CPU, observing the device mode as enabled guarantees that the
corresponding coresight_device::path is valid.

Thanks,
Leo

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@linutronix.de> 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

On 2026-07-17 00:41:54 [+0300], Mohamed Ayman wrote:
> Dropping the last reference to a coresight_device can trigger a kernel
> panic on PREEMPT_RT builds due to a "scheduling while atomic" violation.

It is not a panic, it is a warning. A panic is after panic() is invoked.
[ Or a warning with /proc/sys/kernel/panic_on_warn=1 but in this case I
wouldn't count this a panic but as warning followed by consequences of
your own action ]

> When the CPU enters an idle state, coresight_cpu_pm_notify() is invoked
> with local interrupts disabled. It calls coresight_cpu_get_active_path(),
> which currently uses coresight_get_percpu_source_ref() to get a kobject
> reference, and then immediately drops it with coresight_put_percpu_source_ref().
> 
> If this put_device() call drops the very last reference (e.g., due to a
> concurrent device unregistration), it synchronously triggers the release
> cascade. On PREEMPT_RT, free_percpu() takes a sleeping rt-mutex.

Again, spinlock_t which becomes a sleeping lock on PREEMPT_RT.

> Furthermore, any parent device in the release chain might also acquire
> sleeping locks, causing a system crash in the atomic PM context.
> 
> Fix this by eliminating the get/put dance entirely in the PM notifier path.
> Since coresight_cpu_pm_notify() runs with IRQs disabled, it is safe to
> read the per-cpu source pointer directly under the coresight_dev_lock,
> check the mode, and return the path without unnecessarily manipulating the
> kobject refcount.

Right.

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>

There is coresight_put_percpu_source_ref() with holds coresight_dev_lock
during a put_cpu. I don't know why the lock is held, it is not obvious
to me. But it will lead to a similar problem if the put actually invokes
the release callback.

Sebastian

Hi Mohamed & Sebastian,

On Fri, Jul 17, 2026 at 10:14:53AM +0200, Sebastian Andrzej Siewior wrote:

[...]

> There is coresight_put_percpu_source_ref() with holds coresight_dev_lock
> during a put_cpu. I don't know why the lock is held, it is not obvious
> to me. But it will lead to a similar problem if the put actually invokes
> the release callback.

Sorry I am a bit late to join the discussion.

Essentially, this is about how to manage a module's lifetime correctly
so that the CPU PM notifier can safely access a device. There is an edge
case where the CPU PM notifier is accessing the device while, at the
same time, the module is being unloaded and the device's resources are
being released.

For now, I'd suggest please hold on this patch. Give me a bit time to
study this thread, I will share back more details.

Thanks,
Leo

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling put_device() inside a raw spinlock atomic context in coresight_put_percpu_source_ref() causes a system crash on PREEMPT_RT.
- [High] Double-decrement of hardware refcounts for links/sinks when a device is unregistered while the CPU is idle.
--

--- Patch [1]: [PATCH v3] coresight: Fix scheduling while atomic in coresight_cpu_pm_notify() ---
commit cbdaaaf0779f6cae5c8595e4a2bcad24b819e017
Author: Mohamed Ayman <mohamedaymanworkspace@gmail.com>

coresight: Fix scheduling while atomic in coresight_cpu_pm_notify()

This commit modifies coresight_cpu_get_active_path() to avoid calling
coresight_get_percpu_source_ref() and coresight_put_percpu_source_ref()
during CPU PM transitions. Instead, it reads the per-cpu source pointer
directly under the coresight_dev_lock raw spinlock to prevent potential
sleeping in atomic context on PREEMPT_RT.

> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d574fd..713bcff53bf4f 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1850,25 +1850,15 @@ static void coresight_release_device_list(void)
>  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);

[Severity: High]
This isn't a bug introduced by this patch, but while this change fixes the
issue for the PM notifier by removing the get/put dance, does the same
"scheduling while atomic" problem still exist in
coresight_put_percpu_source_ref() for other callers?

In coresight_put_percpu_source_ref():

	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
	...
	put_device(&csdev->dev);

This unconditionally acquires coresight_dev_lock as a raw spinlock and calls
put_device(). If concurrent device unregistration occurs, this put_device()
call can drop the final kobject reference, triggering
coresight_device_release().

This calls free_percpu(), which acquires sleepable locks on PREEMPT_RT,
causing a panic. Are there plans to address this remaining put_device()
call under a raw spinlock?

> +	source = per_cpu(csdev_source, smp_processor_id());
> +	if (source && (coresight_get_mode(source) & mode))
> +		path = source->path;
>  
> -	/*
> -	 * It is expected to run in atomic context or with the CPU lock held for
> -	 * sysfs mode, so it cannot be preempted to disable the path. Here
> -	 * returns the active path pointer without concern that its state may
> -	 * change. Since the build path has taken a reference on the component,
> -	 * the path can be safely used by the caller.
> -	 */
> -	return is_active ? source->path : NULL;
> +	return path;
>  }

[Severity: High]
This is a pre-existing issue, but does this PM logic cause a permanent
double-decrement of hardware refcounts if a trace device is unregistered
while the CPU is idle?

When the CPU goes idle, coresight_cpu_pm_notify() runs CPU_PM_ENTER and calls
coresight_pm_save(path). This disables components along the active trace path
and decrements their reference counts.

If the trace driver is concurrently unbound on another CPU,
coresight_unregister() clears the csdev_source per-CPU pointer.

When the idle CPU wakes up, coresight_cpu_pm_notify(CPU_PM_EXIT) calls
coresight_cpu_get_active_path(). Because the pointer was cleared, this
returns NULL.

Consequently, coresight_pm_restore() is skipped, and the hardware refcounts
are never incremented back:

	struct coresight_path *path =
		coresight_cpu_get_active_path(CS_MODE_SYSFS | CS_MODE_PERF);
	int ret;

	ret = coresight_pm_is_needed(path); // returns 0 if path is NULL
	if (ret <= 0)
		return ret ? NOTIFY_BAD : NOTIFY_DONE; // Skips restore

When the trace session is later explicitly closed, coresight_disable_path()
executes and decrements those refcounts again.

Does this cause an underflow that permanently disables shared hardware
components for all cores?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716214155.2049564-1-mohamedaymanworkspace@gmail.com?part=1