Re: [PATCH v1] misc: fastrpc: Add reference counting for fastrpc_user structure

[Anandu Krishnan E <anandu.e@oss.qualcomm.com>]

On 13-04-2026 06:39 pm, Srinivas Kandagatla wrote:
>
> On 2/26/26 3:11 PM, Anandu Krishnan E wrote:
>> Add reference counting using kref to the fastrpc_user structure to
>> prevent use-after-free issues when contexts are freed from workqueue
>> after device release.
>>
>> The issue occurs when fastrpc_device_release() frees the user structure
>> while invoke contexts are still pending in the workqueue. When the
>> workqueue later calls fastrpc_context_free(), it attempts to access
>> buf->fl->cctx in fastrpc_buf_free(), leading to a use-after-free:
> I guess these are some of the pending invoke requests that are still
> processing and receive callback after user closes the device.
>
>>    pc : fastrpc_buf_free+0x38/0x80 [fastrpc]
>>    lr : fastrpc_context_free+0xa8/0x1b0 [fastrpc]
>>    ...
>>    fastrpc_context_free+0xa8/0x1b0 [fastrpc]
>>    fastrpc_context_put_wq+0x78/0xa0 [fastrpc]
>>    process_one_work+0x180/0x450
>>    worker_thread+0x26c/0x388
>>
>> Implement proper reference counting to fix this:
>> - Initialize kref in fastrpc_device_open()
>> - Take a reference in fastrpc_context_alloc() for each context
>> - Release the reference in fastrpc_context_free() when context is freed
>> - Release the initial reference in fastrpc_device_release()
>>
>> This ensures the user structure remains valid as long as there are
>> contexts holding references to it, preventing the race condition.
>>
>> Fixes: 6cffd79504ce ("misc: fastrpc: Add support for dmabuf exporter")
>> Cc: stable@kernel.org
>> Signed-off-by: Anandu Krishnan E <anandu.e@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 35 +++++++++++++++++++++++++++++++----
>>   1 file changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 47356a5d5804..3ababcf327d7 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -310,6 +310,8 @@ struct fastrpc_user {
>>   	spinlock_t lock;
>>   	/* lock for allocations */
>>   	struct mutex mutex;
>> +	/* Reference count */
>> +	struct kref refcount;
>>   };
>>   
>>   /* Extract SMMU PA from consolidated IOVA */
>> @@ -497,15 +499,36 @@ static void fastrpc_channel_ctx_put(struct fastrpc_channel_ctx *cctx)
>>   	kref_put(&cctx->refcount, fastrpc_channel_ctx_free);
>>   }
>>   
>> +static void fastrpc_user_free(struct kref *ref)
>> +{
>> +	struct fastrpc_user *fl = container_of(ref, struct fastrpc_user, refcount);
>> +
>> +	fastrpc_channel_ctx_put(fl->cctx);
>> +	mutex_destroy(&fl->mutex);
>> +	kfree(fl);
>> +}
>> +
>> +static void fastrpc_user_get(struct fastrpc_user *fl)
>> +{
>> +	kref_get(&fl->refcount);
>> +}
>> +
>> +static void fastrpc_user_put(struct fastrpc_user *fl)
>> +{
>> +	kref_put(&fl->refcount, fastrpc_user_free);
>> +}
>> +
>>   static void fastrpc_context_free(struct kref *ref)
>>   {
>>   	struct fastrpc_invoke_ctx *ctx;
>>   	struct fastrpc_channel_ctx *cctx;
>> +	struct fastrpc_user *fl;
>>   	unsigned long flags;
>>   	int i;
>>   
>>   	ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount);
>>   	cctx = ctx->cctx;
>> +	fl = ctx->fl;
>>   
>>   	for (i = 0; i < ctx->nbufs; i++)
>>   		fastrpc_map_put(ctx->maps[i]);
>> @@ -521,6 +544,8 @@ static void fastrpc_context_free(struct kref *ref)
>>   	kfree(ctx->olaps);
>>   	kfree(ctx);
>>   
>> +	/* Release the reference taken in fastrpc_context_alloc() */
>> +	fastrpc_user_put(fl);
>>   	fastrpc_channel_ctx_put(cctx);
>>   }
>>   
>> @@ -628,6 +653,8 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
>>   
>>   	/* Released in fastrpc_context_put() */
>>   	fastrpc_channel_ctx_get(cctx);
>> +	/* Take a reference to user, released in fastrpc_context_free() */
>> +	fastrpc_user_get(user);
>>   
>>   	ctx->sc = sc;
>>   	ctx->retval = -1;
>> @@ -658,6 +685,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
>>   	spin_lock(&user->lock);
>>   	list_del(&ctx->node);
>>   	spin_unlock(&user->lock);
>> +	fastrpc_user_put(user);
>>   	fastrpc_channel_ctx_put(cctx);
>>   	kfree(ctx->maps);
>>   	kfree(ctx->olaps);
>> @@ -1606,11 +1634,9 @@ static int fastrpc_device_release(struct inode *inode, struct file *file)
>>   	}
>>   
>>   	fastrpc_session_free(cctx, fl->sctx);
>> -	fastrpc_channel_ctx_put(cctx);
>> -
>> -	mutex_destroy(&fl->mutex);
>> -	kfree(fl);
>>   	file->private_data = NULL;
>> +	/* Release the reference taken in fastrpc_device_open */
>> +	fastrpc_user_put(fl);
> Now that we are trying to handle(wait) for the cases where wq will get
> callbacks after the fd is closed, how are we making sure that memory
> associated with these invoke, init memory and mmaps are not released?

I will move the cleanups to fastrpc_user_free so that cleanup wont happen until

all user references are released

>
>
> Also now that we have a refcount for the fastrpc_user, is it possible to
>   remove the refcount on the context, as this seems redundant now, we
> could just use the user refcount instead. May be other patch..

I will check on this and if possible to remove, i will share a separate patch

-Anandu

>
>
> --srini
>>   
>>   	return 0;
>>   }
>> @@ -1654,6 +1680,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
>>   	spin_lock_irqsave(&cctx->lock, flags);
>>   	list_add_tail(&fl->user, &cctx->users);
>>   	spin_unlock_irqrestore(&cctx->lock, flags);
>> +	kref_init(&fl->refcount);
>>   
>>   	return 0;
>>   }