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

During CPU idle transitions, coresight_cpu_pm_notify() runs with
interrupts disabled. It eventually calls put_device(), which can
synchronously trigger the device's release callback and drop the parent
device's reference. On PREEMPT_RT, free_percpu() takes a sleeping lock
(rt-mutex), and the parent's release callback might also sleep. Sleeping
in this atomic PM context crashes the system.

A previous patch tried deferring just the coresight_device_release() body,
but this still left the synchronous put_device() call dangerously exposed
to sleeping parent release functions.

Fix this by entirely deferring the put_device() call to process context.
We add a pending counter (put_pending) and a work_struct to the coresight
device. When releasing a reference, we increment the counter and queue
the work. A worker thread then safely drains the counter and calls
put_device(). The counter prevents leaking references if multiple puts
are queued before the worker even has a chance to run.

To prevent a use-after-free race condition during module unload, the work
is queued on a dedicated coresight_wq which is safely drained and
destroyed in coresight_exit().

Finally, remove the unnecessary raw_spinlock_irqsave in the put path,
as dropping a reference doesn't require protecting the per-CPU table.

Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 37 ++++++++++++++------
 include/linux/coresight.h                    |  3 ++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 6d65c43d5..e931e6bdc 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -63,6 +63,8 @@ static LIST_HEAD(coresight_dev_idx_list);
 
 static const struct cti_assoc_op *cti_assoc_ops;
 
+static struct workqueue_struct *coresight_wq;
+
 static struct coresight_node *
 coresight_path_first_node(struct coresight_path *path)
 {
@@ -132,6 +134,16 @@ static void coresight_clear_percpu_source(struct coresight_device *csdev)
 	per_cpu(csdev_source, csdev->cpu) = NULL;
 }
 
+static void coresight_put_device_work(struct work_struct *work)
+{
+	struct coresight_device *csdev =
+		container_of(work, struct coresight_device, put_work);
+	int n = atomic_xchg(&csdev->put_pending, 0);
+
+	while (n--)
+		put_device(&csdev->dev);
+}
+
 struct coresight_device *coresight_get_percpu_source_ref(int cpu)
 {
 	struct coresight_device *csdev;
@@ -163,16 +175,9 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
 	if (!csdev || !coresight_is_percpu_source(csdev))
 		return;
 
-	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
+	atomic_inc(&csdev->put_pending);
 
-	/*
-	 * TODO: coresight_device_release() is invoked to release resources when
-	 * the device's refcount reaches zero. It then calls free_percpu(),
-	 * which acquires pcpu_lock — a sleepable lock when PREEMPT_RT is
-	 * enabled. Since the raw spinlock coresight_dev_lock is held, this can
-	 * lead to a potential "scheduling while atomic" issue.
-	 */
-	put_device(&csdev->dev);
+	queue_work(coresight_wq, &csdev->put_work);
 }
 
 struct coresight_device *coresight_get_source(struct coresight_path *path)
@@ -1563,6 +1568,9 @@ coresight_init_device(struct coresight_desc *desc)
 	csdev->dev.release = coresight_device_release;
 	csdev->dev.bus = &coresight_bustype;
 
+	INIT_WORK(&csdev->put_work, coresight_put_device_work);
+	atomic_set(&csdev->put_pending, 0);
+
 	return csdev;
 }
 
@@ -2090,9 +2098,13 @@ static int __init coresight_init(void)
 {
 	int ret;
 
+	coresight_wq = alloc_workqueue("coresight_wq", 0, 0);
+	if (!coresight_wq)
+		return -ENOMEM;
+
 	ret = bus_register(&coresight_bustype);
 	if (ret)
-		return ret;
+		goto exit_wq;
 
 	ret = etm_perf_init();
 	if (ret)
@@ -2121,6 +2133,8 @@ static int __init coresight_init(void)
 	etm_perf_exit();
 exit_bus_unregister:
 	bus_unregister(&coresight_bustype);
+exit_wq:
+	destroy_workqueue(coresight_wq);
 	return ret;
 }
 
@@ -2133,6 +2147,9 @@ static void __exit coresight_exit(void)
 	etm_perf_exit();
 	bus_unregister(&coresight_bustype);
 	coresight_release_device_list();
+
+	if (coresight_wq)
+		destroy_workqueue(coresight_wq);
 }
 
 module_init(coresight_init);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index ddf18c970..589a6c20d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -13,6 +13,7 @@
 #include <linux/perf_event.h>
 #include <linux/sched.h>
 #include <linux/platform_device.h>
