[PATCH] net: ethtool: Adjust exactly ETH_GSTRING_LEN-long stats to use memcpy

[Keller, Jacob E]

> -----Original Message-----
> From: Kees Cook <kees at kernel.org>
> Sent: Wednesday, April 16, 2025 10:49 AM
> To: Kory Maincent <kory.maincent at bootlin.com>
> Cc: Jakub Kicinski <kuba at kernel.org>; Andrew Lunn <andrew+netdev at lunn.ch>;
> David S. Miller <davem at davemloft.net>; Dumazet, Eric <edumazet at google.com>;
> Paolo Abeni <pabeni at redhat.com>; Claudiu Manoil <claudiu.manoil at nxp.com>;
> Vladimir Oltean <vladimir.oltean at nxp.com>; Wei Fang <wei.fang at nxp.com>; Clark
> Wang <xiaoning.wang at nxp.com>; Jeroen de Borst <jeroendb at google.com>;
> Harshitha Ramamurthy <hramamurthy at google.com>; Ido Schimmel
> <idosch at nvidia.com>; Petr Machata <petrm at nvidia.com>; Maxime Coquelin
> <mcoquelin.stm32 at gmail.com>; Alexandre Torgue
> <alexandre.torgue at foss.st.com>; Simon Horman <horms at kernel.org>; Geoff
> Levand <geoff at infradead.org>; Wolfram Sang <wsa+renesas at sang-
> engineering.com>; Lobakin, Aleksander <aleksander.lobakin at intel.com>; Praveen
> Kaligineedi <pkaligineedi at google.com>; Willem de Bruijn <willemb at google.com>;
> Joshua Washington <joshwash at google.com>; Furong Xu <0x1207 at gmail.com>;
> Russell King (Oracle) <rmk+kernel at armlinux.org.uk>; Jisheng Zhang
> <jszhang at kernel.org>; Petr Tesarik <petr at tesarici.cz>; netdev at vger.kernel.org;
> imx at lists.linux.dev; linux-stm32 at st-md-mailman.stormreply.com; linux-arm-
> kernel at lists.infradead.org; Richard Cochran <richardcochran at gmail.com>; Keller,
> Jacob E <jacob.e.keller at intel.com>; Shannon Nelson
> <shannon.nelson at amd.com>; Ziwei Xiao <ziweixiao at google.com>; Shailend
> Chand <shailend at google.com>; Choong Yong Liang
> <yong.liang.choong at linux.intel.com>; Andrew Halaney <ahalaney at redhat.com>;
> linux-kernel at vger.kernel.org; linux-hardening at vger.kernel.org
> Subject: Re: [PATCH] net: ethtool: Adjust exactly ETH_GSTRING_LEN-long stats to
> use memcpy
> 
> On Wed, Apr 16, 2025 at 11:03:51AM +0200, Kory Maincent wrote:
> > On Tue, 15 Apr 2025 18:02:15 -0700
> > Kees Cook <kees at kernel.org> wrote:
> >
> > > Many drivers populate the stats buffer using C-String based APIs (e.g.
> > > ethtool_sprintf() and ethtool_puts()), usually when building up the
> > > list of stats individually (i.e. with a for() loop). This, however,
> > > requires that the source strings be populated in such a way as to have
> > > a terminating NUL byte in the source.
> > >
> > > Other drivers populate the stats buffer directly using one big memcpy()
> > > of an entire array of strings. No NUL termination is needed here, as the
> > > bytes are being directly passed through. Yet others will build up the
> > > stats buffer individually, but also use memcpy(). This, too, does not
> > > need NUL termination of the source strings.
> > >
> > > However, there are cases where the strings that populate the
> > > source stats strings are exactly ETH_GSTRING_LEN long, and GCC
> > > 15's -Wunterminated-string-initialization option complains that the
> > > trailing NUL byte has been truncated. This situation is fine only if the
> > > driver is using the memcpy() approach. If the C-String APIs are used,
> > > the destination string name will have its final byte truncated by the
> > > required trailing NUL byte applied by the C-string API.
> > >
> > > For drivers that are already using memcpy() but have initializers that
> > > truncate the NUL terminator, mark their source strings as __nonstring to
> > > silence the GCC warnings.
> >
> > Shouldn't we move on to ethtool_cpy in these drivers too to unify the code?
> 
> I decided that the code churn wasn't worth it. Perhaps in a follow-up
> patch if folks want to see the removal of the explicit memcpy() uses?
> 

A follow-up seems better to me.

Thanks,
Jake

> --
> Kees Cook