Re: [PATCH v2 2/3] remoteproc: core: Check subdev start status in rproc_stop()

[Jingyi Wang]

On 5/22/2026 8:14 PM, Stephan Gerhold wrote:

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 <jingyi.w...@oss.qualcomm.com>
---
  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

Thx, I think it be better to modify the flag inside these two functions.
Will fix in next version.

Thanks,
Jingyi