linux-kernel - Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put

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: <5EB2ED95-0BA3-4E71-8887-2FCAF002577C@nutanix.com>
Date: Thu, 27 Nov 2025 03:11:57 +0000
From: Jon Kohler <jon@...anix.com>
To: Jason Wang <jasowang@...hat.com>
CC: Arnd Bergmann <arnd@...db.de>, "Michael S. Tsirkin" <mst@...hat.com>,
        Eugenio Pérez <eperezma@...hat.com>,
        "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>,
        "virtualization@...ts.linux.dev"
	<virtualization@...ts.linux.dev>,
        Netdev <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Torvalds
	<torvalds@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>,
        Sean
 Christopherson <seanjc@...gle.com>,
        "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
        Russell King <linux@...linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
        Alexandre Belloni
	<alexandre.belloni@...tlin.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Drew Fustini <fustini@...nel.org>
Subject: Re: [PATCH net-next] vhost: use "checked" versions of get_user() and
 put_user()



> On Nov 26, 2025, at 8:08 PM, Jason Wang <jasowang@...hat.com> wrote:
> 
> On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@...anix.com> wrote:
>> 
>> 
>>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@...db.de> wrote:
>>> 
>>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@...anix.com> wrote:
>>>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@...hat.com> wrote:
>>>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@...anix.com> wrote:
>>>>> Same deal goes for __put_user() vs put_user by way of commit
>>>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>>>>> 
>>>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
>>>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
>>>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
>>>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
>>>>> about everything ARM would include this by default?
>>> 
>>> I think the more relevant commit is for 64-bit Arm here, but this does
>>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
>>> eliding access_ok checks in __{get, put}_user").
>> 
>> Ah! Right, this is definitely the important bit, as it makes it
>> crystal clear that these are exactly the same thing. The current
>> code is:
>> #define get_user        __get_user
>> #define put_user        __put_user
>> 
>> So, this patch changing from __* to regular versions is a no-op
>> on arm side of the house, yea?
>> 
>>> I would think that if we change the __get_user() to get_user()
>>> in this driver, the same should be done for the
>>> __copy_{from,to}_user(), which similarly skips the access_ok()
>>> check but not the PAN/SMAP handling.
>> 
>> Perhaps, thats a good call out. I’d file that under one battle
>> at a time. Let’s get get/put user dusted first, then go down
>> that road?
>> 
>>> In general, the access_ok()/__get_user()/__copy_from_user()
>>> pattern isn't really helpful any more, as Linus already
>>> explained. I can't tell from the vhost driver code whether
>>> we can just drop the access_ok() here and use the plain
>>> get_user()/copy_from_user(), or if it makes sense to move
>>> to the newer user_access_begin()/unsafe_get_user()/
>>> unsafe_copy_from_user()/user_access_end() and try optimize
>>> out a few PAN/SMAP flips in the process.
> 
> Right, according to my testing in the past, PAN/SMAP is a killer for
> small packet performance (PPS).

For sure, every little bit helps along that path

> 
>> 
>> In general, I think there are a few spots where we might be
>> able to optimize (vhost_get_vq_desc perhaps?) as that gets
>> called quite a bit and IIRC there are at least two flips
>> in there that perhaps we could elide to one? An investigation
>> for another day I think.
> 
> Did you mean trying to read descriptors in a batch, that would be
> better and with IN_ORDER it would be even faster as a single (at most
> two) copy_from_user() might work (without the need to use
> user_access_begin()/user_access_end().

Yep. I haven’t fully thought through it, just a drive-by idea
from looking at code for the recent work I’ve been doing, just
scratching my head thinking there *must* be something we can do
better there.

Basically on the get rx/tx bufs path as well as the
vhost_add_used_and_signal_n path, I think we could cluster together
some of the get/put users and copy to/from’s. Would take some
massaging, but I think there is something there.

>> 
>> Anyhow, with this info - Jason - is there anything else you
>> can think of that we want to double click on?
> 
> Nope.
> 
> Thanks

Ok thanks. Perhaps we can land this in next and let it soak in,
though, knock on wood, I don’t think there will be fallout
(famous last words!) ?