Re: [PATCH v2] misc: fastrpc: fix UAF and kernel panic during cleanup on process abort

[Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>]

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: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>  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--;
>  		}