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

François Roux <[email protected]>
Newsgroups org.kernel.vger.linux-remoteproc
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.