[PATCH v2] misc: fastrpc: avoid duplicate DMA mappings in fastrpc_create_maps()

[PATCH v2] misc: fastrpc: avoid duplicate DMA mappings in fastrpc_create_maps()

[Jianping Li]

DMA handles passed as invoke arguments (scalars beyond nbufs) may refer
to the same dma_buf fd as an input/output buffer argument. Taking an
extra reference for such DMA handle maps leads to duplicate mappings and
an unbalanced reference count, since DMA handle maps are released
separately when the DSP returns the fd through the fdlist.

Fix this by not taking an extra reference for DMA handle arguments
(take_ref = false) and tagging them with FASTRPC_MAP_DMA_HANDLE. As
these maps are borrowed references, fastrpc_get_args() re-validates the
map via fastrpc_map_lookup() before dereferencing it, so it is not used
after being freed. fastrpc_put_args() only releases maps flagged as
FASTRPC_MAP_DMA_HANDLE and clears the flag to guarantee the map is freed
exactly once.

Also reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since such
handles are already mapped implicitly during the remote invoke call and
must not be mapped again through the explicit MEM_MAP path.

Fixes: 10df039834f84 ("misc: fastrpc: Skip reference for DMA handles")
Cc: stable@kernel.org
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
Patch [v1]: https://lore.kernel.org/all/20260625080832.17477-1-jianping.li@oss.qualcomm.com/

Changes in v2:
- Rework the commit message to describe the DMA handle reference and
  lifetime problem more precisely.
- Introduce a new FASTRPC_MAP_DMA_HANDLE uapi flag and a 'flags' field
  in struct fastrpc_map to explicitly tag DMA handle maps, instead of
  relying only on the nbufs boundary / take_ref.
- Plumb an mflags argument through fastrpc_map_create() and
  fastrpc_map_attach() so DMA handle maps are tagged at creation time.
- Re-validate the borrowed map in fastrpc_get_args() via
  fastrpc_map_lookup() before dereferencing it, to avoid a
  use-after-free when the map was created with take_ref = false.
- In fastrpc_put_args(), only release maps tagged FASTRPC_MAP_DMA_HANDLE
  and clear the flag afterwards, so such maps are freed exactly once.
- Reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since these
  handles are already mapped implicitly during the remote invoke and
  must not be mapped again through the explicit MEM_MAP path.
---
 drivers/misc/fastrpc.c      | 56 ++++++++++++++++++++++++++-----------
 include/uapi/misc/fastrpc.h |  2 ++
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index d86e79134c68..fa8c58a97e35 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -223,6 +223,7 @@ struct fastrpc_map {
 	u64 len;
 	u64 raddr;
 	u32 attr;
+	u32 flags;
 	struct kref refcount;
 };
 
@@ -833,7 +834,7 @@ static dma_addr_t fastrpc_compute_dma_addr(struct fastrpc_user *fl, dma_addr_t s
 }
 
 static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
-			      u64 len, u32 attr, struct fastrpc_map **ppmap)
+			      u64 len, u32 attr, struct fastrpc_map **ppmap, int mflags)
 {
 	struct fastrpc_session_ctx *sess = fl->sctx;
 	struct fastrpc_map *map = NULL;
@@ -850,6 +851,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
 
 	map->fl = fl;
 	map->fd = fd;
+	map->flags = mflags;
 	map->buf = dma_buf_get(fd);
 	if (IS_ERR(map->buf)) {
 		err = PTR_ERR(map->buf);
@@ -924,13 +926,13 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
 	return err;
 }
 
-static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
-			      u64 len, u32 attr, struct fastrpc_map **ppmap)
+static int fastrpc_map_create(struct fastrpc_user *fl, int fd, u64 len, u32 attr,
+			      struct fastrpc_map **ppmap, bool take_ref, int mflags)
 {
-	if (!fastrpc_map_lookup(fl, fd, ppmap, true))
+	if (!fastrpc_map_lookup(fl, fd, ppmap, take_ref))
 		return 0;
 
-	return fastrpc_map_attach(fl, fd, len, attr, ppmap);
+	return fastrpc_map_attach(fl, fd, len, attr, ppmap, mflags);
 }
 
 /*
@@ -1000,23 +1002,25 @@ static int fastrpc_create_maps(struct fastrpc_invoke_ctx *ctx)
 	int i, err;
 
 	for (i = 0; i < ctx->nscalars; ++i) {
+		bool take_ref = i < ctx->nbufs;
+		int mflags = 0;
 
 		if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1 ||
 		    ctx->args[i].length == 0)
 			continue;
 
-		if (i < ctx->nbufs)
-			err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
-				 ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
-		else
-			err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd,
-				 ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
+		/* Set the DMA handle mapping flag for DMA handles */
+		if (i >= ctx->nbufs)
+			mflags = FASTRPC_MAP_DMA_HANDLE;
+
+		err = fastrpc_map_create(ctx->fl, ctx->args[i].fd, ctx->args[i].length,
+					 ctx->args[i].attr, &ctx->maps[i], take_ref, mflags);
 		if (err) {
 			dev_err(dev, "Error Creating map %d\n", err);
 			return -EINVAL;
 		}