+#include <linux/workqueue.h>
 
 /* Peripheral id registers (0xFD0-0xFEC) */
 #define CORESIGHT_PERIPHIDR4	0xfd0
@@ -293,6 +294,8 @@ struct coresight_device {
 	struct csdev_access access;
 	struct device dev;
 	struct coresight_path *path;
+	struct work_struct put_work;
+	atomic_t put_pending;
 	atomic_t mode;
 	int refcnt;
 	int cpu;
-- 
2.34.1

On 2026-07-14 02:00:27 [+0300], Mohamed Ayman wrote:
> Dropping the last reference to a coresight_device triggers a kernel panic
> on PREEMPT_RT builds due to a "scheduling while atomic" violation.
> 
> During CPU idle transitions, coresight_cpu_pm_notify() runs with
> interrupts disabled. It eventually calls put_device(), which can
> synchronously trigger the device's release callback and drop the parent
> device's reference. On PREEMPT_RT, free_percpu() takes a sleeping lock
> (rt-mutex), and the parent's release callback might also sleep. Sleeping

It is a spinlock_t which we refer as a sleeping lock. There is "struct
rt_mutex" which is somehow different.
I would suggest to word it like "uses a spinlock_t for locking which
becomes a sleeping lock on PREEMPT_RT".

> in this atomic PM context crashes the system.
> 
> A previous patch tried deferring just the coresight_device_release() body,
> but this still left the synchronous put_device() call dangerously exposed
> to sleeping parent release functions.
> 
> Fix this by entirely deferring the put_device() call to process context.
> We add a pending counter (put_pending) and a work_struct to the coresight
> device. When releasing a reference, we increment the counter and queue
> the work. A worker thread then safely drains the counter and calls
> put_device(). The counter prevents leaking references if multiple puts
> are queued before the worker even has a chance to run.
> 
> To prevent a use-after-free race condition during module unload, the work
> is queued on a dedicated coresight_wq which is safely drained and
> destroyed in coresight_exit().

Do you have anything that keeps the module-ref counter up with each new
device?

> Finally, remove the unnecessary raw_spinlock_irqsave in the put path,
> as dropping a reference doesn't require protecting the per-CPU table.
> 
> Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
> ---
…
> @@ -163,16 +175,9 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
>  	if (!csdev || !coresight_is_percpu_source(csdev))
>  		return;
>  
> -	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> +	atomic_inc(&csdev->put_pending);
>  
> -	/*
> -	 * TODO: coresight_device_release() is invoked to release resources when
> -	 * the device's refcount reaches zero. It then calls free_percpu(),
> -	 * which acquires pcpu_lock — a sleepable lock when PREEMPT_RT is
> -	 * enabled. Since the raw spinlock coresight_dev_lock is held, this can
> -	 * lead to a potential "scheduling while atomic" issue.
> -	 */
> -	put_device(&csdev->dev);
> +	queue_work(coresight_wq, &csdev->put_work);

What about you keep this as-is and just delay coresight_device_release()
instead?

>  }
>  
>  struct coresight_device *coresight_get_source(struct coresight_path *path)

Sebastian

Hi Sebastian,

Thank you for the review and the feedback.

Regarding the commit message:
You are completely right. I will update the wording in the v4 patch to
explicitly state that it "uses a spinlock_t for locking which becomes
a sleeping lock on PREEMPT_RT" instead of calling it an rt_mutex
directly.

Regarding the module-ref counter:
We don't explicitly bump the module reference count for each new
device. Instead, we rely on `destroy_workqueue(coresight_wq)` inside
`coresight_exit()`. `destroy_workqueue()` synchronously drains all
pending work items before returning, which ensures no deferred puts
are executed after the module is unmapped.

Regarding deferring coresight_device_release() vs put_device():
My initial v1 patch did exactly what you suggested, it only deferred
the body of `coresight_device_release()`. However, it was pointed out
that `put_device()` synchronously recurses into `kobject_cleanup()`,
which invokes the child's release function and immediately afterwards
calls `kobject_put(parent)`.

If we only defer the child's release callback, the `put_device()` call
itself will still execute in the atomic CPU_PM notifier context. If
the parent device's release path acquires any sleeping locks, we will
still hit a "scheduling while atomic" panic. Deferring `put_device()`
entirely protects against this parent cascade.

If it is strictly guaranteed that Coresight parent devices (liike
AMBA) will never sleep during their release paths, I can happily
revert to the simpler approach of just deferring
`coresight_device_release()`.

