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

[sashiko-bot@kernel.org]

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