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

[Kees Cook <kees@kernel.org>]

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@kernel.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