> -----Original Message-----
> From: Kees Cook <[email protected]>
> Sent: Wednesday, April 16, 2025 10:49 AM
> To: Kory Maincent <[email protected]>
> Cc: Jakub Kicinski <[email protected]>; Andrew Lunn <[email protected]>;
> David S. Miller <[email protected]>; Dumazet, Eric <[email protected]>;
> Paolo Abeni <[email protected]>; Claudiu Manoil <[email protected]>;
> Vladimir Oltean <[email protected]>; Wei Fang <[email protected]>; Clark
> Wang <[email protected]>; Jeroen de Borst <[email protected]>;
> Harshitha Ramamurthy <[email protected]>; Ido Schimmel
> <[email protected]>; Petr Machata <[email protected]>; Maxime Coquelin
> <[email protected]>; Alexandre Torgue
> <[email protected]>; Simon Horman <[email protected]>; Geoff
> Levand <[email protected]>; Wolfram Sang <wsa+renesas@sang-
> engineering.com>; Lobakin, Aleksander <[email protected]>; Praveen
> Kaligineedi <[email protected]>; Willem de Bruijn <[email protected]>;
> Joshua Washington <[email protected]>; Furong Xu <[email protected]>;
> Russell King (Oracle) <[email protected]>; Jisheng Zhang
> <[email protected]>; Petr Tesarik <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; Richard Cochran <[email protected]>;
> Keller,
> Jacob E <[email protected]>; Shannon Nelson
> <[email protected]>; Ziwei Xiao <[email protected]>; Shailend
> Chand <[email protected]>; Choong Yong Liang
> <[email protected]>; Andrew Halaney <[email protected]>;
> [email protected]; [email protected]
> 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 <[email protected]> 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
