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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <202509050856.E5D810BC@keescook>
Date: Fri, 5 Sep 2025 08:59:15 -0700
From: Kees Cook <kees@...nel.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Alexander Lobakin <aleksander.lobakin@...el.com>,
	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 Tue, Aug 05, 2025 at 02:48:23PM -0700, Nathan Chancellor wrote:
> On Tue, Aug 05, 2025 at 04:50:28PM +0200, Alexander Lobakin wrote:
> > From: Nathan Chancellor <nathan@...nel.org>
> > Date: Sun, 3 Aug 2025 10:32:35 -0700
> > 
> > > On Sat, Aug 02, 2025 at 11:43:32AM -0700, Kees Cook wrote:
> > >> With the few remaining fixes now landed, we can re-enable the option
> > >> -Wunterminated-string-initialization (via -Wextra). Both GCC and Clang
> > >> have the required multi-dimensional nonstring attribute support.
> > 
> > [...]
> > 
> > > diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
> > > index 55a1a96cd834..05d4323c6a13 100644
> > > --- a/drivers/net/ethernet/ti/netcp_ethss.c
> > > +++ b/drivers/net/ethernet/ti/netcp_ethss.c
> > > @@ -771,7 +771,7 @@ static struct netcp_module xgbe_module;
> > >  
> > >  /* Statistic management */
> > >  struct netcp_ethtool_stat {
> > > -	char desc[ETH_GSTRING_LEN];
> > > +	char desc[ETH_GSTRING_LEN] __nonstring;
> > 
> > 
> > Hmmm, ETH_GSTRING_LEN is the maximum length of the driver's statistics
> > name to be reported to Ethtool and this *includes* \0 at the end.

This is not true. ethtool uses non-C-String logic to report these values
in userspace. These are _not_ C-Strings -- they're NUL padded to
ETH_GSTRING_LEN, so some drivers _happen_ to use C-String APIs, but
they're "wasting" a byte.

> > If this compilation flag triggers a warning here, the driver devs need
> > to fix their code. There should always be \0 at the end, `desc` is a
> > "proper" C 0-terminated string.
> 
> Ack, I had misunderstood a previous fix that Kees did for a similar but
> different instance of the warning in another Ethernet driver and I
> did not look much further than the driver copying these values around
> with memcpy(). This does trigger a warning, from the original message:

These don't need to be renamed -- they just need to use memcpy instead
of C String helpers.

ETH_GSTRING stuff is not NUL terminated.

-Kees

-- 
Kees Cook