Would you prefer I revert to deferring just the release function, or
keep the current architecture to safeguard the parent put?

Best regards,,
Mohamed Ayman

On Tue, Jul 14, 2026 at 1:42 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2026-07-14 02:00:27 [+0300], Mohamed Ayman wrote:
> > Dropping the last reference to a coresight_device triggers a kernel panic
> > on PREEMPT_RT builds due to a "scheduling while atomic" violation.
> >
> > During CPU idle transitions, coresight_cpu_pm_notify() runs with
> > interrupts disabled. It eventually calls put_device(), which can
> > synchronously trigger the device's release callback and drop the parent
> > device's reference. On PREEMPT_RT, free_percpu() takes a sleeping lock
> > (rt-mutex), and the parent's release callback might also sleep. Sleeping
>
> It is a spinlock_t which we refer as a sleeping lock. There is "struct
> rt_mutex" which is somehow different.
> I would suggest to word it like "uses a spinlock_t for locking which
> becomes a sleeping lock on PREEMPT_RT".
>
> > in this atomic PM context crashes the system.
> >
> > A previous patch tried deferring just the coresight_device_release() body,
> > but this still left the synchronous put_device() call dangerously exposed
> > to sleeping parent release functions.
> >
> > Fix this by entirely deferring the put_device() call to process context.
> > We add a pending counter (put_pending) and a work_struct to the coresight
> > device. When releasing a reference, we increment the counter and queue
> > the work. A worker thread then safely drains the counter and calls
> > put_device(). The counter prevents leaking references if multiple puts
> > are queued before the worker even has a chance to run.
> >
> > To prevent a use-after-free race condition during module unload, the work
> > is queued on a dedicated coresight_wq which is safely drained and
> > destroyed in coresight_exit().
>
> Do you have anything that keeps the module-ref counter up with each new
> device?
>
> > Finally, remove the unnecessary raw_spinlock_irqsave in the put path,
> > as dropping a reference doesn't require protecting the per-CPU table.
> >
> > Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
> > ---
> …
> > @@ -163,16 +175,9 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
> >       if (!csdev || !coresight_is_percpu_source(csdev))
> >               return;
> >
> > -     guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> > +     atomic_inc(&csdev->put_pending);
> >
> > -     /*
> > -      * TODO: coresight_device_release() is invoked to release resources when
> > -      * the device's refcount reaches zero. It then calls free_percpu(),
> > -      * which acquires pcpu_lock — a sleepable lock when PREEMPT_RT is
> > -      * enabled. Since the raw spinlock coresight_dev_lock is held, this can
> > -      * lead to a potential "scheduling while atomic" issue.
> > -      */
> > -     put_device(&csdev->dev);
> > +     queue_work(coresight_wq, &csdev->put_work);
>
> What about you keep this as-is and just delay coresight_device_release()
> instead?
>
> >  }
> >
> >  struct coresight_device *coresight_get_source(struct coresight_path *path)
>
> Sebastian

On 2026-07-14 22:42:12 [+0300], MOHAMED AYMAN wrote:
> Hi Sebastian,
Hi Mohamed,

> Thank you for the review and the feedback.
> 
> Regarding the commit message:
> You are completely right. I will update the wording in the v4 patch to
> explicitly state that it "uses a spinlock_t for locking which becomes
> a sleeping lock on PREEMPT_RT" instead of calling it an rt_mutex
> directly.
> 
> Regarding the module-ref counter:
> We don't explicitly bump the module reference count for each new
> device. Instead, we rely on `destroy_workqueue(coresight_wq)` inside
> `coresight_exit()`. `destroy_workqueue()` synchronously drains all
> pending work items before returning, which ensures no deferred puts
> are executed after the module is unmapped.

But what stops the module unload before all devices are released?

> Regarding deferring coresight_device_release() vs put_device():
> My initial v1 patch did exactly what you suggested, it only deferred
> the body of `coresight_device_release()`. However, it was pointed out
> that `put_device()` synchronously recurses into `kobject_cleanup()`,
> which invokes the child's release function and immediately afterwards
> calls `kobject_put(parent)`.
> 
> If we only defer the child's release callback, the `put_device()` call
> itself will still execute in the atomic CPU_PM notifier context. If
> the parent device's release path acquires any sleeping locks, we will
> still hit a "scheduling while atomic" panic. Deferring `put_device()`
> entirely protects against this parent cascade.
> 
> If it is strictly guaranteed that Coresight parent devices (liike
> AMBA) will never sleep during their release paths, I can happily
> revert to the simpler approach of just deferring
> `coresight_device_release()`.
> 
> Would you prefer I revert to deferring just the release function, or
> keep the current architecture to safeguard the parent put?

