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

[Kees Cook <kees@kernel.org>]

On Wed, Aug 06, 2025 at 05:29:55PM +0200, Alexander Lobakin wrote:
> From: Nathan Chancellor <nathan@kernel.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