Repository navigation
misc: fastrpc: fix init ordering race in rpmsg probe - #1256
Vinayak Katoch (quic-vkatoch) wants to merge 5 commits into
Conversation
…form_populate of_platform_populate() only guarantees that child devices are registered, not that their probes have completed before it returns. This creates a window where fastrpc_cb_init() may not have run for all context bank nodes, leaving the channel context partially initialised. Introduce fastrpc_cb_devices_create() to iterate over child DT nodes directly and call fastrpc_cb_init() synchronously for each qcom,fastrpc-compute-cb node. This ensures all context banks are fully initialised before fastrpc_rpmsg_probe() returns. Introduce fastrpc_cb_devices_destroy() as the symmetric counterpart. Before destroying the CB platform devices, invalidate all sessions under the channel lock so that any fastrpc_user still holding a reference to the channel context cannot acquire a new session backed by a destroyed device. Since fastrpc_cb_driver is no longer needed as an independent platform driver, remove it along with its match table and remove callback. Use module_rpmsg_driver() now that only a single driver registration remains. Link: https://lore.kernel.org/all/20260923-dup-sessions-v5-1-e953133a1827@oss.qualcomm.com/ Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
…driver For ADSP, only a limited number of FastRPC context banks (CBs) are available. Each CB supports a single session, which means only a few processes can run on ADSP simultaneously. If all sessions are consumed by fastrpc daemons, no session remains available when a user application starts, causing the application to fail. To work around this, qcom,nsessions = <5> was set in DT to duplicate sessions inline during fastrpc_cb_init(). This policy does not belong in DT and should be handled at the driver level instead. Remove the qcom,nsessions DT property read and the per-CB duplication logic from fastrpc_cb_init(). After all context banks have been initialised in fastrpc_rpmsg_probe(), append FASTRPC_DUP_SESSIONS (4) copies of the last session for the ADSP domain. Link: https://lore.kernel.org/all/20260923-dup-sessions-v5-2-e953133a1827@oss.qualcomm.com/ Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
The qcom,nsessions property was used to duplicate FastRPC sessions inline during context bank initialisation. Session duplication is now handled at the driver level, making this DT property redundant. Mark it deprecated. Link: https://lore.kernel.org/all/20260923-dup-sessions-v5-3-e953133a1827@oss.qualcomm.com/ Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
…c fails When a fastrpc client process dies abnormally without closing its file descriptor, fastrpc_device_release() is never called and the compute-cb session slot is never freed. The abnormal client death on the DSP side triggers a glink channel teardown which propagates up through the rpmsg bus and calls fastrpc_rpmsg_remove(). fastrpc_rpmsg_remove() nulls cctx->rpdev before calling misc_deregister(). If a concurrent open() enters fastrpc_device_open() in this window and fastrpc_session_alloc() returns NULL (sessions exhausted by the leaked slot), the error path dereferences cctx->rpdev->dev causing a NULL pointer dereference: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 pc : fastrpc_device_open+0x1b8/0x258 [fastrpc] Call trace: fastrpc_device_open+0x1b8/0x258 [fastrpc] (P) misc_open+0xd8/0x1a0 Fix by moving misc_deregister() before rpdev = NULL. misc_open() and misc_deregister() both take misc_mtx, so misc_deregister() will either block until fastrpc_device_open() completes or prevent new opens from entering it entirely. Link: https://lore.kernel.org/all/20261001-fastrpc-probe-fixes-v1-1-1376d93b8b3e@oss.qualcomm.com/ Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
fastrpc_device_register() calls misc_register() which makes the device node visible to userspace. However, kref_init(), spin_lock_init(), INIT_LIST_HEAD(), dev_set_drvdata() and fastrpc_cb_devices_create() were all called after the switch/misc_register block, leaving a window where userspace can open the device and call fastrpc_device_open() on a partially initialised channel context. On a concurrent open(), fastrpc_channel_ctx_get() calls kref_get() on a kzalloc-zeroed, never kref_init()'d refcount of 0, triggering: refcount_t: addition on 0; use-after-free. Call trace: refcount_warn_saturate+0x120/0x148 (P) fastrpc_device_open+0x204/0x258 [fastrpc] misc_open+0xd8/0x1a0 Additionally, fastrpc_cb_devices_create() not having run means sesscount is still 0, so fastrpc_session_alloc() returns NULL and fastrpc_device_open() hits the error path which calls fastrpc_channel_ctx_put() on the already-bogus refcount. This triggers the free callback, kfree()-ing the channel context. A subsequent IRQ-path fastrpc_rpmsg_callback() then dereferences the freed spinlock: Unable to handle kernel paging request at virtual address ffffcd5841510ca0 Internal error: Oops: 0000000096000047 [qualcomm-linux#1] SMP Call trace: queued_spin_lock_slowpath+0x414/0x5e0 (P) _raw_spin_lock_irqsave+0x74/0x90 fastrpc_rpmsg_callback+0x4c/0xf0 [fastrpc] qcom_glink_native_rx+0x6c4/0xf10 qcom_glink_smem_intr+0x1c/0x38 [qcom_glink_smem] Kernel panic - not syncing: Oops: Fatal exception in interrupt Fix by moving all channel context initialisation (kref_init, spin_lock_init, INIT_LIST_HEAD, idr_init, dev_set_drvdata) and fastrpc_cb_devices_create() before fastrpc_device_register(), ensuring the struct is fully ready before the device node is visible to userspace. Add err_depopulate label in the error path since fastrpc_cb_devices_create() now runs before misc_register(); a fastrpc_device_register() failure must call fastrpc_cb_devices_destroy() to clean up the compute-cb children before freeing data. Link: https://lore.kernel.org/all/20261001-fastrpc-probe-fixes-v1-2-1376d93b8b3e@oss.qualcomm.com/ Fixes: 3abe3ab ("misc: fastrpc: add secure domain support") Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
|
Merge Check Failed: No Change Task Found No associated change tasks found for CR 4670685 on any of the following entities: Entities:
CR: 4670685 Please ensure the CR has a change task associated with at least one of the entities for this branch. |
PR #1256 — validate-patchPR: #1256
Final Summary
|
PR #1256 — checker-log-analyzerPR: #1256
Detailed report: Full report
|
2 participants
fastrpc_device_register() calls misc_register(), making the device node visible to userspace. However kref_init(), spin_lock_init(), INIT_LIST_HEAD(), dev_set_drvdata(), and fastrpc_cb_devices_create() were all called after the switch block, leaving a window where userspace can open the device on a partially initialised channel context.
A concurrent open() in this window calls kref_get() on a kzalloc-zeroed, never kref_init()'d refcount of 0:
Since fastrpc_cb_devices_create() hasn't run yet, sesscount is 0, so fastrpc_session_alloc() returns NULL and the error path calls fastrpc_channel_ctx_put() on the bogus refcount, freeing the channel context. A subsequent IRQ-path fastrpc_rpmsg_callback() then dereferences the freed spinlock:
Move all channel context initialisation before fastrpc_device_register() so the struct is fully ready before the device node is visible to userspace. Add an err_depopulate label to properly tear down CB children when fastrpc_device_register() fails.
Link: https://lore.kernel.org/all/20261001-fastrpc-probe-fixes-v1-2-1376d93b8b3e@oss.qualcomm.com/
CRs-Fixed: 4670685