Re: [PATCH 2/4] remoteproc: core: Guard against a missing stop() in rproc_start()

[Mathieu Poirier]

On Tue, Sep 29, 2026 at 03:54:51PM +0800, Yonghao Zhang wrote:
> When the subdevice registration fails after a successful ops->start(),
> rproc_start() unrolls with an unconditional ops->stop() call: an
> implementation without stop() turns that error path into a NULL
> dereference, and there is no other way to undo a start once the
> processor is up.
> 
> Nothing rules that implementation out.  rproc_validate() checks the
> callbacks against the state a processor registers in: start() for
> an offline one, attach() for a detached one, which never look at
> stop(); the only written rule, Documentation/staging/remoteproc.rst
> ("Every remoteproc implementation should at least provide the ->start
> and ->stop handlers"), is a should the core does not enforce.  The
> in-tree implementations all provide both, or neither when they only
> attach (commit 1168af40b1ad ("remoteproc: k3-r5: Add support for
> IPC-only mode for all R5Fs")), so none of them can reach the call
> today.
> 
> Skip the rollback call when there is no stop(): with nothing to roll
> the start back with, the processor stays running and the failure is
> reported by the boot attempt itself.
> 
> Fixes: 7bdc9650f036 ("remoteproc: Introduce subdevices")
> Signed-off-by: Yonghao Zhang <hyz3367@xxxxxxxxx>
> ---
>  drivers/remoteproc/remoteproc_core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 123aadb467a0..19e0ea3e7240 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1336,7 +1336,10 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	return 0;
>  
>  stop_rproc:
> -	rproc->ops->stop(rproc);
> +	if (rproc->ops->stop)
> +		rproc->ops->stop(rproc);
> +	else
> +		dev_err(dev, "can't roll %s back: no stop()\n", rproc->name);

I don't want to clutter the function's error path.  Please move that to the
error path for rproc_start_subdevices() and call the correct "goto" based on the
validity of rproc->ops->stop.

>  unprepare_subdevices:
>  	rproc_unprepare_subdevices(rproc);
>  reset_table_ptr:
> -- 
> 2.34.1
>