[<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
Powered by blists - more mailing lists