[PATCH v3] misc: fastrpc: Fix double free and races in fastrpc_req_mmap() error path

[PATCH v3] misc: fastrpc: Fix double free and races in fastrpc_req_mmap() error path

[Yao Yiqi]

fastrpc_req_mmap() links the newly allocated buffer into fl->mmaps before
copying the response back to userspace. If copy_to_user() fails, the
err_assign path calls fastrpc_req_munmap_impl() to unmap the buffer from
the DSP and, on success, to free it.

Since commit 6102ceb4eab8 ("misc: fastrpc: Remove buffer from list prior
to unmap operation"), fastrpc_req_munmap_impl() no longer removes the
buffer from fl->mmaps, as that is now the responsibility of the caller.
fastrpc_req_mmap() was not updated accordingly, so the buffer is freed
while still linked in the list. The fl->mmaps cleanup in
fastrpc_user_free() then calls list_del() and fastrpc_buf_free() on the
already freed buffer, resulting in a use-after-free and a double free.

Publishing the buffer before copy_to_user() returns is also racy:
another thread of the same process can read the DSP address from the
partially written user buffer (or via userfaultfd) and issue
FASTRPC_IOCTL_MUNMAP, which unlinks and frees the buffer while
fastrpc_req_mmap() is still using it.

Fix this by adding the buffer to fl->mmaps only after copy_to_user() has
succeeded, so the buffer is not visible to a concurrent unmap while the
response is still being copied. If unmapping the buffer fails on the
error path, fastrpc_buf_free() is not called, so re-add the buffer to
fl->mmaps to keep it tracked and let fastrpc_user_free() reclaim it on
release instead of leaking the DMA allocation.

Fixes: 6102ceb4eab8 ("misc: fastrpc: Remove buffer from list prior to unmap operation")
Cc: stable@vger.kernel.org
Signed-off-by: Yao Yiqi <yaoyiqi3@huawei.com>
---
Changes in v3:
 - Re-add the buffer to fl->mmaps if unmapping it on the error path
   fails, since fastrpc_buf_free() is not called in that case and the
   DMA allocation would otherwise be leaked.

Changes in v2:
 - Link the buffer into fl->mmaps only after copy_to_user() succeeds.
   Besides fixing the double free, this also closes the concurrent
   FASTRPC_IOCTL_MUNMAP race, where a partially written user buffer let
   another thread unmap and free the buffer while fastrpc_req_mmap() was
   still using it.

 drivers/misc/fastrpc.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index d4fac2caca86..917a5dfe3774 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -2153,22 +2153,26 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 		}
 	}
 
-	spin_lock(&fl->lock);
-	list_add_tail(&buf->node, &fl->mmaps);
-	spin_unlock(&fl->lock);
-
 	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
 		err = -EFAULT;
 		goto err_assign;
 	}
 
+	spin_lock(&fl->lock);
+	list_add_tail(&buf->node, &fl->mmaps);
+	spin_unlock(&fl->lock);
+
 	dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
 		buf->raddr, buf->size);
 
 	return 0;
 
 err_assign:
-	fastrpc_req_munmap_impl(fl, buf);
+	if (fastrpc_req_munmap_impl(fl, buf)) {
+		spin_lock(&fl->lock);
+		list_add_tail(&buf->node, &fl->mmaps);
+		spin_unlock(&fl->lock);
+	}
 
 	return err;
 }
-- 
2.34.1

Re: [PATCH v3] misc: fastrpc: Fix double free and races in fastrpc_req_mmap() error path

[Ekansh Gupta]

