Skip to content

misc: fastrpc: fix UAF and Oops around SSR teardown - #1212

Open
Jianping (Jianping-Li) wants to merge 4 commits into
qualcomm-linux:qcom-6.18.yfrom
Jianping-Li:SSR_fix
Open

Jianping (Jianping-Li) wants to merge 4 commits into
qualcomm-linux:qcom-6.18.yfrom
Jianping-Li:SSR_fix

Conversation

@Jianping-Li

Copy link
Copy Markdown

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

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

Copy link
Copy Markdown

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misc: fastrpc: initialise channel refcount before exposing the misc device

Please submit patch upstream and bring it as FROMLIST.

@qcomlnxci

Copy link
Copy Markdown

Test Matrix

Test Case hamoa-iot-evk-multimedia lemans-evk-multimedia monaco-evk-multimedia purwa-iot-evk-multimedia qcs615-ride-multimedia qcs6490-rb3gen2-multimedia qcs8300-ride-multimedia qcs9100-ride-r3-multimedia shikra-iqs-evk-multimedia
Audio_Card_Registration ✅ Pass ✅ Pass ✅ Pass ✅ Pass ⚠️ skip ✅ Pass ⚠️ skip ⚠️ skip ⚠️ skip
BT_FW_KMD_Service ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
BT_ON_OFF ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
BT_SCAN ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
CPUFreq_Validation ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
CPU_affinity ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
DSP_AudioPD ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ⚠️ skip
Ethernet_Basic_Validation ⚠️ skip ✅ Pass ✅ Pass ⚠️ skip ⚠️ skip ⚠️ skip ❌ Fail ⚠️ skip ⚠️ skip
Freq_Scaling ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ❌ Fail ✅ Pass ✅ Pass ✅ Pass
GIC ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ❌ Fail ✅ Pass ✅ Pass ❌ Fail
IPA ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
Interrupts ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
KVM_Driver ❌ Fail ✅ Pass ✅ Pass ❌ Fail ❌ Fail ✅ Pass ❌ Fail ❌ Fail ❌ Fail
KVM_EL2_DTB ❌ Fail ✅ Pass ✅ Pass ❌ Fail ❌ Fail ✅ Pass ❌ Fail ❌ Fail ❌ Fail
KVM_Infra ❌ Fail ✅ Pass ✅ Pass ❌ Fail ❌ Fail ✅ Pass ❌ Fail ❌ Fail ❌ Fail
OpenCV ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
PCIe ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ❌ Fail
Probe_Failure_Check ❌ Fail ❌ Fail ❌ Fail ❌ Fail ❌ Fail ❌ Fail ❌ Fail ❌ Fail ❌ Fail
RMNET ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
UFS_Validation ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ⚠️ skip
USBHost ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ⚠️ skip ❌ Fail ✅ Pass ❌ Fail
WiFi_Firmware_Driver ✅ Pass ✅ Pass ❌ Fail ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
WiFi_OnOff ✅ Pass ✅ Pass ❌ Fail ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ⚠️ skip
adsp_remoteproc ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ⚠️ skip
cdsp_remoteproc ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ❌ Fail ✅ Pass ✅ Pass ✅ Pass
gpdsp_remoteproc ⚠️ skip ✅ Pass ✅ Pass ⚠️ skip ⚠️ skip ⚠️ skip ✅ Pass ✅ Pass ⚠️ skip
hotplug ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
irq ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
kaslr ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
pinctrl ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
qcom_hwrng ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ◻️
rngtest ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
shmbridge ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
smmu ❌ Fail ❌ Fail ✅ Pass ❌ Fail ❌ Fail ✅ Pass ✅ Pass ❌ Fail ✅ Pass
watchdog ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass
wpss_remoteproc ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass ✅ Pass

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.

3 participants