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 <[email protected]>
Thank you,
Bjorn
