[BUG] qcom_q6v5_pas: NULL deref in recovery when using attach-only ops (qcom,broken-reset)

[BUG] qcom_q6v5_pas: NULL deref in recovery when using attach-only ops (qcom,broken-reset)

[François Roux]

Hi Stephan,

Running your remoteproc "attach" series on a Surface Pro 12in (X1P42100) at
EL2, I hit a kernel oops when the CDSP crashed on its own during a long
build. I was not trying to break anything -- this was a 28-minute kernel
compile as a stability test, and the DSP failed unprompted.

Reproducer: any remoteproc using qcom_pas_ops_no_reset (i.e. a node with
qcom,broken-reset) that crashes at runtime. No user action needed.

What happens
============

	qcom_q6v5_pas 32300000.remoteproc: fatal error received: sleep_statsi.c:537:
	remoteproc remoteproc1: crash detected in cdsp: type fatal error
	remoteproc remoteproc1: handling crash #1 in cdsp
	remoteproc remoteproc1: recovering cdsp
	remoteproc remoteproc1: stopped remote processor cdsp
	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
	Mem abort info:
	  ESR = 0x0000000086000004
	  EC = 0x21: IABT (current EL), IL = 32 bits
	  FSC = 0x04: level 0 translation fault
	Internal error: Oops: 0000000086000004 [#1]  SMP
	CPU: 3 UID: 0 PID: 82168 Comm: kworker/u34:5 Not tainted 7.1.0-next-20260626 #10
	Hardware name: Microsoft Corporation Surface Pro 12in 1st Ed with Snapdragon
	Workqueue: rproc_recovery_wq rproc_crash_handler_work
	pc : 0x0
	lr : rproc_start+0xc0/0x164
	Call trace:
	 rproc_trigger_recovery+0x148/0x164
	 rproc_crash_handler_work+0xb4/0xb8
	 process_one_work+0x15c/0x29c
	 worker_thread+0x18c/0x2e0
	 kthread+0x11c/0x13c
	 ret_from_fork+0x10/0x20

Analysis
========

The link register points at rproc_start+0xc0, and the instruction before it
is the indirect call:

	rproc_start+0xbc:  ldr x1, [x0, #16]   <- rproc->ops->start
	                   blr x1              <- x1 == NULL

which is remoteproc_core.c:1292:

	ret = rproc->ops->start(rproc);

rproc_start() calls ops->start unconditionally, and qcom_pas_ops_no_reset
does not provide one:

	static const struct rproc_ops qcom_pas_ops_no_reset = {
		.attach    = qcom_pas_attach,
		.da_to_va  = qcom_pas_da_to_va,
		.stop      = qcom_pas_stop,
		.panic     = qcom_pas_panic,
	};

The reason that path is reached at all is the branch in
rproc_trigger_recovery():

	if (rproc_has_feature(rproc, RPROC_FEAT_ATTACH_ON_RECOVERY))
		ret = rproc_attach_recovery(rproc);
	else
		ret = rproc_boot_recovery(rproc);

qcom_q6v5_pas.c never calls rproc_set_feature(..., RPROC_FEAT_ATTACH_ON_RECOVERY)
-- in this tree only imx_rproc.c does. So an attach-only remoteproc takes the
boot path on recovery, which tries to load firmware and start it, neither of
which it can do.

Suggested direction (untested)
==============================

Setting RPROC_FEAT_ATTACH_ON_RECOVERY when qcom_pas_ops_no_reset is selected
looks like the natural fix, so recovery goes through rproc_attach_recovery().
Note that path calls __rproc_detach() first, and .detach is also absent from
qcom_pas_ops_no_reset -- but __rproc_detach() does check for it and returns
an error rather than dereferencing NULL, so the failure would at least be
graceful.

I have not tried either change: I have exactly one machine, the crash is not
something I can trigger on demand, and I would rather report the trace than
guess at a patch I cannot exercise.

Practical consequence, for what it is worth: after this, the CDSP stays
offline until reboot. The ADSP was unaffected and audio kept working, and the
build completed normally -- only the recovery worker died.

Environment: linux-next next-20260626 plus your 16-patch attach series, EL2
via slbounce, Arch Linux ARM (aarch64). The DTB is derived from the vendor
one, with qcom,broken-reset on both remoteproc nodes.

Thanks,
François Roux

Re: [BUG] qcom_q6v5_pas: NULL deref in recovery when using attach-only ops (qcom,broken-reset)

[Stephan Gerhold]

On Tue, Aug 11, 2026 at 07:15:46PM +0200, François Roux wrote:
> Running your remoteproc "attach" series on a Surface Pro 12in (X1P42100) at
> EL2, I hit a kernel oops when the CDSP crashed on its own during a long
> build. I was not trying to break anything -- this was a 28-minute kernel
> compile as a stability test, and the DSP failed unprompted.
> 
> Reproducer: any remoteproc using qcom_pas_ops_no_reset (i.e. a node with
> qcom,broken-reset) that crashes at runtime. No user action needed.
> 

Thanks for the report. Please note that this is a non-upstream change,
so while I appreciate the report we should not ping the upstream
maintainers about it. They have likely never seen the change.

> What happens
> ============
> 
> 	qcom_q6v5_pas 32300000.remoteproc: fatal error received: sleep_statsi.c:537:
> 	remoteproc remoteproc1: crash detected in cdsp: type fatal error
> 	remoteproc remoteproc1: handling crash #1 in cdsp
> 	remoteproc remoteproc1: recovering cdsp
> 	remoteproc remoteproc1: stopped remote processor cdsp
> 	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> 	Mem abort info:
> 	  ESR = 0x0000000086000004
> 	  EC = 0x21: IABT (current EL), IL = 32 bits
> 	  FSC = 0x04: level 0 translation fault
> 	Internal error: Oops: 0000000086000004 [#1]  SMP
> 	CPU: 3 UID: 0 PID: 82168 Comm: kworker/u34:5 Not tainted 7.1.0-next-20260626 #10
> 	Hardware name: Microsoft Corporation Surface Pro 12in 1st Ed with Snapdragon
> 	Workqueue: rproc_recovery_wq rproc_crash_handler_work
> 	pc : 0x0
> 	lr : rproc_start+0xc0/0x164
> 	Call trace:
> 	 rproc_trigger_recovery+0x148/0x164
> 	 rproc_crash_handler_work+0xb4/0xb8
> 	 process_one_work+0x15c/0x29c
> 	 worker_thread+0x18c/0x2e0
> 	 kthread+0x11c/0x13c
> 	 ret_from_fork+0x10/0x20
> 
> Analysis
> ========
> 
> The link register points at rproc_start+0xc0, and the instruction before it
> is the indirect call:
> 
> 	rproc_start+0xbc:  ldr x1, [x0, #16]   <- rproc->ops->start
> 	                   blr x1              <- x1 == NULL
> 
> which is remoteproc_core.c:1292:
> 
> 	ret = rproc->ops->start(rproc);
> 
> rproc_start() calls ops->start unconditionally, and qcom_pas_ops_no_reset
> does not provide one:
> 
> 	static const struct rproc_ops qcom_pas_ops_no_reset = {
> 		.attach    = qcom_pas_attach,
> 		.da_to_va  = qcom_pas_da_to_va,
> 		.stop      = qcom_pas_stop,
> 		.panic     = qcom_pas_panic,
> 	};
> 
> The reason that path is reached at all is the branch in
> rproc_trigger_recovery():
> 
> 	if (rproc_has_feature(rproc, RPROC_FEAT_ATTACH_ON_RECOVERY))
> 		ret = rproc_attach_recovery(rproc);
> 	else
> 		ret = rproc_boot_recovery(rproc);
> 
> qcom_q6v5_pas.c never calls rproc_set_feature(..., RPROC_FEAT_ATTACH_ON_RECOVERY)
> -- in this tree only imx_rproc.c does. So an attach-only remoteproc takes the
> boot path on recovery, which tries to load firmware and start it, neither of
> which it can do.
> 
> Suggested direction (untested)
> ==============================
> 
> Setting RPROC_FEAT_ATTACH_ON_RECOVERY when qcom_pas_ops_no_reset is selected
> looks like the natural fix, so recovery goes through rproc_attach_recovery().
> Note that path calls __rproc_detach() first, and .detach is also absent from
> qcom_pas_ops_no_reset -- but __rproc_detach() does check for it and returns
> an error rather than dereferencing NULL, so the failure would at least be
> graceful.
> 

We can't recover the remoteproc in this case, because we don't know how
to restart it. Attaching a crashed remoteproc (that is no longer
functional) does not make sense either.

I think we should set rproc->recovery_disabled = true to fix this.
Practically speaking though, the whole qcom,broken-reset approach is
kind of controversial and I'm not sure yet if I will ever post it
upstream in the current state.

Thanks,
Stephan

Re: [BUG] qcom_q6v5_pas: NULL deref in recovery when using attach-only ops (qcom,broken-reset)

[François Roux]

Hi Stephan,

Thanks -- and you are right on both counts. Attaching a crashed remoteproc
makes no sense, so my suggested RPROC_FEAT_ATTACH_ON_RECOVERY was the wrong
answer. I also have a correction to make: I wrote that the crash could not be
triggered on demand. That was wrong, debugfs exposes exactly that, and I
should have looked before claiming otherwise.

So I tested your suggestion instead of guessing. Both runs on the same boot,
same trigger, only rproc->recovery_disabled differing.

With recovery_disabled = true
=============================

	# echo disabled > /sys/kernel/debug/remoteproc/remoteproc1/recovery
	# echo 2 > /sys/kernel/debug/remoteproc/remoteproc1/crash

	remoteproc remoteproc1: crash detected in cdsp: type fatal error
	remoteproc remoteproc1: handling crash #1 in cdsp

That is all of it. No recovery attempt, no oops, kernel untainted. The state
goes to "crashed" rather than "offline", which is also more honest: nothing
stopped the DSP, it is simply known to be dead.

Control, recovery re-enabled
============================

	# echo enabled > /sys/kernel/debug/remoteproc/remoteproc1/recovery

	remoteproc remoteproc1: recovering cdsp
	remoteproc remoteproc1: stopped remote processor cdsp
	Unable to handle kernel NULL pointer dereference at virtual address 0
	Internal error: Oops: 0000000086000004 [#1]  SMP
	pc : 0x0
	lr : rproc_start+0xc0/0x164
	Call trace:
	 rproc_trigger_recovery+0x148/0x164
	 rproc_recovery_write+0x16c/0x180

Same fault, same lr, as the spontaneous CDSP crash I reported -- only the
entry point differs, debugfs write instead of rproc_crash_handler_work. The
writing process took a SIGSEGV, since the oops happened in its write() path.

So: recovery_disabled = true fixes it, verified on hardware.

One observation, take it or leave it: rproc_start() calls ops->start without
checking it, and rproc_boot_recovery() does the same for ops->coredump. Even
with the flag set correctly by every driver, an rproc_ops missing .start
remains one debugfs write away from a NULL call. A check there would turn a
driver bug into an error return.

On qcom,broken-reset being controversial and possibly never posted upstream:
understood, and thanks for saying so plainly rather than leaving it implied.
I have written that caveat into my notes next to the workaround, so anyone
picking it up from there knows it is not headed for mainline as it stands.
The machine is a Surface Pro 12in (X1P42100) booted at EL2 via slbounce,
where the DSPs are started by the UEFI driver before ExitBootServices; your
series is what makes audio work there at all, so it has been worth having
even in this state.

Thanks again,
François Roux

--
This reply, and the report before it, were drafted with AI assistance; the
testing and the machine are mine.