linux-kernel - Re: [PATCH -mm] -funsigned-char, x86: make struct p4_event

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2022 13:17:33 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
        mm-commits@...r.kernel.org, masahiroy@...nel.org,
        gregkh@...uxfoundation.org, andriy.shevchenko@...ux.intel.com,
        Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH -mm] -funsigned-char, x86: make struct p4_event_bind::cntr
 signed array

On Thu, Oct 20, 2022 at 12:39 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> So getting rid of that kind of pointless noise is just about all the
> effort here.

Current status: of 22.5k object files, 971 have differences.

In many cases, the differences are small and trivial. Example:

 - fscrypt_show_test_dummy_encryption() does that same "print a char with %c"

        seq_printf(seq, "%ctest_dummy_encryption=v%d", sep, vers);

which is entirely harmless and exactly the same as that  (but I most
certainly haven't figured out how to automatically script away that
"oh, %c is fine).

And in other cases, there's no actual difference at all, just
different register usage, so the diff looks fairly big, but doesn't
seem to be real.  In one case I looked at, it started with a 'movzbl',
but it was that in both cases, because the type was actually 'unsigned
char' to begin with. But for some reason it just used different
registers. Example:

 - handle_control_request() in drivers/usb/gadget/udc/dummy_hcd.c

   The reason here *seems* to be that

                        char *buf;
                        buf = (char *)urb->transfer_buffer;

   where it really probably should be 'u8 *buf', since it actually
does a cast to 'u8' in one place, but there isn't even any read of
that 'buf' pointer. So the difference seems to be entirely just some
"different type in assignment" cast internal to gcc that then
incidentally generated a random other choice in register allocation.

And in some cases the differences are enormous:

 - drivers/net/wireless/ralink/rt2x00/rt2800lib.c generates a 220kB diff

which seems to be due to entirely different inlining decisions or
something, and the differences are so enormous that I didn't even
start looking at the cause.

There's a fair number of things in between, like fs/ext4/super.c that
generates a lot of differences, some of them obvious, some of them
very much not obvious that may br similar to the
handle_control_request() ones.

*Presumably* the ext4 super.c code is fine (since it has been used on
architectures that already had unsigned char), but it actually
generates a bigger diff than the p4 events driver does...

And that arch/x86/events/intel/p4.c thing that Alexey found sadly does
not stand out at all in that "somewhere in the middle" bunch.

So I think my "hey, we can automate the comparison" is pretty much a
dud, but I'm not giving up quite yet. It's annoying how *most* of the
kernel files show no differences at all, but then I can't even figure
our why other files do show differences with no obvious reason for
them at all.

                Linus