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

[Bjorn Andersson <andersson@kernel.org>]

On Thu, Jul 23, 2026 at 12:09:52PM +0200, Konrad Dybcio wrote:
> On 7/23/26 5:52 AM, Bjorn Andersson wrote:
> > There's no synchronization between rproc_crash_handler_work() and
> > rproc_del(), as such it's possible for a driver to be removed while
> > crash-handler work is scheduled, or even executing - resulting in
> > use-after-free issues.
> 
> [...]
> 
> > +	spin_lock_irqsave(&rproc->crash_handler_lock, flags);
> > +	if (READ_ONCE(rproc->deleting)) {
> > +		spin_unlock_irqrestore(&rproc->crash_handler_lock, flags);
> > +		return;
> > +	}
> > +
> >  	/* Prevent suspend while the remoteproc is being recovered */
> >  	pm_stay_awake(rproc->dev.parent);
> > +	queue_work(rproc_recovery_wq, &rproc->crash_handler);
> > +	spin_unlock_irqrestore(&rproc->crash_handler_lock, flags);
> >  
> >  	dev_err(&rproc->dev, "crash detected in %s: type %s\n",
> >  		rproc->name, rproc_crash_to_string(type));
> 
> GPT reported that the rproc may be gone/deleting at the time of
> this print since it's outside the lock, so it's not a given that
> rproc->dev and rproc->name are valid (although the window is pretty
> slim..)
> 

rproc->dev will be a valid object as long as the caller is holding a
reference to the rproc device (explicitly through rproc_get_by_phandle()
or implicitly through the device hierarchy), so there's no problem
there.

rproc->name has two different life cycles:

1) rproc_alloc(name) with name living on the heap or stack, will result
in `name` being owned by the rproc->dev life cycle and hence there's no
problem - as long as adequate references are held.

2) rproc_alloc(name) with name being a constant, here the rproc context
will hold a reference to the name in constant memory, and rmmod might
in this window have unmapped that constant.

So, the case when `name` is referencing a constant is a possible
problem, but unrelated to this patch.

Regards,
Bjorn