[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