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
