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

[Anandu Krishnan E]

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@xxxxxxxxxx
> > Signed-off-by: Anandu Krishnan E <anandu.e@xxxxxxxxxxxxxxxx>
> > ---
> >   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;
> >   }