On 30-06-2026 12:04, Jianping Li wrote:
> When a userspace FastRPC client is abruptly terminated, FastRPC
> cleanup paths can race with device and session teardown.
>
> This results in kernel panics in different release paths:
> - fastrpc_release() when using remote heap, originating from
> fastrpc_buf_free()
> - fastrpc_device_release() when using system heap, originating from
> fastrpc_free_map()
>
> In addition, fastrpc_map_put() may trigger refcount use-after-free
> due to concurrent cleanup without proper synchronization.
>
> The root cause is that buffer and map cleanup paths may access map
> and buf resources after the associated device or session has
> already been released.
>
> Fix this by:
> - Introducing mutex protection for map and buf lifetime
> - Serializing buffer and map cleanup against device teardown
> - Skipping buffer and map operations when the device is already gone
>
> These changes ensure cleanup paths are safe against unexpected
> process aborts and prevent use-after-free and kernel panic scenarios.
>
> Fixes: c68cfb718c8f9 ("misc: fastrpc: Add support for context Invoke method")
> Cc: [email protected]
> Signed-off-by: Jianping Li <[email protected]>
> ---
> drivers/misc/fastrpc.c | 57 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index a9b2ae44c06f..3521518f9fe5 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -255,6 +255,8 @@ struct fastrpc_session_ctx {
> int sid;
> bool used;
> bool valid;
> + bool allocated;
> + struct mutex mutex;
> };
>
> struct fastrpc_soc_data {
> @@ -335,9 +337,14 @@ static inline u64 fastrpc_sid_offset(struct
> fastrpc_channel_ctx *cctx,
> static void fastrpc_free_map(struct kref *ref)
> {
> struct fastrpc_map *map;
> + struct fastrpc_user *fl;
>
> map = container_of(ref, struct fastrpc_map, refcount);
>
> + fl = map->fl;
> + if (!fl)
> + return;
> +
> if (map->table) {
> if (map->attr & FASTRPC_ATTR_SECUREMAP) {
> struct qcom_scm_vmperm perm;
> @@ -356,10 +363,16 @@ static void fastrpc_free_map(struct kref *ref)
> return;
> }
> }
> + mutex_lock(&fl->sctx->mutex);
> + if (!fl->sctx->dev) {
> + mutex_unlock(&fl->sctx->mutex);
in this case, should you still delete map from fl->maps list?
should you still do dma_buf_put to remove dma_buf reference?
Also, there was a recent change where fl refcounting was added[1]. Can
you see if you can take use of that mechanism instead of adding checks
at all these places?
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e85eb5feca8e254905ffa6c57a3c99c89a674a0f>
+ return;
> + }
> dma_buf_unmap_attachment_unlocked(map->attach, map->table,
> DMA_BIDIRECTIONAL);
> dma_buf_detach(map->buf, map->attach);
> dma_buf_put(map->buf);
> + mutex_unlock(&fl->sctx->mutex);
> }
>
> if (map->fl) {
> @@ -422,9 +435,18 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl,
> int fd,
>
> static void fastrpc_buf_free(struct fastrpc_buf *buf)
> {
> - dma_free_coherent(buf->dev, buf->size, buf->virt,
> - fastrpc_ipa_to_dma_addr(buf->fl->cctx,
> buf->dma_addr));
> - kfree(buf);
> + struct fastrpc_user *fl = buf->fl;
> +
> + if (!fl)
> + return;
> + mutex_lock(&fl->sctx->mutex);
> + if (fl->sctx->dev) {
> + dma_free_coherent(buf->dev, buf->size, buf->virt,
> + fastrpc_ipa_to_dma_addr(buf->fl->cctx,
> + buf->dma_addr));
> + kfree(buf);
kfree should happen unconditionally here, it's a leak if you skip it.
//ekansh> + }
> + mutex_unlock(&fl->sctx->mutex);
> }
>
> static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> @@ -447,8 +469,11 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl,
> struct device *dev,
> buf->dev = dev;
> buf->raddr = 0;
>
> - buf->virt = dma_alloc_coherent(dev, buf->size, &buf->dma_addr,
> - GFP_KERNEL);
> + mutex_lock(&fl->sctx->mutex);
> + if (fl->sctx->dev)
> + buf->virt = dma_alloc_coherent(dev, buf->size, &buf->dma_addr,
> + GFP_KERNEL);
> + mutex_unlock(&fl->sctx->mutex);
> if (!buf->virt) {
> mutex_destroy(&buf->lock);
> kfree(buf);
> @@ -491,6 +516,10 @@ static void fastrpc_channel_ctx_free(struct kref *ref)
> struct fastrpc_channel_ctx *cctx;
>
> cctx = container_of(ref, struct fastrpc_channel_ctx, refcount);
> + for (int i = 0; i < FASTRPC_MAX_SESSIONS; i++) {
> + if (cctx->session[i].allocated)
> + mutex_destroy(&cctx->session[i].mutex);
> + }
>
> kfree(cctx);
> }
> @@ -855,19 +884,28 @@ static int fastrpc_map_attach(struct fastrpc_user *fl,
> int fd,
> goto get_err;
> }
>
> + mutex_lock(&fl->sctx->mutex);
> + if (!fl->sctx->dev) {
> + err = -ENODEV;
> + mutex_unlock(&fl->sctx->mutex);
> + goto attach_err;
> + }
> map->attach = dma_buf_attach(map->buf, sess->dev);
> if (IS_ERR(map->attach)) {
> dev_err(sess->dev, "Failed to attach dmabuf\n");
> err = PTR_ERR(map->attach);
> + mutex_unlock(&fl->sctx->mutex);
> goto attach_err;
> }
>
> table = dma_buf_map_attachment_unlocked(map->attach, DMA_BIDIRECTIONAL);
> if (IS_ERR(table)) {
> err = PTR_ERR(table);
> + mutex_unlock(&fl->sctx->mutex);
> goto map_err;
> }
> map->table = table;
> + mutex_unlock(&fl->sctx->mutex);
>
> if (attr & FASTRPC_ATTR_SECUREMAP)
> map->dma_addr = sg_phys(map->table->sgl);
> @@ -2246,6 +2284,8 @@ static int fastrpc_cb_probe(struct platform_device
> *pdev)
> sess->used = false;
> sess->valid = true;
> sess->dev = dev;
> + mutex_init(&sess->mutex);
> + sess->allocated = true;
> dev_set_drvdata(dev, sess);
> sess->sid = sid;
>
> @@ -2260,6 +2300,8 @@ static int fastrpc_cb_probe(struct platform_device
> *pdev)
> break;
> dup_sess = &cctx->session[cctx->sesscount++];
> memcpy(dup_sess, sess, sizeof(*dup_sess));
> + mutex_init(&dup_sess->mutex);
> + dup_sess->allocated = true;
> }
> }
> spin_unlock_irqrestore(&cctx->lock, flags);
> @@ -2282,6 +2324,11 @@ static void fastrpc_cb_remove(struct platform_device
> *pdev)
> spin_lock_irqsave(&cctx->lock, flags);
> for (i = 0; i < FASTRPC_MAX_SESSIONS; i++) {
> if (cctx->session[i].sid == sess->sid) {
> + spin_unlock_irqrestore(&cctx->lock, flags);
> + mutex_lock(&cctx->session[i].mutex);
> + cctx->session[i].dev = NULL;
> + mutex_unlock(&cctx->session[i].mutex);
> + spin_lock_irqsave(&cctx->lock, flags);
> cctx->session[i].valid = false;
> cctx->sesscount--;
> }
