* Re: [PATCH v2] misc: fastrpc: Use devm_of_reserved_mem_device_init()
2026-09-02 19:40 [PATCH v2] misc: fastrpc: Use devm_of_reserved_mem_device_init() Mukesh Ojha
@ 2026-09-02 19:56 ` sashiko-bot
2026-09-03 6:33 ` Konrad Dybcio
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-02 19:56 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Critical] The patch introduces a critical lifetime mismatch by tying the reserved memory region's release to the device unbind phase via `devm_of_reserved_mem_device_init()`. The fastrpc driver allows user-specific DMA buffers (held by `miscdev` open file descriptors) to outlive the device's unbind phase. When userspace closes the file descriptor after `remove()` has completed, `fastrpc_buf_free()` calls `dma_free_coherent()` on `rdev`. Because the devres cleanup has already executed and cleared the device's coherent memory pool (`dev->dma_mem = NULL`), `dma_free_coherent()` falls back to freeing the reserved memory pages into the general kernel buddy allocator, causing severe memory corruption.
Pre-existing issues:
- [High] In `fastrpc_rpmsg_probe()`, `rdev->dma_mask` is pointed to the dynamically allocated `&data->dma_mask`. If the probe function fails later (e.g., in `of_platform_populate()`), `data` is freed via `kfree(data)`, but `rdev->dma_mask` is not cleared. This leaves a dangling pointer in the persistent `rpmsg_device` structure that can lead to a Use-After-Free if the DMA mask is later queried by the driver core or during reprobe.
- [High] For `SDSP_DOMAIN_ID`, the entire reserved memory region is assigned to the DSP via `qcom_scm_assign_mem()`. However, the driver fails to revert this assignment back to `QCOM_SCM_VMID_HLOS` on subsequent probe error paths and in `fastrpc_rpmsg_remove()`. As a result, the physical memory remains permanently owned by TrustZone, preventing driver reload and potentially causing hypervisor faults on subsequent Linux accesses.
- [High] Missing NULL check for `cctx->rpdev` in `fastrpc_remote_heap_alloc` leading to a potential crash if ioctls are called after device removal.
- [High] The fastrpc misc devices are registered and exposed to userspace before their critical context data structures (locks, IDR, refcount, rpdev) are initialized.
- [High] Use-after-free of child platform_device pointer during buffer freeing.
--
commit 690dc3f2f1a71f7a802b45a29611ac1e33c88795
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
misc: fastrpc: Use devm_of_reserved_mem_device_init()
The commit updates the fastrpc driver to use the devres-managed
devm_of_reserved_mem_device_init() during probe. This ensures the reserved
memory region is automatically released on device removal.
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 90fd669636ec1..21f084c76f81d 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2562,7 +2562,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> return -EINVAL;
> }
>
> - if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0))
> + if (devm_of_reserved_mem_device_init(rdev))
[Severity: Critical]
Does this introduce a lifetime mismatch for user-specific DMA buffers?
By tying the reserved memory region's release to the device unbind phase via
devres, the region is cleared before all userspace file descriptors might be
closed. The fastrpc driver allows user-specific DMA buffers (held by miscdev
open file descriptors) to outlive the device's unbind phase.
It allocates user-managed memory from the rdev reserved memory pool, which
outlives device unbind:
drivers/misc/fastrpc.c:fastrpc_remote_heap_alloc() {
...
struct device *rdev = &fl->cctx->rpdev->dev;
return __fastrpc_buf_alloc(fl, rdev, size, obuf);
...
}
When userspace closes the file descriptor after remove() has completed,
fastrpc_buf_free() calls dma_free_coherent() on rdev:
drivers/misc/fastrpc.c:fastrpc_buf_free() {
...
dma_free_coherent(buf->dev, buf->size, buf->virt,
fastrpc_ipa_to_dma_addr(buf->fl->cctx, buf->dma_addr));
...
}
Because the devres cleanup has already executed and cleared the device's
coherent memory pool, will dma_free_coherent() fall back to freeing the
reserved memory pages into the general kernel buddy allocator, causing severe
memory corruption?
> dev_info(rdev, "no reserved DMA memory for FASTRPC\n");
>
> vmcount = of_property_read_variable_u32_array(rdev->of_node,
[Severity: High]
This is a pre-existing issue, but does fastrpc_rpmsg_probe() leave a dangling
pointer if probe fails?
During probe, rdev->dma_mask is pointed to dynamically allocated memory:
drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
...
rdev->dma_mask = &data->dma_mask;
...
}
If a subsequent step like of_platform_populate() fails, the error path
frees data:
drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
...
err_free_data:
kfree(data);
return err;
}
Since rdev->dma_mask is not cleared, could this result in a Use-After-Free
if the rpmsg bus or driver core later queries the dangling DMA mask pointer?
[Severity: High]
This is a pre-existing issue, but does the driver permanently lose physical
memory to TrustZone on probe error or device unbind?
For SDSP_DOMAIN_ID, the entire reserved memory region is assigned to the DSP:
drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
...
err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
data->vmperms, data->vmcount);
...
}
However, neither the probe error paths:
drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
...
err_deregister_fdev:
if (data->fdevice)
...
}
nor fastrpc_rpmsg_remove() revert this assignment back to QCOM_SCM_VMID_HLOS.
Can this cause hypervisor faults (XPU/SMMU violation) and kernel panics if
the kernel attempts to reuse the physical memory after device unbind or
driver reload?
[Severity: High]
This is a pre-existing issue, but is there a missing NULL check for
cctx->rpdev in fastrpc_remote_heap_alloc()?
During device unbind, cctx->rpdev is cleared:
drivers/misc/fastrpc.c:fastrpc_rpmsg_remove() {
...
cctx->rpdev = NULL;
...
}
If ioctls like FASTRPC_IOCTL_MMAP are called after device removal,
fastrpc_remote_heap_alloc() dereferences the pointer without checking:
drivers/misc/fastrpc.c:fastrpc_remote_heap_alloc() {
...
struct device *rdev = &fl->cctx->rpdev->dev;
...
}
Could this result in a NULL pointer dereference and a kernel crash?
[Severity: High]
This is a pre-existing issue, but are the fastrpc misc devices exposed to
userspace too early during probe?
The misc device is registered before its downstream context fields are
initialized:
drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
...
err = fastrpc_device_register(rdev, data, secure_dsp, domain);
...
}
If userspace calls open() on the device node exactly between misc_register()
and the initialization of context structures (like locks, IDR, refcount),
could functions like fastrpc_session_alloc() acquire an uninitialized lock:
drivers/misc/fastrpc.c:fastrpc_session_alloc() {
...
spin_lock_irqsave(&cctx->lock, flags);
...
}
Would this lead to a kernel panic under lock debugging?
[Severity: High]
This is a pre-existing issue, but is there a use-after-free of the child
platform_device pointer during buffer freeing?
When a buffer is allocated, an un-referenced pointer to a child
platform_device is stored:
drivers/misc/fastrpc.c:__fastrpc_buf_alloc() {
...
buf->dev = dev;
...
}
If the parent rpmsg device is unbound, of_platform_depopulate() destroys
the child platform devices. If a user subsequently closes their file
descriptor, fastrpc_buf_free() dereferences the freed pointer:
drivers/misc/fastrpc.c:fastrpc_buf_free() {
...
dma_free_coherent(buf->dev, buf->size, buf->virt,
fastrpc_ipa_to_dma_addr(buf->fl->cctx, buf->dma_addr));
...
}
Could this result in memory corruption or a kernel crash when the dangling
pointer is passed to dma_free_coherent()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902194041.1999834-1-mukesh.ojha@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread