All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] coresight: Fix scheduling while atomic in coresight_device_release()
@ 2026-07-12 21:04 Mohamed Ayman
  2026-07-12 21:17 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Mohamed Ayman @ 2026-07-12 21:04 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Sebastian Andrzej Siewior, Clark Williams,
	Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT
  Cc: Mohamed Ayman, moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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 (atomic context). This function eventually
calls coresight_put_percpu_source_ref(), which drops the device reference
via put_device(). If this is the last reference, it triggers the release
chain:

  coresight_cpu_pm_notify() (IRQs off)
    -> coresight_put_percpu_source_ref()
      -> put_device()
        -> coresight_device_release()
          -> free_percpu()

On a PREEMPT_RT kernel, free_percpu() acquires pcpu_lock, which is
implemented as a sleeping rt-mutex. Sleeping while in an atomic context
causes a system crash.

Fix this by deferring the teardown of the coresight_device to process
context. Add a work_struct to `struct coresight_device` and use
schedule_work() inside coresight_device_release() to safely execute
free_percpu() and kfree() in a worker thread, away from the atomic PM
notifier path.

Additionally, remove the redundant raw_spinlock_irqsave guard in
coresight_put_percpu_source_ref(). The lock was originally intended to
protect the per-CPU pointer table, but dropping a reference does not
touch this table.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 6d65c43d5..9dbdb2977 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -165,13 +165,6 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
 
 	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
 
-	/*
-	 * 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);
 }
 
@@ -1257,13 +1250,30 @@ static void coresight_clear_default_sink(struct coresight_device *csdev)
 	}
 }
 
+static void coresight_device_release_work(struct work_struct *work)
+{
+	struct coresight_device *csdev =
+		container_of(work, struct coresight_device, free_work);
+
+	free_percpu(csdev->perf_sink_id_map.cpu_map);
+	kfree(csdev);
+}
+
 static void coresight_device_release(struct device *dev)
 {
 	struct coresight_device *csdev = to_coresight_device(dev);
 
 	fwnode_handle_put(csdev->dev.fwnode);
-	free_percpu(csdev->perf_sink_id_map.cpu_map);
-	kfree(csdev);
+
+	/*
+	 * This release callback can run with the last reference dropped
+	 * from atomic/IRQs-off context (e.g. coresight_put_percpu_source_ref()
+	 * called from the CPU_PM notifier). free_percpu() takes pcpu_lock,
+	 * which is a sleeping lock under PREEMPT_RT, so defer the actual
+	 * teardown to process context.
+	 */
+	INIT_WORK(&csdev->free_work, coresight_device_release_work);
+	schedule_work(&csdev->free_work);
 }
 
 static int coresight_orphan_match(struct device *dev, void *data)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index ddf18c970..63253f6c4 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,7 @@ struct coresight_device {
 	struct csdev_access access;
 	struct device dev;
 	struct coresight_path *path;
+	struct work_struct free_work;
 	atomic_t mode;
 	int refcnt;
 	int cpu;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] coresight: Fix scheduling while atomic in coresight_device_release()
  2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
@ 2026-07-12 21:17 ` sashiko-bot
  2026-07-13  3:07 ` Jie Gan
  2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
  2 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-12 21:17 UTC (permalink / raw)
  To: Mohamed Ayman; +Cc: linux-rt-devel, linux-kernel

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

New issues:
- [High] Asynchronous device release via the global workqueue lacks module exit synchronization, leading to a Use-After-Free.

Pre-existing issues:
- [High] The patch commit message claims to remove a redundant raw_spinlock_irqsave guard, but fails to actually do so.
- [High] Deferring coresight_device_release does not make put_device() safe to call from an atomic context.
--

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

coresight: Fix scheduling while atomic in coresight_device_release()

This patch attempts to fix a scheduling while atomic bug on PREEMPT_RT builds
by deferring coresight_device teardown to a workqueue. It also mentions
removing a redundant raw spinlock guard.

> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d574fd..9dbdb29770260 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -165,13 +165,6 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
>  
>  	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
>  
> -	/*
> -	 * 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);

[Severity: High]
This is a pre-existing issue, but the commit message states it will
"remove the redundant raw_spinlock_irqsave guard in
coresight_put_percpu_source_ref()". However, the diff only removes
the comment block.

Since the guard remains intact, won't put_device() still execute with
coresight_dev_lock held, continuing to trigger the atomic violation?

[Severity: High]
This is also a pre-existing issue, but is it safe to call put_device()
from an atomic context even if the release callback is deferred?

If this is the last reference to the parent device (such as an AMBA or
platform device), kobject_cleanup() can synchronously drop the parent
reference:

put_device()
  kobject_put()
    kobject_cleanup()
      kobject_put(parent)

If the parent's release function acquires sleepable locks, could this
still result in an atomic sleep bug? Would it be safer to defer the
put_device() call itself to process context?

>  }

[ ... ]

> +static void coresight_device_release_work(struct work_struct *work)
> +{
> +	struct coresight_device *csdev =
> +		container_of(work, struct coresight_device, free_work);
> +
> +	free_percpu(csdev->perf_sink_id_map.cpu_map);
> +	kfree(csdev);
> +}
> +
>  static void coresight_device_release(struct device *dev)
>  {
>  	struct coresight_device *csdev = to_coresight_device(dev);
>  
>  	fwnode_handle_put(csdev->dev.fwnode);
> -	free_percpu(csdev->perf_sink_id_map.cpu_map);
> -	kfree(csdev);
> +
> +	/*
> +	 * This release callback can run with the last reference dropped
> +	 * from atomic/IRQs-off context (e.g. coresight_put_percpu_source_ref()
> +	 * called from the CPU_PM notifier). free_percpu() takes pcpu_lock,
> +	 * which is a sleeping lock under PREEMPT_RT, so defer the actual
> +	 * teardown to process context.
> +	 */
> +	INIT_WORK(&csdev->free_work, coresight_device_release_work);
> +	schedule_work(&csdev->free_work);

[Severity: High]
Could scheduling this work on the global system_percpu_wq lead to a
use-after-free during module unload?

If the coresight module is unloaded while there is pending release work,
coresight_exit() does not appear to flush this workqueue or wait for
pending tasks to complete. 

If the module is unmapped before the asynchronous work executes, won't
the worker thread attempt to execute the unmapped 
coresight_device_release_work() address?

>  }

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] coresight: Fix scheduling while atomic in coresight_device_release()
  2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
  2026-07-12 21:17 ` sashiko-bot
@ 2026-07-13  3:07 ` Jie Gan
  2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
  2 siblings, 0 replies; 10+ messages in thread
From: Jie Gan @ 2026-07-13  3:07 UTC (permalink / raw)
  To: Mohamed Ayman, Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Sebastian Andrzej Siewior, Clark Williams,
	Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT



On 7/13/2026 5:04 AM, 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.
> 
> When the CPU enters an idle state, coresight_cpu_pm_notify() is invoked
> with local interrupts disabled (atomic context). This function eventually
> calls coresight_put_percpu_source_ref(), which drops the device reference
> via put_device(). If this is the last reference, it triggers the release
> chain:
> 
>    coresight_cpu_pm_notify() (IRQs off)
>      -> coresight_put_percpu_source_ref()
>        -> put_device()
>          -> coresight_device_release()
>            -> free_percpu()
> 
> On a PREEMPT_RT kernel, free_percpu() acquires pcpu_lock, which is
> implemented as a sleeping rt-mutex. Sleeping while in an atomic context
> causes a system crash.
> 
> Fix this by deferring the teardown of the coresight_device to process
> context. Add a work_struct to `struct coresight_device` and use
> schedule_work() inside coresight_device_release() to safely execute
> free_percpu() and kfree() in a worker thread, away from the atomic PM
> notifier path.
> 
> Additionally, remove the redundant raw_spinlock_irqsave guard in

The code diff doesnt include this description.

> coresight_put_percpu_source_ref(). The lock was originally intended to
> protect the per-CPU pointer table, but dropping a reference does not
> touch this table.
> 
> Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 28 +++++++++++++-------
>   include/linux/coresight.h                    |  2 ++
>   2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d5..9dbdb2977 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -165,13 +165,6 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
>   
>   	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
>   
> -	/*
> -	 * 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);
>   }
>   
> @@ -1257,13 +1250,30 @@ static void coresight_clear_default_sink(struct coresight_device *csdev)
>   	}
>   }
>   
> +static void coresight_device_release_work(struct work_struct *work)
> +{
> +	struct coresight_device *csdev =
> +		container_of(work, struct coresight_device, free_work);
> +
> +	free_percpu(csdev->perf_sink_id_map.cpu_map);
> +	kfree(csdev);
> +}
> +
>   static void coresight_device_release(struct device *dev)
>   {
>   	struct coresight_device *csdev = to_coresight_device(dev);
>   
>   	fwnode_handle_put(csdev->dev.fwnode);
> -	free_percpu(csdev->perf_sink_id_map.cpu_map);
> -	kfree(csdev);
> +
> +	/*
> +	 * This release callback can run with the last reference dropped
> +	 * from atomic/IRQs-off context (e.g. coresight_put_percpu_source_ref()
> +	 * called from the CPU_PM notifier). free_percpu() takes pcpu_lock,
> +	 * which is a sleeping lock under PREEMPT_RT, so defer the actual
> +	 * teardown to process context.
> +	 */
> +	INIT_WORK(&csdev->free_work, coresight_device_release_work);
> +	schedule_work(&csdev->free_work);

