On Tue, May 19, 2026 at 12:20:03AM -0700, Jingyi Wang wrote: > For rproc that doing attach, rproc_start_subdevices() is called only when > attach successfully. If rproc_report_crash() is called in the attach > function, rproc_boot_recovery()->rproc_stop()->rproc_stop_subdevices()-> > glink_subdev_stop() could be called and cause NULL pointer dereference: > > Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000300 > Mem abort info: > ... > pc : qcom_glink_smem_unregister+0x14/0x48 [qcom_glink_smem] > lr : glink_subdev_stop+0x1c/0x30 [qcom_common] > ... > Call trace: > qcom_glink_smem_unregister+0x14/0x48 [qcom_glink_smem] (P) > glink_subdev_stop+0x1c/0x30 [qcom_common] > rproc_stop+0x58/0x17c > rproc_trigger_recovery+0xb0/0x150 > rproc_crash_handler_work+0xa4/0xc4 > process_scheduled_works+0x18c/0x2d8 > worker_thread+0x144/0x280 > kthread+0x124/0x138 > ret_from_fork+0x10/0x20 > Code: a9be7bfd 910003fd a90153f3 aa0003f3 (b9430000) > ---[ end trace 0000000000000000 ]--- > > Introduce "subdevs_started" flag to indicate rproc_start_subdevices() has > been called successfully. Ensure subdevices are only stopped if they have > been started. > > Signed-off-by: Jingyi Wang <[email protected]> > --- > drivers/remoteproc/remoteproc_core.c | 4 +++- > include/linux/remoteproc.h | 2 ++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c > b/drivers/remoteproc/remoteproc_core.c > index f02db1113fae..6e23cb11e515 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -1308,6 +1308,7 @@ static int rproc_start(struct rproc *rproc, const > struct firmware *fw) > goto stop_rproc; > } > > + rproc->subdevs_started = true; > rproc->state = RPROC_RUNNING; > > dev_info(dev, "remote processor %s is now up\n", rproc->name); > @@ -1712,7 +1713,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed) > return -EINVAL; > > /* Stop any subdevices for the remote processor */ > - rproc_stop_subdevices(rproc, crashed); > + if (rproc->subdevs_started) > + rproc_stop_subdevices(rproc, crashed);
Thanks, this approach looks better. But where do we clear this flag? Shouldn't we do that here? In addition, I think we also need to set subdevs_started = true if attach succeeds. And clear it when detaching a remoteproc. I think you just need to update all the callers of rproc_stop_subdevices() and rproc_start_subdevices(). Or, which is probably simpler, just make the check directly inside these two functions to cover all users. Thanks, Stephan