-
 	}
+
 	return 0;
 }
 
@@ -1144,6 +1148,16 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
 		list[i].num = ctx->args[i].length ? 1 : 0;
 		list[i].pgidx = i;
 		if (ctx->maps[i]) {
+			/* It is possible that map is created with
+			 * mflags FASTRPC_MAP_DMA_HANDLE and take_ref
+			 * is false. Check if map still exists or is
+			 * being freed as take_ref is false
+			 */
+			if (fastrpc_map_lookup(ctx->fl, ctx->args[i].fd,
+					       &ctx->maps[i], false)) {
+				ctx->maps[i] = NULL;
+				return -EINVAL;
+			}
 			pages[i].addr = ctx->maps[i]->dma_addr;
 			pages[i].size = ctx->maps[i]->size;
 		}
@@ -1200,8 +1214,13 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
 	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
 		if (!fdlist[i])
 			break;
-		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false))
+		/* Validate the map flags for DMA handles and skip freeing map if invalid */
+		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false) &&
+		    mmap->flags == FASTRPC_MAP_DMA_HANDLE) {
+			/* Allow DMA handle maps to free only once */
+			mmap->flags = 0;
 			fastrpc_map_put(mmap);
+		}
 	}
 
 	return ret;
@@ -1511,7 +1530,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
 	fl->pd = USER_PD;
 
 	if (init.filelen && init.filefd) {
-		err = fastrpc_map_create(fl, init.filefd, init.filelen, 0, &map);
+		err = fastrpc_map_create(fl, init.filefd, init.filelen, 0, &map, true, 0);
 		if (err)
 			goto err;
 	}
@@ -2107,9 +2126,14 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
 
 	if (copy_from_user(&req, argp, sizeof(req)))
 		return -EFAULT;
-
+	/*
+	 * Prevent mapping backward compatible DMA handles here, as they are
+	 * already mapped in the remote call.
+	 */
+	if (req.flags == FASTRPC_MAP_DMA_HANDLE)
+		return -EINVAL;
 	/* create SMMU mapping */
