From: Guenter Roeck <linux@roeck-us.net>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
linux-arch@vger.kernel.org, linux-toolchains@vger.kernel.org,
Masahiro Yamada <masahiroy@kernel.org>,
Kees Cook <keescook@chromium.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
linux-m68k@lists.linux-m68k.org
Subject: Re: [PATCH v2] kbuild: treat char as always unsigned
Date: Wed, 21 Dec 2022 06:53:32 -0800 [thread overview]
Message-ID: <20221221145332.GA2399037@roeck-us.net> (raw)
In-Reply-To: <20221019203034.3795710-1-Jason@zx2c4.com>
On Wed, Oct 19, 2022 at 02:30:34PM -0600, Jason A. Donenfeld wrote:
> Recently, some compile-time checking I added to the clamp_t family of
> functions triggered a build error when a poorly written driver was
> compiled on ARM, because the driver assumed that the naked `char` type
> is signed, but ARM treats it as unsigned, and the C standard says it's
> architecture-dependent.
>
> I doubt this particular driver is the only instance in which
> unsuspecting authors make assumptions about `char` with no `signed` or
> `unsigned` specifier. We were lucky enough this time that that driver
> used `clamp_t(char, negative_value, positive_value)`, so the new
> checking code found it, and I've sent a patch to fix it, but there are
> likely other places lurking that won't be so easily unearthed.
>
> So let's just eliminate this particular variety of heisensign bugs
> entirely. Set `-funsigned-char` globally, so that gcc makes the type
> unsigned on all architectures.
>
> This will break things in some places and fix things in others, so this
> will likely cause a bit of churn while reconciling the type misuse.
>
There is an interesting fallout: When running the m68k:q800 qemu emulation,
there are lots of warning backtraces.
WARNING: CPU: 0 PID: 23 at crypto/testmgr.c:5724 alg_test.part.0+0x7c/0x326
testmgr: alg_test_descs entries in wrong order: 'adiantum(xchacha12,aes)' before 'adiantum(xchacha20,aes)'
------------[ cut here ]------------
WARNING: CPU: 0 PID: 23 at crypto/testmgr.c:5724 alg_test.part.0+0x7c/0x326
testmgr: alg_test_descs entries in wrong order: 'adiantum(xchacha20,aes)' before 'aegis128'
and so on for pretty much every entry in the alg_test_descs[] array.
Bisect points to this patch, and reverting it fixes the problem.
It looks like the problem is that arch/m68k/include/asm/string.h
uses "char res" to store the result of strcmp(), and char is now
unsigned - meaning strcmp() will now never return a value < 0.
Effectively that means that strcmp() is broken on m68k if
CONFIG_COLDFIRE=n.
The fix is probably quite simple.
diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
index f759d944c449..b8f4ae19e8f6 100644
--- a/arch/m68k/include/asm/string.h
+++ b/arch/m68k/include/asm/string.h
@@ -42,7 +42,7 @@ static inline char *strncpy(char *dest, const char *src, size_t n)
#define __HAVE_ARCH_STRCMP
static inline int strcmp(const char *cs, const char *ct)
{
- char res;
+ signed char res;
asm ("\n"
"1: move.b (%0)+,%2\n" /* get *cs */
Does that make sense ? If so I can send a patch.
Guenter
next prev parent reply other threads:[~2022-12-21 14:53 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-19 16:26 [PATCH] kbuild: treat char as always signed Jason A. Donenfeld
2022-10-19 16:54 ` Segher Boessenkool
2022-10-19 17:14 ` Linus Torvalds
2022-10-19 17:26 ` Linus Torvalds
2022-10-19 18:10 ` Nick Desaulniers
2022-10-19 18:35 ` Linus Torvalds
2022-10-19 19:23 ` Andy Shevchenko
2022-10-19 19:36 ` Linus Torvalds
2022-10-19 17:43 ` Segher Boessenkool
2022-10-19 18:11 ` Linus Torvalds
2022-10-19 18:20 ` Nick Desaulniers
2022-10-19 18:56 ` Linus Torvalds
2022-10-19 19:11 ` Kees Cook
2022-10-19 19:30 ` Linus Torvalds
2022-10-19 20:35 ` Jason A. Donenfeld
2022-10-20 0:10 ` Linus Torvalds
2022-10-20 3:11 ` Jason A. Donenfeld
2022-10-19 20:15 ` Segher Boessenkool
2022-10-19 21:07 ` David Laight
2022-10-19 21:26 ` Segher Boessenkool
2022-10-20 10:41 ` Gabriel Paubert
2022-10-21 22:46 ` Linus Torvalds
2022-10-22 6:06 ` Gabriel Paubert
2022-10-22 18:16 ` Linus Torvalds
2022-10-23 20:23 ` Gabriel Paubert
2022-10-25 23:00 ` Kees Cook
2022-10-26 0:04 ` Jason A. Donenfeld
2022-10-26 15:41 ` Kees Cook
2022-10-19 19:54 ` Linus Torvalds
2022-10-19 20:23 ` Jason A. Donenfeld
2022-10-19 20:30 ` [PATCH v2] kbuild: treat char as always unsigned Jason A. Donenfeld
2022-10-19 23:56 ` Linus Torvalds
2022-10-20 0:02 ` Jason A. Donenfeld
2022-10-20 0:38 ` Linus Torvalds
2022-10-20 2:59 ` Jason A. Donenfeld
2022-10-20 18:41 ` Kees Cook
2022-10-21 1:01 ` Jason A. Donenfeld
2022-10-20 20:24 ` Segher Boessenkool
2022-10-24 9:24 ` Dan Carpenter
2022-10-24 9:30 ` Dan Carpenter
2022-10-24 16:33 ` Jason A. Donenfeld
2022-10-24 17:10 ` Linus Torvalds
2022-10-24 17:17 ` Jason A. Donenfeld
2022-10-25 19:22 ` Kalle Valo
2022-10-25 10:16 ` David Laight
2022-10-24 15:17 ` Jason A. Donenfeld
2022-12-21 14:53 ` Guenter Roeck [this message]
2022-12-21 15:05 ` Geert Uytterhoeven
2022-12-21 15:23 ` Guenter Roeck
2022-12-21 15:29 ` Rasmus Villemoes
2022-12-21 15:56 ` Guenter Roeck
2022-12-21 17:06 ` Linus Torvalds
2022-12-21 17:19 ` Guenter Roeck
2022-12-21 18:46 ` Linus Torvalds
2022-12-21 19:08 ` Linus Torvalds
2022-12-21 21:01 ` Guenter Roeck
2022-12-22 13:05 ` Geert Uytterhoeven
2022-12-22 10:41 ` David Laight
[not found] ` <f02e0ac7f2d805020a7ba66803aaff3e31b5eeff.camel@t-online.de>
2022-12-24 9:47 ` Geert Uytterhoeven
2022-12-30 11:39 ` David Laight
2022-12-30 13:13 ` David Laight
2023-01-02 8:29 ` Geert Uytterhoeven
2022-12-21 17:49 ` Andreas Schwab
2022-12-21 16:57 ` Geert Uytterhoeven
2022-10-19 20:58 ` [PATCH] kbuild: treat char as always signed David Laight
2022-10-26 0:10 ` make ctype ascii only? (was [PATCH] kbuild: treat char as always signed) Rasmus Villemoes
2022-10-26 18:10 ` Linus Torvalds
2022-10-27 7:59 ` Rasmus Villemoes
2022-10-27 18:28 ` Linus Torvalds
[not found] ` <202210201618.8XhEGsLd-lkp@intel.com>
2022-10-20 16:33 ` [PATCH] kbuild: treat char as always signed Jason A. Donenfeld
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221221145332.GA2399037@roeck-us.net \
--to=linux@roeck-us.net \
--cc=Jason@zx2c4.com \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=keescook@chromium.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-toolchains@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).