linux-kernel - Re: [PATCH] kbuild: treat char as always signed

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-=wiD90ZphsbTzSetHsK3_kQzhgyiYYS0msboVsJ3jbNALQ@mail.gmail.com>
Date:   Wed, 19 Oct 2022 11:56:00 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Segher Boessenkool <segher@...nel.crashing.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
        linux-arch@...r.kernel.org, linux-toolchains@...r.kernel.org,
        Masahiro Yamada <masahiroy@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] kbuild: treat char as always signed

On Wed, Oct 19, 2022 at 11:21 AM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
>
> A few times in the past, we've split a warning flag into a group so
> that we could be more specific about distinct cases. Perhaps if
> -Wpointer-sign was a group that implied -Wpointer-char-sign, then the
> kernel could use -Wpointer-sign -Wno-pointer-char-sign.

That might be interesting, just to see how much of the kernel is about
'char *' and how much is other noise.

Just for fun (for some definition of "fun") I tried to remove the
-Wno-pointer-sign thing, and started building a kernel.

After fixing fortify-string.h to not complain (which was indeed about
strlen() signedness), it turns out a lot were still about 'char', but
not necessarily the <string,h> functions.

We use 'unsigned char *' for our dentry data, for example, and then you get

     warning: pointer targets in initialization of ‘const unsigned
char *’ from ‘char *’ differ in signedness

when you do something like

    QSTR_INIT(NULL_FILE_NAME,

which is simply doing a regular initializer assignment, and wants to
assign a constant string (in this case the constant string "null") to
that "const unsigned char *name".

That's certainly another example of "why the heck did the compiler
warn about that thing".

You can literally try to compile this one-liner with gcc:

     const unsigned char *c = "p";

and it will complain. What a hugely pointless warning.

BUT.

It turns out we have a lot of non-char warnings too.

The kernel does all these "generic functions" that are based on size, like

        atomic_try_cmpxchg_acquire()

which are basically defined to be about "int sized object", but with
unspecified sign.

And the sign is basically pointless. Some people want "unsigned int",
others might want a signed int.

So from a quick grep, we do have a lot of strlen/strcpy cases, but we
also do have a lot of other cases.

Hundreds and hundreds of that atomic_try_cmpxchg_acquire(), for
example. And they might be trivial to fix (it might be similar to the
fortify-string.h one where it's just a header file that generates most
of them in one single place), but with all the ones that are just
clearly the compiler being silly, they aren't really even worth
looking at.

                    Linus