Consider the module unload process, the async work can lead to UAF:
For example:

1. unload ETM4X module:
coresight_unregister() -> device_unregister() -> put_device() -> ref 0
-> coresight_device_release() -> schedule_work(&csdev->free_work)

At this point, the release work is only queued, while the ETM4X module 
unload path proceeds as if the release operation has finished.

2. unload the coresight module without flushing the workqueue
3. A UAF occurs when the pending work is eventually executed and 
attempts to free csdev via coresight_device_release_work(), after the 
relevant code or resources have already been unloaded.

We must drain out the workqueue in coresight_exit to close the gap.

Thanks,
Jie

>   }
>   
>   static int coresight_orphan_match(struct device *dev, void *data)
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index ddf18c970..63253f6c4 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,7 @@ struct coresight_device {
>   	struct csdev_access access;
>   	struct device dev;
>   	struct coresight_path *path;
> +	struct work_struct free_work;
>   	atomic_t mode;
>   	int refcnt;
>   	int cpu;



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
  2026-07-12 21:17 ` sashiko-bot
  2026-07-13  3:07 ` Jie Gan
@ 2026-07-13 23:00 ` Mohamed Ayman
  2026-07-13 23:17   ` sashiko-bot
  2026-07-14 10:42   ` Sebastian Andrzej Siewior
  2 siblings, 2 replies; 10+ messages in thread
From: Mohamed Ayman @ 2026-07-13 23:00 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Sebastian Andrzej Siewior, Clark Williams,
	Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT
  Cc: Mohamed Ayman, moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
@ 2026-07-13 23:17   ` sashiko-bot
  2026-07-14 10:42   ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-13 23:17 UTC (permalink / raw)
  To: Mohamed Ayman; +Cc: linux-kernel, linux-rt-devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
  2026-07-13 23:17   ` sashiko-bot
@ 2026-07-14 10:42   ` Sebastian Andrzej Siewior
  2026-07-14 19:42     ` MOHAMED AYMAN
  1 sibling, 1 reply; 10+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-07-14 10:42 UTC (permalink / raw)
  To: Mohamed Ayman
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Clark Williams, Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-14 10:42   ` Sebastian Andrzej Siewior
@ 2026-07-14 19:42     ` MOHAMED AYMAN
  2026-07-15  6:58       ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 10+ messages in thread
From: MOHAMED AYMAN @ 2026-07-14 19:42 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Clark Williams, Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-14 19:42     ` MOHAMED AYMAN
@ 2026-07-15  6:58       ` Sebastian Andrzej Siewior
  2026-07-16  3:07         ` MOHAMED AYMAN
  0 siblings, 1 reply; 10+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-07-15  6:58 UTC (permalink / raw)
  To: MOHAMED AYMAN
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Clark Williams, Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-15  6:58       ` Sebastian Andrzej Siewior
@ 2026-07-16  3:07         ` MOHAMED AYMAN
  2026-07-16 12:32           ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 10+ messages in thread
From: MOHAMED AYMAN @ 2026-07-16  3:07 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Clark Williams, Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
  2026-07-16  3:07         ` MOHAMED AYMAN
@ 2026-07-16 12:32           ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 10+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-07-16 12:32 UTC (permalink / raw)
  To: MOHAMED AYMAN
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Alexander Shishkin, Clark Williams, Steven Rostedt,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS,
	moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS, open list,
	open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-16 12:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
2026-07-12 21:17 ` sashiko-bot
2026-07-13  3:07 ` Jie Gan
2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
2026-07-13 23:17   ` sashiko-bot
2026-07-14 10:42   ` Sebastian Andrzej Siewior
2026-07-14 19:42     ` MOHAMED AYMAN
2026-07-15  6:58       ` Sebastian Andrzej Siewior
2026-07-16  3:07         ` MOHAMED AYMAN
2026-07-16 12:32           ` Sebastian Andrzej Siewior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.