Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Dmitry Baryshkov]

On Tue, May 26, 2026 at 02:59:31PM +0800, Jianping Li wrote:
> 
> On 5/25/2026 5:35 PM, Dmitry Baryshkov wrote:
> > On Mon, 25 May 2026 at 11:34, Jianping Li <jianping.li@xxxxxxxxxxxxxxxx> wrote:
> > > 
> > > On 5/25/2026 4:30 PM, Dmitry Baryshkov wrote:
> > > > On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> > > > > On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > > > > > On Fri, May 15, 2026 at 08:42:14PM +0800, Jianping Li wrote:
> > > > > > > From: Ekansh Gupta<ekansh.gupta@xxxxxxxxxxxxxxxx>
> > > > > > > 
> > > > > > > fastrpc_req_munmap_impl() is called to unmap any buffer. The buffer is
> > > > > > > getting removed from the list after it is unmapped from DSP. This can
> > > > > > > create potential race conditions if any other thread removes the entry
> > > > > > > from list while unmap operation is ongoing. Remove the entry before
> > > > > > How can it remove the entry from the list?
> > > > > Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> > > > => commit message
> > > > 
> > > > > > > @@ -1898,7 +1897,14 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
> > > > > > >                     return -EINVAL;
> > > > > > >             }
> > > > > > > -  return fastrpc_req_munmap_impl(fl, buf);
> > > > > > > +  err = fastrpc_req_munmap_impl(fl, buf);
> > > > > > > +  if (err) {
> > > > > > > +          spin_lock(&fl->lock);
> > > > > > > +          list_add_tail(&buf->node, &fl->mmaps);
> > > > > > > +          spin_unlock(&fl->lock);
> > > > > > > +  }
> > > > > > Is it expected that userspace tries to unmap it again? Or why is it
> > > > > > being added to the list?
> > > > > User process can call unmap and fastrpc library won't call the unmap again.
> > > > In the other email you wrote that the driver can be used by random apps.
> > > > So... what happens if userspace unmaps it again? What if the userspace
> > > > _doesn't_ unmap it (although you've just readded it back)?
> > > If the same buf is unmapped again, because it has already been added back to the list, the unmap logic will be executed again.
> > > If userspace no longer performs unmap, the driver will not unmap it proactively.
> > > The Fastrpc driver will free up this list during fastrpc user-free.
> > It will free the list. But what happens with the memory mapping?
> 
> When device release is called, DSP side PD is also cleaned up, which includes cleaning up of DSP side mappings
> 
> Before kref_put of fl, we call DSP process release, where DSP PD is cleaned up.
> 
> After calling this, we go ahead and do buf_free of the list

Okay, capture this discussion in the commit message.

> 
> Thanks,
> Jianping.
> 
> > 
> > > > > Fastrpc driver will free up this list during fastrpc user-free.
> > 
> > 

-- 
With best wishes
Dmitry