linux-kernel - Re: [PATCH v3 1/2] capability: add cap

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-=wjrQ_+PCrdNRWKsmf=SZP45_N7r51NbbB55DUdGb5f76A@mail.gmail.com>
Date:   Tue, 28 Feb 2023 11:51:37 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mateusz Guzik <mjguzik@...il.com>
Cc:     Casey Schaufler <casey@...aufler-ca.com>,
        Serge Hallyn <serge@...lyn.com>, viro@...iv.linux.org.uk,
        paul@...l-moore.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v3 1/2] capability: add cap_isidentical

On Tue, Feb 28, 2023 at 11:39 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> This actually looks sane enough that I might even boot it. Call me crazy.

Oh, and while I haven't actually booted it or tested it in any way, I
did verify that it changes

-       movq    48(%rbx), %rax
-       movq    56(%rbx), %rcx
-       cmpl    %eax, %ecx
-       jne     .LBB58_13
-       shrq    $32, %rax
-       shrq    $32, %rcx
-       cmpl    %eax, %ecx
-       jne     .LBB58_13

into

+       movq    56(%rbx), %rax
+       cmpq    48(%rbx), %rax
+       jne     .LBB58_12

because it looks like clang was smart enough to unroll the silly
fixed-size loop and do the two adjacent 32-bit loads of the old cap[]
array as one 64-bit load, but then was _not_ smart enough to combine
the two 32-bit compares into one 64-bit one.

And gcc didn't do the load optimization (which is questionable since
it then just results in extra shifts and extra registers), so it just
kept it as two 32-bit loads and compares. Again, with the patch, gcc
obviously does the sane "one 64-bit load, one 64-bit compare" too.

There's a lot to be said for compiler optimizations fixing up silly
source code, but I personally would just prefer to make the source
code DTRT.

Could the compiler have been even smarter and generated the same code?
Yes it could. We shouldn't expect that, though. Particularly when the
sane code is much more legible to humans too.

                 Linus