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

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

[Jianping Li]

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);
+			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);
+	}
+	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--;
 		}
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [Critical] `fastrpc_cb_remove` locks uninitialized mutexes when matching unallocated sessions.
- [High] The patch intentionally leaks DMA allocations, dmabufs, and tracking structures to avoid a use-after-free on device unbind.
- [High] The error path in `fastrpc_map_attach` calls `dma_buf_detach` without holding `fl->sctx->mutex`, circumventing the synchronization intended to prevent use-after-free.

Pre-existing issues:
- [High] The error handling path in `fastrpc_map_attach` triggers double-detach and double-put of dmabufs.
- [High] Preexisting memory leak in `fastrpc_free_map` during secure memory assignment failure.
- [High] Array overwrite and race condition vulnerability due to blind decrement of `sesscount` in `fastrpc_cb_remove` without array compaction.
- [High] Probe failure leaves a dangling device pointer in the active session array.
--

commit 70c262510b69042fa36e0aa92b5ee69b7cc10c62
Author: Jianping Li <jianping.li@oss.qualcomm.com>

misc: fastrpc: fix UAF and kernel panic during cleanup on process abort

This commit introduces mutex protection for map and buffer lifetimes to fix
use-after-free issues when a FastRPC client is aborted. It serializes cleanup
against device teardown and skips operations when the device is gone.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -335,9 +337,14 @@ static void fastrpc_free_map(struct kref *ref)

[ ... ]

>  		if (map->attr & FASTRPC_ATTR_SECUREMAP) {
>  			struct qcom_scm_vmperm perm;

[ ... ]

>  			err = qcom_scm_assign_mem(map->dma_addr, map->len,
>  				&src_perms, &perm, 1);
>  			if (err) {
>  				dev_err(map->fl->sctx->dev,
>  					"Failed to assign memory dma_addr %pad size 0x%llx err %d\n",
>  					&map->dma_addr, map->len, err);
>  				return;

[Severity: High]
This is a pre-existing issue, but does this early return completely abandon
the release sequence?

If qcom_scm_assign_mem() fails, it looks like this returns immediately,
bypassing all subsequent teardown logic and permanently leaking the map on the
linked list along with the dmabuf references.

>  			}
>  		}
> +		mutex_lock(&fl->sctx->mutex);
> +		if (!fl->sctx->dev) {
> +			mutex_unlock(&fl->sctx->mutex);
> +			return;

[Severity: High]
Can this early return permanently leak the map and dmabuf?

If a process terminates after a device unbinds, fastrpc_cb_remove() sets dev
to NULL. When fastrpc_free_map() runs, it hits this early return and
completely bypasses the mandatory resource deallocation such as kfree(),
list_del(), and dma_buf_put().

> +		}
>  		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);
>  	}

[ ... ]

> @@ -422,9 +435,18 @@ static void fastrpc_buf_free(struct fastrpc_buf *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);
> +	}
> +	mutex_unlock(&fl->sctx->mutex);

[Severity: High]
Could the absence of an else-branch permanently leak the buffer memory?

Similar to fastrpc_free_map(), if the device is gone, skipping the
dma_free_coherent() and kfree() leaves the memory permanently leaked
instead of safely handling the cleanup.

>  }

[ ... ]

> @@ -855,19 +884,28 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,

[ ... ]

>  	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;

[Severity: High]
This is a pre-existing issue, but does jumping to map_err cause a double
detach and put of the dmabuf?

If we take this error path after map->table has been assigned, we execute:

fastrpc_map_attach()
  goto map_err;
  map_err:
    dma_buf_detach(map->buf, map->attach);
    ...
    fastrpc_map_put(map);

Because map->table is non-NULL, the kref release function fastrpc_free_map()
will re-execute dma_buf_detach() and dma_buf_put() on the identical
structures.

[Severity: High]
Would unlocking the mutex here expose the error path to a use-after-free?

By explicitly unlocking sctx->mutex before jumping to map_err, dma_buf_detach()
is called completely unprotected. A concurrent fastrpc_cb_remove() can run
between the unlock and detach, clearing the device and leading to a
use-after-free in dma_buf_detach() when it accesses attach->dev.

>  	}
>  	map->table = table;
> +	mutex_unlock(&fl->sctx->mutex);

[ ... ]

> @@ -2246,6 +2284,8 @@ static int fastrpc_cb_probe(struct platform_device *pdev)

[ ... ]

>  	sess = &cctx->session[cctx->sesscount++];
>  	sess->used = false;
>  	sess->valid = true;
>  	sess->dev = dev;
> +	mutex_init(&sess->mutex);
> +	sess->allocated = true;

[Severity: High]
This is a pre-existing issue, but does appending to sesscount here
blindly overwrite a trailing active session?

When fastrpc_cb_remove() executes, it decrements sesscount blindly without
shifting downstream array elements to close the gap. A subsequent probe
uses the decremented sesscount as an append index, corrupting the active
session.

With this patch, the newly added mutex_init() will now reinitialize an
in-use lock on that overwritten session.

>  	dev_set_drvdata(dev, sess);
>  	sess->sid = sid;

[ ... ]

> @@ -2260,6 +2300,8 @@ static int fastrpc_cb_probe(struct platform_device *pdev)

[ ... ]

>  			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);

[Severity: High]
This isn't a bug introduced by this patch, but does a probe failure
leave a dangling device pointer in the active session array?

Just below this hunk, dma_set_mask() is called. If that call fails, it
returns an error and aborts the probe. The driver core will destroy the
device, but FastRPC has already permanently registered the dangling dev
pointer into its session array.

> @@ -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);

[Severity: Critical]
Will this loop lock uninitialized mutexes when matching unallocated sessions?

The elements beyond sesscount in cctx->session are zero-initialized.
If sess->sid is 0 (the default if a DT lacks a 'reg' property), the loop
condition cctx->session[i].sid == sess->sid will match all unused slots.

This attempts to acquire an uninitialized, all-zero mutex, which might
lead to a panic or lockdep crash. Should this loop check
cctx->session[i].allocated?

> +			cctx->session[i].dev = NULL;
> +			mutex_unlock(&cctx->session[i].mutex);
> +			spin_lock_irqsave(&cctx->lock, flags);
>  			cctx->session[i].valid = false;
>  			cctx->sesscount--;

[Severity: High]
This is a pre-existing issue, but does decrementing sesscount without
moving elements create a discrepancy between the index and active entries?

As mentioned earlier, this blind decrement leaves a gap in the array that
leads to state corruption during the next probe.

>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630063409.475-1-jianping.li@oss.qualcomm.com?part=1