Re: [PATCH 2/3] remoteproc: Prevent crash handling to race with rproc_del()

[Pradnya Dahiwale <pradnya.dahiwale@oss.qualcomm.com>]

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 <pradnya.dahiwale@oss.qualcomm.com>