-	err = fastrpc_map_create(fl, req.fd, req.length, 0, &map);
+	err = fastrpc_map_create(fl, req.fd, req.length, 0, &map, true, 0);
 	if (err) {
 		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
 		return err;
diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
index c6e2925f47e6..142ddaeed85f 100644
--- a/include/uapi/misc/fastrpc.h
+++ b/include/uapi/misc/fastrpc.h
@@ -44,6 +44,8 @@ enum fastrpc_map_flags {
 	FASTRPC_MAP_FD = 2,
 	FASTRPC_MAP_FD_DELAYED,
 	FASTRPC_MAP_FD_NOMAP = 16,
+	/* Map the DMA handle in the invoke call for backward compatibility */
+	FASTRPC_MAP_DMA_HANDLE = 0x20000,
 	FASTRPC_MAP_MAX,
 };
 
-- 
2.43.0

Re: [PATCH v2] misc: fastrpc: avoid duplicate DMA mappings in fastrpc_create_maps()

[Ekansh Gupta]

On 16-07-2026 17:02, Jianping Li wrote:
> DMA handles passed as invoke arguments (scalars beyond nbufs) may refer
> to the same dma_buf fd as an input/output buffer argument. Taking an
> extra reference for such DMA handle maps leads to duplicate mappings and
> an unbalanced reference count, since DMA handle maps are released
> separately when the DSP returns the fd through the fdlist.
> 
> Fix this by not taking an extra reference for DMA handle arguments
> (take_ref = false) and tagging them with FASTRPC_MAP_DMA_HANDLE. As
> these maps are borrowed references, fastrpc_get_args() re-validates the
> map via fastrpc_map_lookup() before dereferencing it, so it is not used
> after being freed. fastrpc_put_args() only releases maps flagged as
> FASTRPC_MAP_DMA_HANDLE and clears the flag to guarantee the map is freed
> exactly once.
> 
> Also reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since such
> handles are already mapped implicitly during the remote invoke call and
> must not be mapped again through the explicit MEM_MAP path.
> 
> Fixes: 10df039834f84 ("misc: fastrpc: Skip reference for DMA handles")
> Cc: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
> Patch [v1]: https://lore.kernel.org/all/20260625080832.17477-1-jianping.li@oss.qualcomm.com/
> 
> Changes in v2:
> - Rework the commit message to describe the DMA handle reference and
>   lifetime problem more precisely.
> - Introduce a new FASTRPC_MAP_DMA_HANDLE uapi flag and a 'flags' field
>   in struct fastrpc_map to explicitly tag DMA handle maps, instead of
>   relying only on the nbufs boundary / take_ref.
> - Plumb an mflags argument through fastrpc_map_create() and
>   fastrpc_map_attach() so DMA handle maps are tagged at creation time.
> - Re-validate the borrowed map in fastrpc_get_args() via
>   fastrpc_map_lookup() before dereferencing it, to avoid a
>   use-after-free when the map was created with take_ref = false.
> - In fastrpc_put_args(), only release maps tagged FASTRPC_MAP_DMA_HANDLE
>   and clear the flag afterwards, so such maps are freed exactly once.
> - Reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since these
>   handles are already mapped implicitly during the remote invoke and
>   must not be mapped again through the explicit MEM_MAP path.
> ---
>  drivers/misc/fastrpc.c      | 56 ++++++++++++++++++++++++++-----------
>  include/uapi/misc/fastrpc.h |  2 ++
>  2 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d86e79134c68..fa8c58a97e35 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -223,6 +223,7 @@ struct fastrpc_map {
>  	u64 len;
>  	u64 raddr;
>  	u32 attr;
> +	u32 flags;
>  	struct kref refcount;
>  };
>  
> @@ -833,7 +834,7 @@ static dma_addr_t fastrpc_compute_dma_addr(struct fastrpc_user *fl, dma_addr_t s
>  }
>  
>  static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> -			      u64 len, u32 attr, struct fastrpc_map **ppmap)
> +			      u64 len, u32 attr, struct fastrpc_map **ppmap, int mflags)
>  {
>  	struct fastrpc_session_ctx *sess = fl->sctx;
>  	struct fastrpc_map *map = NULL;
> @@ -850,6 +851,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  
>  	map->fl = fl;
>  	map->fd = fd;
> +	map->flags = mflags;
>  	map->buf = dma_buf_get(fd);
>  	if (IS_ERR(map->buf)) {
>  		err = PTR_ERR(map->buf);
> @@ -924,13 +926,13 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  	return err;
>  }
>  
> -static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
> -			      u64 len, u32 attr, struct fastrpc_map **ppmap)
> +static int fastrpc_map_create(struct fastrpc_user *fl, int fd, u64 len, u32 attr,
> +			      struct fastrpc_map **ppmap, bool take_ref, int mflags)
>  {
> -	if (!fastrpc_map_lookup(fl, fd, ppmap, true))
> +	if (!fastrpc_map_lookup(fl, fd, ppmap, take_ref))
>  		return 0;
>  
> -	return fastrpc_map_attach(fl, fd, len, attr, ppmap);
> +	return fastrpc_map_attach(fl, fd, len, attr, ppmap, mflags);
>  }
>  
>  /*
> @@ -1000,23 +1002,25 @@ static int fastrpc_create_maps(struct fastrpc_invoke_ctx *ctx)
>  	int i, err;
>  
>  	for (i = 0; i < ctx->nscalars; ++i) {
> +		bool take_ref = i < ctx->nbufs;
> +		int mflags = 0;
>  
>  		if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1 ||
>  		    ctx->args[i].length == 0)
>  			continue;
>  
> -		if (i < ctx->nbufs)
> -			err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
> -				 ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
> -		else
> -			err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd,
> -				 ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
> +		/* Set the DMA handle mapping flag for DMA handles */
> +		if (i >= ctx->nbufs)
> +			mflags = FASTRPC_MAP_DMA_HANDLE;
> +
> +		err = fastrpc_map_create(ctx->fl, ctx->args[i].fd, ctx->args[i].length,
> +					 ctx->args[i].attr, &ctx->maps[i], take_ref, mflags);
>  		if (err) {
>  			dev_err(dev, "Error Creating map %d\n", err);
>  			return -EINVAL;
>  		}
> -
>  	}
> +
>  	return 0;
>  }
>  
> @@ -1144,6 +1148,16 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  		list[i].num = ctx->args[i].length ? 1 : 0;
>  		list[i].pgidx = i;
>  		if (ctx->maps[i]) {
> +			/* It is possible that map is created with
> +			 * mflags FASTRPC_MAP_DMA_HANDLE and take_ref
> +			 * is false. Check if map still exists or is
> +			 * being freed as take_ref is false
> +			 */
> +			if (fastrpc_map_lookup(ctx->fl, ctx->args[i].fd,
> +					       &ctx->maps[i], false)) {
> +				ctx->maps[i] = NULL;
> +				return -EINVAL;
> +			}
if map is created by R1 call with DMA handle flag, when any inbuf/outbuf
carries the same fd for R2 call, wouldn't the refcount get increased as
part of fastrpc_create_maps()? So if the refcount is already increased
there, why do you need to have a check here?>  			pages[i].addr =
ctx->maps[i]->dma_addr;
>  			pages[i].size = ctx->maps[i]->size;
>  		}
> @@ -1200,8 +1214,13 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
>  	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
>  		if (!fdlist[i])
>  			break;
> -		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false))
> +		/* Validate the map flags for DMA handles and skip freeing map if invalid */
> +		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false) &&
> +		    mmap->flags == FASTRPC_MAP_DMA_HANDLE) {
> +			/* Allow DMA handle maps to free only once */
> +			mmap->flags = 0;
flags are getting updated without any locks, this is considering that
DSP can updated a particular fd on fdlist only once. Can you add this
information here?>  			fastrpc_map_put(mmap);
> +		}
>  	}
>  
>  	return ret;
> @@ -1511,7 +1530,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>  	fl->pd = USER_PD;
>  
>  	if (init.filelen && init.filefd) {
> -		err = fastrpc_map_create(fl, init.filefd, init.filelen, 0, &map);
> +		err = fastrpc_map_create(fl, init.filefd, init.filelen, 0, &map, true, 0);
>  		if (err)
>  			goto err;
>  	}
> @@ -2107,9 +2126,14 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>  
>  	if (copy_from_user(&req, argp, sizeof(req)))
>  		return -EFAULT;
> -
> +	/*
> +	 * Prevent mapping backward compatible DMA handles here, as they are
> +	 * already mapped in the remote call.
> +	 */
> +	if (req.flags == FASTRPC_MAP_DMA_HANDLE)
> +		return -EINVAL;
>  	/* create SMMU mapping */
> -	err = fastrpc_map_create(fl, req.fd, req.length, 0, &map);
> +	err = fastrpc_map_create(fl, req.fd, req.length, 0, &map, true, 0);
>  	if (err) {
>  		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
>  		return err;
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index c6e2925f47e6..142ddaeed85f 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -44,6 +44,8 @@ enum fastrpc_map_flags {
>  	FASTRPC_MAP_FD = 2,
>  	FASTRPC_MAP_FD_DELAYED,
>  	FASTRPC_MAP_FD_NOMAP = 16,
> +	/* Map the DMA handle in the invoke call for backward compatibility */
> +	FASTRPC_MAP_DMA_HANDLE = 0x20000,
>  	FASTRPC_MAP_MAX,
>  };
>  

Re: [PATCH v2] misc: fastrpc: avoid duplicate DMA mappings in fastrpc_create_maps()

[Jianping]

在 2026/7/30 12:34, Ekansh Gupta 写道:
> On 16-07-2026 17:02, Jianping Li wrote:
>> DMA handles passed as invoke arguments (scalars beyond nbufs) may refer
>> to the same dma_buf fd as an input/output buffer argument. Taking an
>> extra reference for such DMA handle maps leads to duplicate mappings and
>> an unbalanced reference count, since DMA handle maps are released
>> separately when the DSP returns the fd through the fdlist.
>>
>> Fix this by not taking an extra reference for DMA handle arguments
>> (take_ref = false) and tagging them with FASTRPC_MAP_DMA_HANDLE. As
>> these maps are borrowed references, fastrpc_get_args() re-validates the
>> map via fastrpc_map_lookup() before dereferencing it, so it is not used
>> after being freed. fastrpc_put_args() only releases maps flagged as
>> FASTRPC_MAP_DMA_HANDLE and clears the flag to guarantee the map is freed
>> exactly once.
>>
>> Also reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since such
>> handles are already mapped implicitly during the remote invoke call and
>> must not be mapped again through the explicit MEM_MAP path.
>>
>> Fixes: 10df039834f84 ("misc: fastrpc: Skip reference for DMA handles")
>> Cc: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>> Patch [v1]: https://lore.kernel.org/all/20260625080832.17477-1-jianping.li@oss.qualcomm.com/
>>
>> Changes in v2:
>> - Rework the commit message to describe the DMA handle reference and
>>    lifetime problem more precisely.
>> - Introduce a new FASTRPC_MAP_DMA_HANDLE uapi flag and a 'flags' field
>>    in struct fastrpc_map to explicitly tag DMA handle maps, instead of
>>    relying only on the nbufs boundary / take_ref.
>> - Plumb an mflags argument through fastrpc_map_create() and
>>    fastrpc_map_attach() so DMA handle maps are tagged at creation time.
>> - Re-validate the borrowed map in fastrpc_get_args() via
>>    fastrpc_map_lookup() before dereferencing it, to avoid a
>>    use-after-free when the map was created with take_ref = false.
>> - In fastrpc_put_args(), only release maps tagged FASTRPC_MAP_DMA_HANDLE
>>    and clear the flag afterwards, so such maps are freed exactly once.
>> - Reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since these
>>    handles are already mapped implicitly during the remote invoke and
>>    must not be mapped again through the explicit MEM_MAP path.
>> ---
>>   drivers/misc/fastrpc.c      | 56 ++++++++++++++++++++++++++-----------
>>   include/uapi/misc/fastrpc.h |  2 ++
>>   2 files changed, 42 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index d86e79134c68..fa8c58a97e35 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -223,6 +223,7 @@ struct fastrpc_map {
>>   	u64 len;
>>   	u64 raddr;
>>   	u32 attr;
>> +	u32 flags;
>>   	struct kref refcount;
>>   };
>>   
>> @@ -833,7 +834,7 @@ static dma_addr_t fastrpc_compute_dma_addr(struct fastrpc_user *fl, dma_addr_t s
>>   }
>>   
>>   static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>> -			      u64 len, u32 attr, struct fastrpc_map **ppmap)
>> +			      u64 len, u32 attr, struct fastrpc_map **ppmap, int mflags)
>>   {
>>   	struct fastrpc_session_ctx *sess = fl->sctx;
>>   	struct fastrpc_map *map = NULL;
>> @@ -850,6 +851,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>   
>>   	map->fl = fl;
>>   	map->fd = fd;
>> +	map->flags = mflags;
>>   	map->buf = dma_buf_get(fd);
>>   	if (IS_ERR(map->buf)) {
>>   		err = PTR_ERR(map->buf);
>> @@ -924,13 +926,13 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>   	return err;
>>   }
>>   
>> -static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
>> -			      u64 len, u32 attr, struct fastrpc_map **ppmap)
>> +static int fastrpc_map_create(struct fastrpc_user *fl, int fd, u64 len, u32 attr,
>> +			      struct fastrpc_map **ppmap, bool take_ref, int mflags)
>>   {
>> -	if (!fastrpc_map_lookup(fl, fd, ppmap, true))
>> +	if (!fastrpc_map_lookup(fl, fd, ppmap, take_ref))
>>   		return 0;
>>   
>> -	return fastrpc_map_attach(fl, fd, len, attr, ppmap);
>> +	return fastrpc_map_attach(fl, fd, len, attr, ppmap, mflags);
>>   }
>>   
>>   /*
>> @@ -1000,23 +1002,25 @@ static int fastrpc_create_maps(struct fastrpc_invoke_ctx *ctx)
>>   	int i, err;
>>   
>>   	for (i = 0; i < ctx->nscalars; ++i) {
>> +		bool take_ref = i < ctx->nbufs;
>> +		int mflags = 0;
>>   
>>   		if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1 ||
>>   		    ctx->args[i].length == 0)
>>   			continue;
>>   
>> -		if (i < ctx->nbufs)
>> -			err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
>> -				 ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
>> -		else
>> -			err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd,
>> -				 ctx->args[i].length, ctx->args[i].attr, &ctx->maps[i]);
>> +		/* Set the DMA handle mapping flag for DMA handles */
>> +		if (i >= ctx->nbufs)
>> +			mflags = FASTRPC_MAP_DMA_HANDLE;
>> +
>> +		err = fastrpc_map_create(ctx->fl, ctx->args[i].fd, ctx->args[i].length,
>> +					 ctx->args[i].attr, &ctx->maps[i], take_ref, mflags);
>>   		if (err) {
>>   			dev_err(dev, "Error Creating map %d\n", err);
>>   			return -EINVAL;
>>   		}
>> -
>>   	}
>> +
>>   	return 0;
>>   }
>>   
>> @@ -1144,6 +1148,16 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>>   		list[i].num = ctx->args[i].length ? 1 : 0;
>>   		list[i].pgidx = i;
>>   		if (ctx->maps[i]) {
>> +			/* It is possible that map is created with
>> +			 * mflags FASTRPC_MAP_DMA_HANDLE and take_ref
>> +			 * is false. Check if map still exists or is
>> +			 * being freed as take_ref is false
>> +			 */
>> +			if (fastrpc_map_lookup(ctx->fl, ctx->args[i].fd,
>> +					       &ctx->maps[i], false)) {
>> +				ctx->maps[i] = NULL;
>> +				return -EINVAL;
>> +			}
> if map is created by R1 call with DMA handle flag, when any inbuf/outbuf
> carries the same fd for R2 call, wouldn't the refcount get increased as
> part of fastrpc_create_maps()? So if the refcount is already increased
> there, why do you need to have a check here?>  			pages[i].addr =
> ctx->maps[i]->dma_addr;
The check is not for the case you described (same fd reused as an in/out 
buffer in a later call). In that case i < nbufs so the map is created 
with take_ref = true and the refcount is indeed held, and the re-lookup 
simply passes.
The check is needed for DMA handle args themselves, which are created 
with take_ref = false. For those maps this context holds only a borrowed 
reference — the owning reference is dropped when the DSP returns the fd 
via fdlist in fastrpc_put_args(), which may run from a concurrent 
invocation. So between fastrpc_create_maps() and fastrpc_get_args() the 
map may already have been freed, and dereferencing ctx->maps[i] would be 
a use-after-free. The re-lookup re-validates that the map is still 
present before we dereference it.
(If you believe there is no competition, I will drop this code.)
>>   			pages[i].size = ctx->maps[i]->size;
>>   		}
>> @@ -1200,8 +1214,13 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
>>   	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
>>   		if (!fdlist[i])
>>   			break;
>> -		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false))
>> +		/* Validate the map flags for DMA handles and skip freeing map if invalid */
>> +		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false) &&
>> +		    mmap->flags == FASTRPC_MAP_DMA_HANDLE) {
>> +			/* Allow DMA handle maps to free only once */
>> +			mmap->flags = 0;
> flags are getting updated without any locks, this is considering that
> DSP can updated a particular fd on fdlist only once. Can you add this
> information here?>  			fastrpc_map_put(mmap);
Agreed, this relies on the DSP reporting a given fd in the fdlist only 
once, so no concurrent fastrpc_put_args() can update the same map's 
flags. I'll add a comment documenting this assumption in v3.
Will send v3 with the clarified comments.
>> +		}
>>   	}
>>   
>>   	return ret;
>> @@ -1511,7 +1530,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>>   	fl->pd = USER_PD;
>>   
>>   	if (init.filelen && init.filefd) {
>> -		err = fastrpc_map_create(fl, init.filefd, init.filelen, 0, &map);
>> +		err = fastrpc_map_create(fl, init.filefd, init.filelen, 0, &map, true, 0);
>>   		if (err)
>>   			goto err;
>>   	}
>> @@ -2107,9 +2126,14 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>>   
>>   	if (copy_from_user(&req, argp, sizeof(req)))
>>   		return -EFAULT;
>> -
>> +	/*
>> +	 * Prevent mapping backward compatible DMA handles here, as they are
>> +	 * already mapped in the remote call.
>> +	 */
>> +	if (req.flags == FASTRPC_MAP_DMA_HANDLE)
>> +		return -EINVAL;
>>   	/* create SMMU mapping */
>> -	err = fastrpc_map_create(fl, req.fd, req.length, 0, &map);
>> +	err = fastrpc_map_create(fl, req.fd, req.length, 0, &map, true, 0);
>>   	if (err) {
>>   		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
>>   		return err;
>> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
>> index c6e2925f47e6..142ddaeed85f 100644
>> --- a/include/uapi/misc/fastrpc.h
>> +++ b/include/uapi/misc/fastrpc.h
>> @@ -44,6 +44,8 @@ enum fastrpc_map_flags {
>>   	FASTRPC_MAP_FD = 2,
>>   	FASTRPC_MAP_FD_DELAYED,
>>   	FASTRPC_MAP_FD_NOMAP = 16,
>> +	/* Map the DMA handle in the invoke call for backward compatibility */
>> +	FASTRPC_MAP_DMA_HANDLE = 0x20000,
>>   	FASTRPC_MAP_MAX,
>>   };
>>   
> 

Re: [PATCH v2] misc: fastrpc: avoid duplicate DMA mappings in fastrpc_create_maps()

[Ekansh Gupta]

On 30-07-2026 13:53, Jianping wrote:
> 
> 
> 在 2026/7/30 12:34, Ekansh Gupta 写道:
>> On 16-07-2026 17:02, Jianping Li wrote:
>>> DMA handles passed as invoke arguments (scalars beyond nbufs) may refer
>>> to the same dma_buf fd as an input/output buffer argument. Taking an
>>> extra reference for such DMA handle maps leads to duplicate mappings and
>>> an unbalanced reference count, since DMA handle maps are released
>>> separately when the DSP returns the fd through the fdlist.
>>>
>>> Fix this by not taking an extra reference for DMA handle arguments
>>> (take_ref = false) and tagging them with FASTRPC_MAP_DMA_HANDLE. As
>>> these maps are borrowed references, fastrpc_get_args() re-validates the
>>> map via fastrpc_map_lookup() before dereferencing it, so it is not used
>>> after being freed. fastrpc_put_args() only releases maps flagged as
>>> FASTRPC_MAP_DMA_HANDLE and clears the flag to guarantee the map is freed
>>> exactly once.
>>>
>>> Also reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since such
>>> handles are already mapped implicitly during the remote invoke call and
>>> must not be mapped again through the explicit MEM_MAP path.
>>>
>>> Fixes: 10df039834f84 ("misc: fastrpc: Skip reference for DMA handles")
>>> Cc: stable@kernel.org
>>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>>> ---
>>> Patch [v1]: https://lore.kernel.org/all/20260625080832.17477-1-
>>> jianping.li@oss.qualcomm.com/
>>>
>>> Changes in v2:
>>> - Rework the commit message to describe the DMA handle reference and
>>>    lifetime problem more precisely.
>>> - Introduce a new FASTRPC_MAP_DMA_HANDLE uapi flag and a 'flags' field
>>>    in struct fastrpc_map to explicitly tag DMA handle maps, instead of
>>>    relying only on the nbufs boundary / take_ref.
>>> - Plumb an mflags argument through fastrpc_map_create() and
>>>    fastrpc_map_attach() so DMA handle maps are tagged at creation time.
>>> - Re-validate the borrowed map in fastrpc_get_args() via
>>>    fastrpc_map_lookup() before dereferencing it, to avoid a
>>>    use-after-free when the map was created with take_ref = false.
>>> - In fastrpc_put_args(), only release maps tagged FASTRPC_MAP_DMA_HANDLE
>>>    and clear the flag afterwards, so such maps are freed exactly once.
>>> - Reject FASTRPC_MAP_DMA_HANDLE in fastrpc_req_mem_map(), since these
>>>    handles are already mapped implicitly during the remote invoke and
>>>    must not be mapped again through the explicit MEM_MAP path.
>>> ---
>>>   drivers/misc/fastrpc.c      | 56 ++++++++++++++++++++++++++-----------
>>>   include/uapi/misc/fastrpc.h |  2 ++
>>>   2 files changed, 42 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>> index d86e79134c68..fa8c58a97e35 100644
>>> --- a/drivers/misc/fastrpc.c
>>> +++ b/drivers/misc/fastrpc.c
>>> @@ -223,6 +223,7 @@ struct fastrpc_map {
>>>       u64 len;
>>>       u64 raddr;
>>>       u32 attr;
>>> +    u32 flags;
>>>       struct kref refcount;
>>>   };
>>>   @@ -833,7 +834,7 @@ static dma_addr_t
>>> fastrpc_compute_dma_addr(struct fastrpc_user *fl, dma_addr_t s
>>>   }
>>>     static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>> -                  u64 len, u32 attr, struct fastrpc_map **ppmap)
>>> +                  u64 len, u32 attr, struct fastrpc_map **ppmap, int
>>> mflags)
>>>   {
>>>       struct fastrpc_session_ctx *sess = fl->sctx;
>>>       struct fastrpc_map *map = NULL;
>>> @@ -850,6 +851,7 @@ static int fastrpc_map_attach(struct fastrpc_user
>>> *fl, int fd,
>>>         map->fl = fl;
>>>       map->fd = fd;
>>> +    map->flags = mflags;
>>>       map->buf = dma_buf_get(fd);
>>>       if (IS_ERR(map->buf)) {
>>>           err = PTR_ERR(map->buf);
>>> @@ -924,13 +926,13 @@ static int fastrpc_map_attach(struct
>>> fastrpc_user *fl, int fd,
>>>       return err;
>>>   }
>>>   -static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
>>> -                  u64 len, u32 attr, struct fastrpc_map **ppmap)
>>> +static int fastrpc_map_create(struct fastrpc_user *fl, int fd, u64
>>> len, u32 attr,
>>> +                  struct fastrpc_map **ppmap, bool take_ref, int
>>> mflags)
>>>   {
>>> -    if (!fastrpc_map_lookup(fl, fd, ppmap, true))
>>> +    if (!fastrpc_map_lookup(fl, fd, ppmap, take_ref))
>>>           return 0;
>>>   -    return fastrpc_map_attach(fl, fd, len, attr, ppmap);
>>> +    return fastrpc_map_attach(fl, fd, len, attr, ppmap, mflags);
>>>   }
>>>     /*
>>> @@ -1000,23 +1002,25 @@ static int fastrpc_create_maps(struct
>>> fastrpc_invoke_ctx *ctx)
>>>       int i, err;
>>>         for (i = 0; i < ctx->nscalars; ++i) {
>>> +        bool take_ref = i < ctx->nbufs;
>>> +        int mflags = 0;
>>>             if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1 ||
>>>               ctx->args[i].length == 0)
>>>               continue;
>>>   -        if (i < ctx->nbufs)
>>> -            err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
>>> -                 ctx->args[i].length, ctx->args[i].attr, &ctx-
>>> >maps[i]);
>>> -        else
>>> -            err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd,
>>> -                 ctx->args[i].length, ctx->args[i].attr, &ctx-
>>> >maps[i]);
>>> +        /* Set the DMA handle mapping flag for DMA handles */
>>> +        if (i >= ctx->nbufs)
>>> +            mflags = FASTRPC_MAP_DMA_HANDLE;
>>> +
>>> +        err = fastrpc_map_create(ctx->fl, ctx->args[i].fd, ctx-
>>> >args[i].length,
>>> +                     ctx->args[i].attr, &ctx->maps[i], take_ref,
>>> mflags);
>>>           if (err) {
>>>               dev_err(dev, "Error Creating map %d\n", err);
>>>               return -EINVAL;
>>>           }
>>> -
>>>       }
>>> +
>>>       return 0;
>>>   }
>>>   @@ -1144,6 +1148,16 @@ static int fastrpc_get_args(u32 kernel,
>>> struct fastrpc_invoke_ctx *ctx)
>>>           list[i].num = ctx->args[i].length ? 1 : 0;
>>>           list[i].pgidx = i;
>>>           if (ctx->maps[i]) {
>>> +            /* It is possible that map is created with
>>> +             * mflags FASTRPC_MAP_DMA_HANDLE and take_ref
>>> +             * is false. Check if map still exists or is
>>> +             * being freed as take_ref is false
>>> +             */
>>> +            if (fastrpc_map_lookup(ctx->fl, ctx->args[i].fd,
>>> +                           &ctx->maps[i], false)) {
>>> +                ctx->maps[i] = NULL;
>>> +                return -EINVAL;
>>> +            }
>> if map is created by R1 call with DMA handle flag, when any inbuf/outbuf
>> carries the same fd for R2 call, wouldn't the refcount get increased as
>> part of fastrpc_create_maps()? So if the refcount is already increased
>> there, why do you need to have a check here?>             
>> pages[i].addr =
>> ctx->maps[i]->dma_addr;
> The check is not for the case you described (same fd reused as an in/out
> buffer in a later call). In that case i < nbufs so the map is created
> with take_ref = true and the refcount is indeed held, and the re-lookup
> simply passes.
> The check is needed for DMA handle args themselves, which are created
> with take_ref = false. For those maps this context holds only a borrowed
> reference — the owning reference is dropped when the DSP returns the fd
> via fdlist in fastrpc_put_args(), which may run from a concurrent
> invocation. So between fastrpc_create_maps() and fastrpc_get_args() the
> map may already have been freed, and dereferencing ctx->maps[i] would be
> a use-after-free. The re-lookup re-validates that the map is still
> present before we dereference it.
> (If you believe there is no competition, I will drop this code.)
Okay, I see this is only for DMA handles. Thanks for the
explaination.>>>               pages[i].size = ctx->maps[i]->size;
>>>           }
>>> @@ -1200,8 +1214,13 @@ static int fastrpc_put_args(struct
>>> fastrpc_invoke_ctx *ctx,
>>>       for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
>>>           if (!fdlist[i])
>>>               break;
>>> -        if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false))
>>> +        /* Validate the map flags for DMA handles and skip freeing
>>> map if invalid */
>>> +        if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap, false) &&
>>> +            mmap->flags == FASTRPC_MAP_DMA_HANDLE) {
>>> +            /* Allow DMA handle maps to free only once */
>>> +            mmap->flags = 0;
>> flags are getting updated without any locks, this is considering that
>> DSP can updated a particular fd on fdlist only once. Can you add this
>> information here?>              fastrpc_map_put(mmap);
> Agreed, this relies on the DSP reporting a given fd in the fdlist only
> once, so no concurrent fastrpc_put_args() can update the same map's
> flags. I'll add a comment documenting this assumption in v3.
> Will send v3 with the clarified comments.
Thanks!

//ekansh>>> +        }
>>>       }
>>>         return ret;
>>> @@ -1511,7 +1530,7 @@ static int fastrpc_init_create_process(struct
>>> fastrpc_user *fl,
>>>       fl->pd = USER_PD;
>>>         if (init.filelen && init.filefd) {
>>> -        err = fastrpc_map_create(fl, init.filefd, init.filelen, 0,
>>> &map);
>>> +        err = fastrpc_map_create(fl, init.filefd, init.filelen, 0,
>>> &map, true, 0);
>>>           if (err)
>>>               goto err;
>>>       }
>>> @@ -2107,9 +2126,14 @@ static int fastrpc_req_mem_map(struct
>>> fastrpc_user *fl, char __user *argp)
>>>         if (copy_from_user(&req, argp, sizeof(req)))
>>>           return -EFAULT;
>>> -
>>> +    /*
>>> +     * Prevent mapping backward compatible DMA handles here, as they
>>> are
>>> +     * already mapped in the remote call.
>>> +     */
>>> +    if (req.flags == FASTRPC_MAP_DMA_HANDLE)
>>> +        return -EINVAL;
>>>       /* create SMMU mapping */
>>> -    err = fastrpc_map_create(fl, req.fd, req.length, 0, &map);
>>> +    err = fastrpc_map_create(fl, req.fd, req.length, 0, &map, true, 0);
>>>       if (err) {
>>>           dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
>>>           return err;
>>> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
>>> index c6e2925f47e6..142ddaeed85f 100644
>>> --- a/include/uapi/misc/fastrpc.h
>>> +++ b/include/uapi/misc/fastrpc.h
>>> @@ -44,6 +44,8 @@ enum fastrpc_map_flags {
>>>       FASTRPC_MAP_FD = 2,
>>>       FASTRPC_MAP_FD_DELAYED,
>>>       FASTRPC_MAP_FD_NOMAP = 16,
>>> +    /* Map the DMA handle in the invoke call for backward
>>> compatibility */
>>> +    FASTRPC_MAP_DMA_HANDLE = 0x20000,
>>>       FASTRPC_MAP_MAX,
>>>   };
>>>   
>>
>