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

[Jianping Li <jianping.li@oss.qualcomm.com>]

On 7/3/2026 1:16 PM, Ekansh Gupta wrote:
> On 25-06-2026 13:38, Jianping Li wrote:
>> fastrpc_create_maps() performs map lookup only for buffer
>> arguments (i < ctx->nbufs) via fastrpc_map_create(). For
>> arguments beyond this range, no lookup is performed, which
>> can result in duplicate DMA mappings for the same file
>> descriptor.
>>
>> Additionally, if the same file descriptor is passed multiple
>> times within a single invocation, performing lookups with
>> reference counting would increment the reference multiple
>> times, while fastrpc_put_args() would release it only once,
>> leading to an imbalanced reference count.
>>
>> Fix this by allowing fastrpc_map_create() to control whether
>> the lookup should take a reference. For arguments beyond
>> ctx->nbufs, the lookup is performed without taking a
>> reference, ensuring that existing mappings are reused
>> without introducing duplicate DMA mappings or reference
>> count imbalance.
>>
>> Fixes: 10df039834f84 ("misc: fastrpc: Skip reference for DMA handles")
>> Cc: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 22 +++++++++++-----------
>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index a9b2ae44c06f..2622a1360a65 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -924,9 +924,9 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>   }
>>   
>>   static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
>> -			      u64 len, u32 attr, struct fastrpc_map **ppmap)
>> +			      u64 len, u32 attr, struct fastrpc_map **ppmap, bool take_ref)
>>   {
>> -	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);
>> @@ -999,23 +999,23 @@ static int fastrpc_create_maps(struct fastrpc_invoke_ctx *ctx)
>>   	int i, err;
>>   
>>   	for (i = 0; i < ctx->nscalars; ++i) {
>> +		bool take_ref = true;
> bool take_ref = i < ctx->nbufs, also add a comment stating why reference
> should be skipped for DMA handles.>

Thanks for the review.

That's a good point. I'll initialize the variable as:

     bool take_ref = i < ctx->nbufs;

and add a comment explaining that DMA handle arguments can reuse an
existing mapping and should not take an additional reference, otherwise
multiple references may be acquired for the same fd while only a single
release is performed during cleanup.

>>   		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]);
>> +		if (i >= ctx->nbufs)
>> +			take_ref = false;
>> +
>> +		err = fastrpc_map_create(ctx->fl, ctx->args[i].fd, ctx->args[i].length,
>> +			 ctx->args[i].attr, &ctx->maps[i], take_ref);
>>   		if (err) {
>>   			dev_err(dev, "Error Creating map %d\n", err);
>>   			return -EINVAL;
>>   		}
>> -
>>   	}
>> +
>>   	return 0;
>>   }
>>   
>> @@ -1508,7 +1508,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);
>>   		if (err)
>>   			goto err;
>>   	}
>> @@ -2100,7 +2100,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>>   		return -EFAULT;
>>   
>>   	/* 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);
>>   	if (err) {
>>   		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
>>   		return err;