Well deferring as I suggested if the kobj goes away is a not good.
That I part I didn't get: You have the call chain:
|  coresight_cpu_pm_notify() (IRQs off)
|    -> coresight_put_percpu_source_ref()
|      -> put_device()
|        -> coresight_device_release()
|          -> free_percpu()


What you skipped is coresight_cpu_get_active_path() and this one has a
get and a put. Your put has a irqlock on coresight_dev_lock which I am
not sure you need. But more importantly, why is the reference going back
to 0? There would have to be a coresight_clear_percpu_source() in
between, right?

If you could avoid grabbing a reference in the coresight_cpu_pm_notify()
path then we wouldn't have that problem or is there more to it?

> Best regards,,
> Mohamed Ayman

Sebastian

Hi Sebastian,

First Thing WOOOOOOOW, This is a brilliant observation and entirely
changes the approach.

To answer your question regarding how the reference drops to 0 in the
PM notifier:
You are exactly right that the device should have a base reference.
The only way `put_device()` inside the CPU_PM notifier drops the lastt
reference (reaching 0) is if there is a race condition. If another CPU
concurrently calls `coresight_unregister()`, it drops the base
reference. That leaves our CPU's PM notifier holding the absolute last
reference. When our PM notifier calls `put_device()`, it inadvertently
triggers the release cascade from an atomic context.

As you correctly pointed out, `coresight_cpu_get_active_path()` is the
root cause. It lazily reuses `coresight_get_percpu_source_ref()`,
which takes a kobject reference (`get_device()`) intended for path
building.

Since `coresight_cpu_pm_notify()` runs with local IRQs disabled, we do
not need to manipulate the kobject refcount at all. We can simply read
the per-cpu pointer under `coresight_dev_lock`, check the mode, and
return the path.

If we avoid the `get_device()` / `put_device()` entirely in the PM
path, we completely eliminate the bug at its source. We can throw away
all the complex workqueue and pending counter logic from v1/v2.

The fix would simply be rewriting `coresight_cpu_get_active_path()` like this:

static struct coresight_path *coresight_cpu_get_active_path(enum cs_mode mode)
{
struct coresight_device *source;
struct coresight_path *path = NULL;

guard(raw_spinlock_irqsave)(&coresight_dev_lock);

source = per_cpu(csdev_source, smp_processor_id());
if (source && (coresight_get_mode(source) & mode))
path = source->path;

return path;
}

This means we don't need `coresight_put_percpu_source_ref()` inside
the PM path at all.

Does this align with your suggestion !??
If so, I will prepare a v3 patch that drops the workqueue architecture
entirely and just applies this much simpler fix..

Best regards,
Mohamed Ayman

On Wed, Jul 15, 2026 at 9:58 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2026-07-14 22:42:12 [+0300], MOHAMED AYMAN wrote:
> > Hi Sebastian,
> Hi Mohamed,
>
> > Thank you for the review and the feedback.
> >
> > Regarding the commit message:
> > You are completely right. I will update the wording in the v4 patch to
> > explicitly state that it "uses a spinlock_t for locking which becomes
> > a sleeping lock on PREEMPT_RT" instead of calling it an rt_mutex
> > directly.
> >
> > Regarding the module-ref counter:
> > We don't explicitly bump the module reference count for each new
> > device. Instead, we rely on `destroy_workqueue(coresight_wq)` inside
> > `coresight_exit()`. `destroy_workqueue()` synchronously drains all
> > pending work items before returning, which ensures no deferred puts
> > are executed after the module is unmapped.
>
> But what stops the module unload before all devices are released?
>
> > Regarding deferring coresight_device_release() vs put_device():
> > My initial v1 patch did exactly what you suggested, it only deferred
> > the body of `coresight_device_release()`. However, it was pointed out
> > that `put_device()` synchronously recurses into `kobject_cleanup()`,
> > which invokes the child's release function and immediately afterwards
> > calls `kobject_put(parent)`.
> >
> > If we only defer the child's release callback, the `put_device()` call
> > itself will still execute in the atomic CPU_PM notifier context. If
> > the parent device's release path acquires any sleeping locks, we will
> > still hit a "scheduling while atomic" panic. Deferring `put_device()`
> > entirely protects against this parent cascade.
> >
> > If it is strictly guaranteed that Coresight parent devices (liike
> > AMBA) will never sleep during their release paths, I can happily
> > revert to the simpler approach of just deferring
> > `coresight_device_release()`.
> >
> > Would you prefer I revert to deferring just the release function, or
> > keep the current architecture to safeguard the parent put?
>
> Well deferring as I suggested if the kobj goes away is a not good.
> That I part I didn't get: You have the call chain:
> |  coresight_cpu_pm_notify() (IRQs off)
> |    -> coresight_put_percpu_source_ref()
> |      -> put_device()
> |        -> coresight_device_release()
> |          -> free_percpu()
>
>
> What you skipped is coresight_cpu_get_active_path() and this one has a
> get and a put. Your put has a irqlock on coresight_dev_lock which I am
> not sure you need. But more importantly, why is the reference going back
> to 0? There would have to be a coresight_clear_percpu_source() in
> between, right?
>
> If you could avoid grabbing a reference in the coresight_cpu_pm_notify()
> path then we wouldn't have that problem or is there more to it?
>
> > Best regards,,
> > Mohamed Ayman
>
> Sebastian

