> -----Original Message-----
> From: Kees Cook <k...@kernel.org>
> Sent: Wednesday, April 16, 2025 10:49 AM
> To: Kory Maincent <kory.mainc...@bootlin.com>
> Cc: Jakub Kicinski <k...@kernel.org>; Andrew Lunn <andrew+net...@lunn.ch>;
> David S. Miller <da...@davemloft.net>; Dumazet, Eric <eduma...@google.com>;
> Paolo Abeni <pab...@redhat.com>; Claudiu Manoil <claudiu.man...@nxp.com>;
> Vladimir Oltean <vladimir.olt...@nxp.com>; Wei Fang <wei.f...@nxp.com>; Clark
> Wang <xiaoning.w...@nxp.com>; Jeroen de Borst <jeroe...@google.com>;
> Harshitha Ramamurthy <hramamur...@google.com>; Ido Schimmel
> <ido...@nvidia.com>; Petr Machata <pe...@nvidia.com>; Maxime Coquelin
> <mcoquelin.st...@gmail.com>; Alexandre Torgue
> <alexandre.tor...@foss.st.com>; Simon Horman <ho...@kernel.org>; Geoff
> Levand <ge...@infradead.org>; Wolfram Sang <wsa+renesas@sang-
> engineering.com>; Lobakin, Aleksander <aleksander.loba...@intel.com>; Praveen
> Kaligineedi <pkaligine...@google.com>; Willem de Bruijn <will...@google.com>;
> Joshua Washington <joshw...@google.com>; Furong Xu <0x1...@gmail.com>;
> Russell King (Oracle) <rmk+ker...@armlinux.org.uk>; Jisheng Zhang
> <jszh...@kernel.org>; Petr Tesarik <p...@tesarici.cz>; net...@vger.kernel.org;
> i...@lists.linux.dev; linux-st...@st-md-mailman.stormreply.com; linux-arm-
> ker...@lists.infradead.org; Richard Cochran <richardcoch...@gmail.com>;
> Keller,
> Jacob E <jacob.e.kel...@intel.com>; Shannon Nelson
> <shannon.nel...@amd.com>; Ziwei Xiao <ziweix...@google.com>; Shailend
> Chand <shail...@google.com>; Choong Yong Liang
> <yong.liang.cho...@linux.intel.com>; Andrew Halaney <ahala...@redhat.com>;
> linux-ker...@vger.kernel.org; linux-hardening@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 <k...@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