On Thu, Jul 23, 2026 at 03:52:28AM +0000, Bjorn Andersson wrote:
> @@ -2598,16 +2609,21 @@ EXPORT_SYMBOL(rproc_put);
> */
> int rproc_del(struct rproc *rproc)
> {
> + spin_lock_irqsave(&rproc->crash_handler_lock, flags);
> + WRITE_ONCE(rproc->deleting, true);
> + spin_unlock_irqrestore(&rproc->crash_handler_lock, flags);
> +
> + if (cancel_work_sync(&rproc->crash_handler))
> + pm_relax(rproc->dev.parent);
> +
> /* TODO: make sure this works with rproc->power > 1 */
> rproc_shutdown(rproc);
rproc_del() cancels the crash-handler work here - should there also be a
matching cancel_work_sync(&rproc->attach_work)? If attach_work were still
pending when rproc_del() finishes and rproc_free() drops the last reference,
could it later run rproc_boot() on a freed rproc?
rproc_del() attach_work (still queued)
|
WRITE_ONCE(deleting, true)
|
cancel_work_sync(crash_handler)
|
rproc_shutdown()
|
device_del()
|
rproc_del() returns
|
(driver's remove path then calls)
|
rproc_free()
attach_work runs
container_of(work, rproc, attach_work)
rproc_boot(rproc) <- freed rproc?
The deleting check inside rproc_boot() dereferences rproc itself, so it can't
guard a freed one. The request_firmware_nowait() / rproc_auto_boot_callback()
path looks similar.
Reviewed-by: Pradnya Dahiwale <[email protected]>
