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

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

On 5/26/2026 3:06 PM, Dmitry Baryshkov wrote:
> 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@oss.qualcomm.com> 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@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> 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.

Got it, thanks.

I'll update the commit message in the next version patch.

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