Skip to content

misc: fastrpc: fix init ordering race in rpmsg probe - #1256

Open
Vinayak Katoch (quic-vkatoch) wants to merge 5 commits into
qualcomm-linux:qcom-6.18.yfrom
quic-vkatoch:fix-probe-init-race
Open

Vinayak Katoch (quic-vkatoch) wants to merge 5 commits into
qualcomm-linux:qcom-6.18.yfrom
quic-vkatoch:fix-probe-init-race

Conversation

@quic-vkatoch

Copy link
Copy Markdown

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:

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

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:

Unable to handle kernel paging request at virtual address ffffcd5841510ca0
Internal error: Oops: 0000000096000047 [#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

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

…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>
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No Change Task Found

No associated change tasks found for CR 4670685 on any of the following entities:

Entities:

  • kernel.qli.2.0

CR: 4670685

Please ensure the CR has a change task associated with at least one of the entities for this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant