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

[Bjorn Andersson <andersson@kernel.org>]

On Tue, Jul 28, 2026 at 12:09:44AM +0530, Pradnya Dahiwale wrote:
> 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.
> 

I agree with your analysis! Let's continue this journey and make sure
these gaps are addressed.

> Reviewed-by: Pradnya Dahiwale <pradnya.dahiwale@oss.qualcomm.com>

Thank you,
Bjorn