linux-kernel - Re: FYI: path walking optimizations pending for 6.11

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: <90b46873-f60f-4ece-bef6-b8eed3b68ac1@app.fastmail.com>
Date: Thu, 20 Jun 2024 09:53:20 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Linus Torvalds" <torvalds@...ux-foundation.org>,
 "Christian Brauner" <brauner@...nel.org>,
 "Alexander Viro" <viro@...iv.linux.org.uk>
Cc: linux-fsdevel <linux-fsdevel@...r.kernel.org>,
 "the arch/x86 maintainers" <x86@...nel.org>,
 "Linux ARM" <linux-arm-kernel@...ts.infradead.org>,
 "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
 "Alexei Starovoitov" <ast@...nel.org>
Subject: Re: FYI: path walking optimizations pending for 6.11

On Wed, Jun 19, 2024, at 22:25, Linus Torvalds wrote:
> The arm64-uaccess branch is just what it says, and makes a big
> difference in strncpy_from_user(). The "access_ok()" change is
> certainly debatable, but I think needs to be done for sanity. I think
> it's one of those "let's do it, and if it causes problems we'll have
> to fix things up" things.

I'm a bit worried about the access_ok() being so different from
the other architectures, after I previously saw all the ways
it could go wrong because of subtle differences.

I don't mind making the bit that makes the untagging
unconditional, and I can see how this improves code
generation. I've tried comparing your version against
the more conventional

static inline int access_ok(const void __user *p, unsigned long size)
{
        return likely(__access_ok(untagged_addr(p), size));
}

Using gcc-13.2, I see your version is likely better in all
cases, but not by much: for the constant-length case, it
saves only one instruction (combining the untagging with the
limit), while for a variable length it avoids a branch.

On a 24MB kernel image, I see this add up to a size difference
of 12KB, while the total savings from avoiding the conditional
untagging are 76KB.

Do you see a measurable performance difference between your
version and the one above?

On a related note, I see that there is one caller of
__access_ok() in common code, and this was added in
d319f344561d ("mm: Fix copy_from_user_nofault().").
I think that one should just go back to using access_ok()
after your 6ccdc91d6af9 ("x86: mm: remove
architecture-specific 'access_ok()' define"). In the
current version, it otherwise misses the untagging
on arm64.

    Arnd