Message-ID: <CO1PR11MB508977671F23B9E45863417DD6BD2@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Wed, 16 Apr 2025 18:57:35 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: Kees Cook <kees@...nel.org>, Kory Maincent <kory.maincent@...tlin.com>
CC: Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>, "Dumazet, Eric"
	<edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Claudiu Manoil
	<claudiu.manoil@....com>, Vladimir Oltean <vladimir.oltean@....com>, Wei Fang
	<wei.fang@....com>, Clark Wang <xiaoning.wang@....com>, Jeroen de Borst
	<jeroendb@...gle.com>, Harshitha Ramamurthy <hramamurthy@...gle.com>, "Ido
 Schimmel" <idosch@...dia.com>, Petr Machata <petrm@...dia.com>, "Maxime
 Coquelin" <mcoquelin.stm32@...il.com>, Alexandre Torgue
	<alexandre.torgue@...s.st.com>, Simon Horman <horms@...nel.org>, Geoff Levand
	<geoff@...radead.org>, Wolfram Sang <wsa+renesas@...g-engineering.com>,
	"Lobakin, Aleksander" <aleksander.lobakin@...el.com>, Praveen Kaligineedi
	<pkaligineedi@...gle.com>, Willem de Bruijn <willemb@...gle.com>, "Joshua
 Washington" <joshwash@...gle.com>, Furong Xu <0x1207@...il.com>, "Russell
 King (Oracle)" <rmk+kernel@...linux.org.uk>, Jisheng Zhang
	<jszhang@...nel.org>, Petr Tesarik <petr@...arici.cz>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "imx@...ts.linux.dev"
	<imx@...ts.linux.dev>, "linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, Richard Cochran
	<richardcochran@...il.com>, Shannon Nelson <shannon.nelson@....com>, "Ziwei
 Xiao" <ziweixiao@...gle.com>, Shailend Chand <shailend@...gle.com>, "Choong
 Yong Liang" <yong.liang.choong@...ux.intel.com>, Andrew Halaney
	<ahalaney@...hat.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>
Subject: RE: [PATCH] net: ethtool: Adjust exactly ETH_GSTRING_LEN-long stats
 to use memcpy



> -----Original Message-----
> From: Kees Cook <kees@...nel.org>
> Sent: Wednesday, April 16, 2025 10:49 AM
> To: Kory Maincent <kory.maincent@...tlin.com>
> Cc: Jakub Kicinski <kuba@...nel.org>; Andrew Lunn <andrew+netdev@...n.ch>;
> David S. Miller <davem@...emloft.net>; Dumazet, Eric <edumazet@...gle.com>;
> Paolo Abeni <pabeni@...hat.com>; Claudiu Manoil <claudiu.manoil@....com>;
> Vladimir Oltean <vladimir.oltean@....com>; Wei Fang <wei.fang@....com>; Clark
> Wang <xiaoning.wang@....com>; Jeroen de Borst <jeroendb@...gle.com>;
> Harshitha Ramamurthy <hramamurthy@...gle.com>; Ido Schimmel
> <idosch@...dia.com>; Petr Machata <petrm@...dia.com>; Maxime Coquelin
> <mcoquelin.stm32@...il.com>; Alexandre Torgue
> <alexandre.torgue@...s.st.com>; Simon Horman <horms@...nel.org>; Geoff
> Levand <geoff@...radead.org>; Wolfram Sang <wsa+renesas@...g-
> engineering.com>; Lobakin, Aleksander <aleksander.lobakin@...el.com>; Praveen
> Kaligineedi <pkaligineedi@...gle.com>; Willem de Bruijn <willemb@...gle.com>;
> Joshua Washington <joshwash@...gle.com>; Furong Xu <0x1207@...il.com>;
> Russell King (Oracle) <rmk+kernel@...linux.org.uk>; Jisheng Zhang
> <jszhang@...nel.org>; Petr Tesarik <petr@...arici.cz>; netdev@...r.kernel.org;
> imx@...ts.linux.dev; linux-stm32@...md-mailman.stormreply.com; linux-arm-
> kernel@...ts.infradead.org; Richard Cochran <richardcochran@...il.com>; Keller,
> Jacob E <jacob.e.keller@...el.com>; Shannon Nelson
> <shannon.nelson@....com>; Ziwei Xiao <ziweixiao@...gle.com>; Shailend
> Chand <shailend@...gle.com>; Choong Yong Liang
> <yong.liang.choong@...ux.intel.com>; Andrew Halaney <ahalaney@...hat.com>;
> linux-kernel@...r.kernel.org; linux-hardening@...r.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@...nel.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

Powered by blists - more mailing lists