linux-hardening - Re: [PATCH v2] kbuild: Re-enable -Wunterminated-string-initialization

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: <202508061158.49E749B46@keescook>
Date: Wed, 6 Aug 2025 12:05:32 -0700
From: Kees Cook <kees@...nel.org>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Nathan Chancellor <nathan@...nel.org>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Nicolas Schier <nicolas.schier@...ux.dev>,
	linux-kbuild@...r.kernel.org,
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] kbuild: Re-enable -Wunterminated-string-initialization

On Wed, Aug 06, 2025 at 05:29:55PM +0200, Alexander Lobakin wrote:
> From: Nathan Chancellor <nathan@...nel.org>

Thank you for the fixes Nathan! I'll dig through these and get them sent
out before I try to land this patch again -- "But COMPILE_TEST is never
wrong!" ;)

> > [...]
> > descriptions expected to be stable once they are released or are we able
> 
> Ethtool private stats are not "ABI" at all. Moreover, if they result in
> incorrect code, this needs to be fixed no matter if someone already
> wrote scripts dependent on these names or not.

Internally there isn't an ABI, but the userspace interface effectively has
an ABI: the strings are fixed-size and NUL-padded but not NUL-terminated.

> > to adjust them? We could maybe shave an 'o' from 'unknown' to easily
> > resolve this without losing much in the way of quick visual processing.
> 
> I've no idea why it's popular to define Ethtool stats names in drivers
> using a fixed array of ETH_GSTRING_LEN and then do memcpy().

The above is why: they are fixed-size, non-NUL-terminated strings, so
many drivers use this memcpy pattern. But not all.

> I've been always using just `const char * const[]` + strscpy() (then
> switched the latter to ethtool_puts()/ethtool_sprintf() -- we even have
> special helpers for that). In case some name goes past ETH_GSTRING_LEN,
> it would just be truncated, but always have \0 at the end.

Unfortunately this is not true: not all sources are NUL terminated.

> Plus most of the names are shorter than 32, so defining such arrays of
> 32 just wastes space in .rodata.

That IS true, but many drivers just keep giant blocks of data they can
memcpy. :(

Regardless, I will double-check this and see what needs to happen here.
I've fixed a lot of these already[1].

-Kees

[1] https://lore.kernel.org/lkml/20250416010210.work.904-kees@kernel.org/

-- 
Kees Cook