On 30-09-2026 12:43, Yao Yiqi wrote:
> fastrpc_req_mmap() links the newly allocated buffer into fl->mmaps before
> copying the response back to userspace. If copy_to_user() fails, the
> err_assign path calls fastrpc_req_munmap_impl() to unmap the buffer from
> the DSP and, on success, to free it.
> 
> Since commit 6102ceb4eab8 ("misc: fastrpc: Remove buffer from list prior
> to unmap operation"), fastrpc_req_munmap_impl() no longer removes the
> buffer from fl->mmaps, as that is now the responsibility of the caller.
> fastrpc_req_mmap() was not updated accordingly, so the buffer is freed
> while still linked in the list. The fl->mmaps cleanup in
> fastrpc_user_free() then calls list_del() and fastrpc_buf_free() on the
> already freed buffer, resulting in a use-after-free and a double free.
> 
> Publishing the buffer before copy_to_user() returns is also racy:
> another thread of the same process can read the DSP address from the
> partially written user buffer (or via userfaultfd) and issue
> FASTRPC_IOCTL_MUNMAP, which unlinks and frees the buffer while
> fastrpc_req_mmap() is still using it.
> 
> Fix this by adding the buffer to fl->mmaps only after copy_to_user() has
> succeeded, so the buffer is not visible to a concurrent unmap while the
> response is still being copied. If unmapping the buffer fails on the
> error path, fastrpc_buf_free() is not called, so re-add the buffer to
> fl->mmaps to keep it tracked and let fastrpc_user_free() reclaim it on
> release instead of leaking the DMA allocation.
> 
> Fixes: 6102ceb4eab8 ("misc: fastrpc: Remove buffer from list prior to unmap operation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yao Yiqi <yaoyiqi3@huawei.com>

Reviewed-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>

> ---
> Changes in v3:
>  - Re-add the buffer to fl->mmaps if unmapping it on the error path
>    fails, since fastrpc_buf_free() is not called in that case and the
>    DMA allocation would otherwise be leaked.
> 
> Changes in v2:
>  - Link the buffer into fl->mmaps only after copy_to_user() succeeds.
>    Besides fixing the double free, this also closes the concurrent
>    FASTRPC_IOCTL_MUNMAP race, where a partially written user buffer let
>    another thread unmap and free the buffer while fastrpc_req_mmap() was
>    still using it.
> 
>  drivers/misc/fastrpc.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d4fac2caca86..917a5dfe3774 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2153,22 +2153,26 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>  		}
>  	}
>  
> -	spin_lock(&fl->lock);
> -	list_add_tail(&buf->node, &fl->mmaps);
> -	spin_unlock(&fl->lock);
> -
>  	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
>  		err = -EFAULT;
>  		goto err_assign;
>  	}
>  
> +	spin_lock(&fl->lock);
> +	list_add_tail(&buf->node, &fl->mmaps);
> +	spin_unlock(&fl->lock);
> +
>  	dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
>  		buf->raddr, buf->size);
>  
>  	return 0;
>  
>  err_assign:
> -	fastrpc_req_munmap_impl(fl, buf);
> +	if (fastrpc_req_munmap_impl(fl, buf)) {
> +		spin_lock(&fl->lock);
> +		list_add_tail(&buf->node, &fl->mmaps);
> +		spin_unlock(&fl->lock);
> +	}
>  
>  	return err;
>  }

Re: [PATCH v3] misc: fastrpc: Fix double free and races in fastrpc_req_mmap() error path

[Yao Yiqi]

On Sep 30th, Sashiko AI wrote:
> [Severity: Critical]
> Is there a risk of a DMA use-after-free if the unmap operation is
> interrupted by a signal?
> ...
> If fastrpc_req_munmap_impl() returns -ERESTARTSYS, this err_assign path
> then manually adds the buffer back to fl->mmaps. When the process later
> exits, fastrpc_user_free() will reclaim this buffer while the DSP may
> still be using it, which could corrupt newly allocated kernel memory.

Thanks for the review.  This path is not reachable.

fastrpc_req_munmap_impl() issues the unmap RPC with kernel = true:

	err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
				      &args[0]);

and fastrpc_internal_invoke() waits differently depending on that flag
(fastrpc_wait_for_response(), drivers/misc/fastrpc.c:1320):

	if (kernel) {
		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
			err = -ETIMEDOUT;
	} else {
		err = wait_for_completion_interruptible(&ctx->work);
	}

wait_for_completion_timeout() is uninterruptible, so the kernel invoke can
only return 0 or -ETIMEDOUT, never -ERESTARTSYS.  The interruptible wait
above is the only source of -ERESTARTSYS in this driver, and it is only
used for user invokes (FASTRPC_IOCTL_INVOKE).  Therefore the
interrupted-invoke cleanup you quoted (fastrpc.c:1418) never runs for the
unmap RPC, no matter whether the buffer is linked into fl->mmaps at that
point, and fastrpc_req_munmap_impl() cannot return -ERESTARTSYS from the
err_assign path.

For the failure that the unmap RPC can actually return (-ETIMEDOUT or a
DSP error), re-adding the buffer to fl->mmaps keeps it tracked so that
fastrpc_user_free() reclaims it on release.  That is exactly what
fastrpc_req_munmap() has done since 6102ceb4eab8 ("misc: fastrpc: Remove
buffer from list prior to unmap operation") for a failed unmap, and this
patch deliberately preserves that existing behaviour.

Best regards,
Yiqi