On 2026-07-16 06:07:35 [+0300], MOHAMED AYMAN wrote:
> Hi Sebastian,
Hi,

> The fix would simply be rewriting `coresight_cpu_get_active_path()` like this:
> 
> static struct coresight_path *coresight_cpu_get_active_path(enum cs_mode mode)
> {
> struct coresight_device *source;
> struct coresight_path *path = NULL;
> 
> guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> 
> source = per_cpu(csdev_source, smp_processor_id());
> if (source && (coresight_get_mode(source) & mode))
> path = source->path;
> 
> return path;
> }
> 
> This means we don't need `coresight_put_percpu_source_ref()` inside
> the PM path at all.
> 
> Does this align with your suggestion !??
> If so, I will prepare a v3 patch that drops the workqueue architecture
> entirely and just applies this much simpler fix..

It looks doable. I don't know what the lifetime expectation is of the
`path' but this returns `path' object on the same rules without the
get/put dance. So yes, why not.

> Best regards,
> Mohamed Ayman

Sebastian

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Queuing work during CPU idle entry wakes a kworker and sets TIF_NEED_RESCHED on a CPU entering deep sleep, violating scheduler invariants.
- [High] A race condition between atomic_inc() and queue_work() leads to a Use-After-Free (UAF) of the coresight device.
--

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

coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()

This commit fixes a "scheduling while atomic" panic on PREEMPT_RT by
deferring the put_device() call to a dedicated workqueue. It introduces
a put_pending atomic counter and a work_struct to queue the release
safely in process context.

> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d574fd..e931e6bdc84fa 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
[ ... ]
> @@ -163,16 +175,9 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
>  	if (!csdev || !coresight_is_percpu_source(csdev))
>  		return;
>  
> -	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> +	atomic_inc(&csdev->put_pending);
>  
> -	/*
> -	 * TODO: coresight_device_release() is invoked to release resources when
> -	 * the device's refcount reaches zero. It then calls free_percpu(),
> -	 * which acquires pcpu_lock — a sleepable lock when PREEMPT_RT is
> -	 * enabled. Since the raw spinlock coresight_dev_lock is held, this can
> -	 * lead to a potential "scheduling while atomic" issue.
> -	 */
> -	put_device(&csdev->dev);
> +	queue_work(coresight_wq, &csdev->put_work);

[Severity: High]
Can a race condition here lead to a use-after-free of the coresight device?

If Thread A calls atomic_inc() and is preempted before calling queue_work(),
and Thread B then calls atomic_inc() and queue_work(), the worker thread
might execute coresight_put_device_work(), read the combined pending count,
and free the device before Thread A resumes.

When Thread A resumes, would it execute queue_work() using a pointer that
now resides in freed memory?

[Severity: High]
Does queuing work here violate scheduler invariants during CPU idle entry?

Since coresight_cpu_pm_notify() calls this during cpu_pm_enter() with
interrupts disabled, the CPU is already committed to sleeping. Calling
queue_work() on a bound workqueue wakes a local kworker, adding it to the
runqueue and setting TIF_NEED_RESCHED on the current CPU.

Will the CPU enter a deep hardware sleep with a runnable task on its runqueue,
delaying the execution of the kworker until an external hardware interrupt
wakes the CPU and breaking the invariant that nr_running must be 0 when
entering idle?

>  }

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