misc: fastrpc: fix UAF and Oops around SSR teardown - #1212
Open
Jianping (Jianping-Li) wants to merge 4 commits into
Open
Jianping (Jianping-Li) wants to merge 4 commits into
Jianping (Jianping-Li) wants to merge 4 commits into
Conversation
…evice
fastrpc_device_register() calls misc_register(), which immediately makes
/dev/fastrpc-<domain> visible to userspace. However, kref_init() on the
channel context refcount only runs after the whole domain switch()
completes, several statements later.
Any process that opens the device in that window reaches
fastrpc_device_open() -> fastrpc_channel_ctx_get() -> kref_get() on a
refcount that has never been initialised and is still zero, which
refcount_t correctly reports as a use-after-free:
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 0 PID: 760 at lib/refcount.c:25 refcount_warn_saturate+0x120/0x144
CPU: 0 UID: 0 PID: 760 Comm: adsprpcd
Call trace:
refcount_warn_saturate
fastrpc_device_open [fastrpc]
misc_open
chrdev_open
do_dentry_open
vfs_open
path_openat
do_filp_open
do_sys_openat2
__arm64_sys_openat
This is easy to hit after a subsystem restart, when the DSP daemon
reopens the device as soon as it observes the PD coming back up, racing
with fastrpc_rpmsg_probe() on the rebind path.
Move kref_init() ahead of the device registration so the refcount is
always valid by the time the node is reachable from userspace.
Fixes: f6f9279 ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
fastrpc_notify_users() sets ctx->retval to -EPIPE and completes ctx->work for every pending and interrupted context, which is enough to release a thread blocked in fastrpc_wait_for_response(). A thread using polling mode does not wait on that completion. It spins in poll_for_remote_response(), whose exit condition is: (val == FASTRPC_POLL_RESPONSE) || ctx->is_work_done Since the DSP is already down, it will never write FASTRPC_POLL_RESPONSE into the poll address, and is_work_done is left untouched by fastrpc_notify_users(). The thread therefore keeps spinning until FASTRPC_POLL_MAX_TIMEOUT_US expires instead of bailing out immediately. Worse, the poll address lives inside ctx->buf, so a polling thread that has not been told to stop is still reading a buffer that the SSR teardown path is about to reclaim. Set is_work_done along with retval so polling waiters observe the termination on their next iteration, exactly like completion waiters do. Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
fastrpc_rpmsg_remove() clears cctx->rpdev and notifies all users, then proceeds to tear the channel down. The rpdev check at the top of fastrpc_internal_invoke() is the only thing stopping new work from entering afterwards, and it is not sufficient: it is read without any lock, so a thread can observe a still-valid rpdev, get preempted, and resume in the middle of the teardown sequence. Add an explicit cctx->teardown flag, set under cctx->lock at the start of fastrpc_rpmsg_remove(), and check it from fastrpc_internal_invoke() under the same lock. Placing the gate inside fastrpc_internal_invoke() rather than in the ioctl dispatcher also covers fastrpc_device_release() -> fastrpc_release_current_dsp_process(), which issues an INIT_RELEASE message directly, outside of any ioctl. The flag is deliberately separate from cctx->rpdev rather than reusing it. They have different lifetimes: the gate must close immediately, but rpdev has to stay valid until the invokes that were already admitted have finished their send. The next patch makes that distinction load-bearing by deferring the rpdev clear until after the drain. No functional change for the non-SSR path: the flag is only ever set from fastrpc_rpmsg_remove(). Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
Rejecting new invokes is not enough: fastrpc_rpmsg_remove() still runs
concurrently with every invoke that was already past the gate. Those
threads have just been woken with -EPIPE and are on their way to the
bail: label, where fastrpc_context_put() releases ctx->buf through
dma_free_coherent(). Nothing stops the teardown from reaching
of_platform_depopulate() first and unbinding the context bank devices
out from under them.
Account for in-flight invokes with cctx->invoke_cnt, incremented in the
same critical section that performs the teardown check so the two cannot
be observed out of order, and decremented on every return path. Then
wait for the count to reach zero in fastrpc_rpmsg_remove() before
touching any channel resource.
The wait cannot hang: the gate added in the previous patch prevents new
invokes from being counted, and fastrpc_notify_users() has already woken
every context that is counted, including poll-mode waiters.
Without this, fastrpc_device_release() ->
fastrpc_release_current_dsp_process() -> fastrpc_internal_invoke() can
dereference a NULL rpdev after the teardown cleared it while the invoke
was sleeping in an allocation:
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000358
CPU: 2 UID: 0 PID: 2003 Comm: adsprpcd
pc : fastrpc_internal_invoke+0xb40/0x1398 [fastrpc]
Call trace:
fastrpc_internal_invoke [fastrpc]
fastrpc_device_release [fastrpc]
__fput
__arm64_sys_close
which resolves to
the cctx->rpdev->ept dereference in rpmsg_send(); the faulting
address is the offset of ept within struct rpmsg_device.
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
Jianping (Jianping-Li)
requested review from
a team,
Nicolas Dechesne (ndechesne),
Kaushal Kumar (quic-kaushalk) and
Salendarsingh Gaud (sgaud-quic)
September 29, 2026 08:52
|
Merge Check Failed: No CR Numbers Found Error: No Change Request numbers were found. Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests. |
Shiraz Hashim (shashim-quic)
requested changes
Sep 29, 2026
Shiraz Hashim (shashim-quic)
left a comment
There was a problem hiding this comment.
misc: fastrpc: initialise channel refcount before exposing the misc device
Please submit patch upstream and bring it as FROMLIST.
Test Matrix
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Hamoa (X1E80100) IoT EVK the ADSP can restart repeatedly, and every
restart has a chance of taking the kernel down with it:
Unable to handle kernel paging request at virtual address fffffdffc1ffffc0
Internal error: Oops: 0000000096000006
CPU: 5 UID: 0 PID: 2766 Comm: adsprpcd
pc : ___free_pages+0x24/0xe0
Call trace:
___free_pages
__free_pages
__dma_direct_free_pages
dma_direct_free
dma_free_attrs
fastrpc_context_free [fastrpc]
fastrpc_internal_invoke [fastrpc]
fastrpc_device_ioctl [fastrpc]
The faulting address decodes to a struct page for a PFN that has no
vmemmap backing. It comes from dma_free_coherent() being called against
a qcom,fastrpc-compute-cb device that of_platform_depopulate() has
already unbound: with the IOMMU torn down, the call falls through to
dma_direct_free(), which takes the buffer's IOVA for a physical address
and hands the resulting page to the page allocator.
fastrpc_rpmsg_remove() wakes every pending invoke with -EPIPE and then
immediately depopulates the context banks, with no synchronisation in
between, so woken threads race the teardown on their way to
fastrpc_context_free().
The series is ordered so each patch stands on its own:
1/4 is an independent probe-time bug found while debugging this: the
misc device is exposed before the channel refcount is initialised,
so an open() racing probe hits "refcount_t: addition on 0".
2/4 makes fastrpc_notify_users() wake poll-mode waiters, which today
keep spinning on a buffer the teardown is about to reclaim.
3/4 adds the teardown flag and rejects new invokes under cctx->lock.
4/4 counts in-flight invokes and drains them before touching any
channel resource.
Fix: #1107