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: <44905acd-3ac4-cfe5-5e91-d182c1959407@sangfor.com.cn>
Date:   Sat, 3 Jun 2023 09:51:34 +0800
From:   Ding Hui <dinghui@...gfor.com.cn>
To:     Alexander Duyck <alexander.duyck@...il.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, pengdonglin@...gfor.com.cn,
        huangcun@...gfor.com.cn
Subject: Re: [PATCH net-next] net: ethtool: Fix out-of-bounds copy to user

On 2023/6/3 2:02, Alexander Duyck wrote:
> On Fri, Jun 2, 2023 at 9:37 AM Andrew Lunn <andrew@...n.ch> wrote:
>>
>>> What this change is essentially doing is clamping the copied data to
>>> the lesser of the current value versus the value when the userspace
>>> was allocated. However I am wondering now if we shouldn't just update
>>> the size value and return that as some sort of error for the userspace
>>> to potentially reallocate and repeat until it has the right size.
>>
>> I'm not sure we should be putting any effort into the IOCTL
>> interface. It is deprecated. We should fix overrun problems, but i
>> would not change the API. Netlink handles this atomically, and that is
>> the interface tools should be using, not this IOCTL.
> 
> If that is the case maybe it would just make more sense to just return
> an error if we are at risk of overrunning the userspace allocated
> buffer.
> 

In that case, I can modify to return an error, however, I think the
ENOSPC or EFBIG mentioned in a previous email may not be suitable,
maybe like others length/size checking return EINVAL.

Another thing I wondered is that should I update the current length
back to user if user buffer is not enough, assuming we update the new
length with error returned, the userspace can use it to reallocate
buffer if he wants to, which can avoid re-call previous ioctl to get
the new length.

-- 
Thanks,
- Ding Hui

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