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] [thread-next>] [day] [month] [year] [list]
Message-ID: <868ceb0e-f4ba-4495-a1e1-0e387049281a@huawei.com>
Date: Sat, 13 Sep 2025 18:40:23 +0800
From: huangchenghai <huangchenghai2@...wei.com>
To: Greg KH <gregkh@...uxfoundation.org>, Zhangfei Gao
	<zhangfei.gao@...aro.org>
CC: <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, 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).

Regards,
ChengHai

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