linux-kernel - Re: [PATCH 3/4] uacce: implement mremap in uacce_vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a6be1e20-ed27-4883-8aaa-a49a2f5438a2@huawei.com>
Date: Mon, 15 Sep 2025 09:48:48 +0800
From: huangchenghai <huangchenghai2@...wei.com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: Zhangfei Gao <zhangfei.gao@...aro.org>, <wangzhou1@...ilicon.com>,
	<linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
	<linux-crypto@...r.kernel.org>, <fanghao11@...wei.com>,
	<shenyang39@...wei.com>, <qianweili@...wei.com>, <linwenkai6@...ilicon.com>,
	<liulongfang@...wei.com>
Subject: Re: [PATCH 3/4] uacce: implement mremap in uacce_vm_ops to return
 -EPERM


On Sat, Sep 13, 2025 at 7:06 PM, Greg KH wrote:
> On Sat, Sep 13, 2025 at 06:40:23PM +0800, huangchenghai wrote:
>> On Sat, 6 Sept 2025 at 20:03, Greg KH wrote:
>>> On Thu, Aug 28, 2025 at 01:59:48PM +0800, Zhangfei Gao wrote:
>>>> Hi, Greg
>>>>
>>>> On Fri, 22 Aug 2025 at 19:46, Greg KH <gregkh@...uxfoundation.org> wrote:
>>>>> On Fri, Aug 22, 2025 at 06:39:03PM +0800, Chenghai Huang wrote:
>>>>>> From: Yang Shen <shenyang39@...wei.com>
>>>>>>
>>>>>> The current uacce_vm_ops does not support the mremap operation of
>>>>>> vm_operations_struct. Implement .mremap to return -EPERM to remind
>>>>>> users
>>>>> Why is this needed?  If mremap is not set, what is the value returned?
>>>> Did some debug locally.
>>>>
>>>> By default, mremap is permitted.
>>>>
>>>> With mremap, the original vma is released,
>>>> The vma_close is called and free resources, including q->qfr.
>>>>
>>>> However, vma->vm_private_data (q) is copied to the new vma.
>>>> When the new vma is closed, vma_close will get q and q->qft=0.
>>>>
>>>> So disable mremap here looks safer.
>>>>
>>>>> And why is -EPERM the correct value to return here?  That's not what the
>>>>> man pages say is valid :(
>>>> if disable mremap, -1 is returned as MAP_FAILED.
>>>> The errno is decided by the return value, -EPERM (-1) or -EINVAL (-22).
>>>> man mremap only lists -EINVAL.
>>>>
>>>> However, here the driver wants to disable mremap, looks -EPERM is more suitable.
>>> Disabling mremap is not a permission issue, it's more of an invalid
>>> call?  I don't know, what do other drivers do?
>>>
>>> thanks,
>>>
>>> greg k-h
>> Hi Greg,
>>
>> Thank you for your feedback.
>>
>> The reason we need to explicitly disable mremap is that when the
>> driver does not implement .mremap, it uses the default mremap
>> method. This could lead to a risk scenario:
>>
>> An application might first mmap address p1, then mremap to p2,
>> followed by munmap(p1), and finally munmap(p2). Since the default
>> mremap copies the original vma's vm_private_data (i.e., q) to the
>> new vma, both munmap operations would trigger vma_close, causing
>> q->qfr to be freed twice(qfr will be set to null here, so repeated release
>> is ok).
> Great, can you please include that in the changelog text?
>
> thanks,
>
> greg k-h
Sure, I will add changelog in the v2 patch lately.

Thanks,
ChengHai