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

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: <Y1C5qZ4sSGuUfXmF@zx2c4.com>
Date:   Wed, 19 Oct 2022 20:59:53 -0600
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     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 v2] kbuild: treat char as always unsigned

On Wed, Oct 19, 2022 at 05:38:55PM -0700, Linus Torvalds wrote:
> On Wed, Oct 19, 2022 at 5:02 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
> >
> > Given I've started with cleaning up one driver already, I'll keep my eye
> > on further breakage.
> 
> I wonder if we could just check for code generation differences some way.
> Having some scripting automation that just notices "this changes code
> generation in function X" might actually be interesting, and judging
> by my quick tests might not be *too* verbose.

Or even just some allyesconfig diffing. 

> I tested a couple of files, and was able to find differences, eg
> 
>   # kernel/sched/core.c:8861: pr_info("task:%-15.15s state:%c",
> p->comm, task_state_to_char(p));
>  - movzbl state_char.149(%rax), %edx # state_char[_60], state_char[_60]
>  + movsbl state_char.149(%rax), %edx # state_char[_60], state_char[_60]
>    call _printk #
> 
> because the 'char' for the '%c' is passed as an integer. And the

Seems harmless though.

> tracing code has the
> 
>         .is_signed = is_signed_type(_type)
> 
> initializers, which obviously change when the type is 'char'.

And likewise, looking at the types of initializers that's used with.
Actually, for the array one, unsigned is probably more sensible anyway.

The thing is, anyhow, that most code that works without -funsigned-char
*will* work with it, because the core of the kernel obviously works fine
on ARM already. The problematic areas will be x86-specific drivers that
have never been tested on other archs. i915 comes to mind -- as a
general rule, it already does all manner of insane things. But there's
obviously a lot of other hardware that's only ever run on Intel. So I'm
much more concerned about that than I am about code in, say, kernel/sched.

Jason