linux-kernel - Re: [RFC PATCH] rseq: Fix broken uapi field layout on 32-bit little endian

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: <CAHk-=whhEB-A-ahgeMsozDfdGNmP_MB9JVnV3bavGbeqgfpStQ@mail.gmail.com>
Date:   Mon, 24 Jan 2022 09:42:01 +0200
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        "H . Peter Anvin" <hpa@...or.com>, Paul Turner <pjt@...gle.com>,
        Linux API <linux-api@...r.kernel.org>,
        stable <stable@...r.kernel.org>,
        Florian Weimer <fw@...eb.enyo.de>,
        Andy Lutomirski <luto@...capital.net>,
        Dave Watson <davejwatson@...com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Russell King <linux@....linux.org.uk>,
        Andi Kleen <andi@...stfloor.org>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Ben Maurer <bmaurer@...com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Josh Triplett <josh@...htriplett.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Joel Fernandes <joelaf@...gle.com>
Subject: Re: [RFC PATCH] rseq: Fix broken uapi field layout on 32-bit little endian

On Sun, Jan 23, 2022 at 9:32 PM Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> wrote:
>
> The rseq rseq_cs.ptr.{ptr32,padding} uapi endianness handling is
> entirely wrong on 32-bit little endian: a preprocessor logic mistake
> wrongly uses the big endian field layout on 32-bit little endian
> architectures.
>
> Fortunately, those ptr32 accessors were never used within the kernel,
> and only meant as a convenience for user-space.

Please don't double down on something that was already broken once.

Just remove the broken 32-bit one entirely that the kernel doesn't
even use, and make everybody use

   __u64 ptr64;

and be done with it.

Adding a new "arch.ptr32" thing to replace the broken ptr.ptr32 is
just not worth it. This "convenience feature" never worked correctly
on any relevant architecture, so it clearly was never a convenience
feature, and deciding to try to re-do it because it was broken and
pointless the first time around isn't sane.

The definition of insanity is literally to do the same broken thing over again.

So just remove the broken ptr.ptr32 thing, don't add anything new to
replace it. Existing binaries will continue to work (or not work) as
well as they ever did. And new people getting new headers will get a
clear and proper compile error for the broken code that they can
trivially fix using 'ptr64' after they have actually thought about it
for a while.

Giving them a "arch.ptr32" doesn't help them at all. Quite the
reverse. You seem to hve the intention that they should just
mindlessly replace "ptr.ptr32" with "arch.ptr32", and now their code
won't actually work the same. Plus it will build with one version but
not the other.

In contrast, if you just tell people "ptr.ptr32 was always broken, use
ptr64 instead", it will actually work THE SAME with both old and new
headers. No odd "changed behavior from syntactic patch". No odd "this
won't work with older headers so now you have to add some
configuration or #ifdef".

The kernel cares about maintaining the ABI. The *binary* interface. If
the API was broken, it needs to be fixed. Not made worse by keeping
the broken fields and adding new ones for no reason.

               Linus