netdev - Re: [PATCH] net: ethtool: Adjust exactly ETH_GSTRING

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ecxswjs4.fsf@nvidia.com>
Date: Wed, 16 Apr 2025 11:09:43 +0200
From: Petr Machata <petrm@...dia.com>
To: Kees Cook <kees@...nel.org>
CC: Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <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>, Alexander Lobakin
	<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>, Jacob Keller <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>,
	Kory Maincent <kory.maincent@...tlin.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


Kees Cook <kees@...nel.org> writes:

> 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.

> Specifically the following warnings were investigated and addressed:
>
> ../drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c:328:24: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (33 chars into 32 available) [-Wunterminated-string-initialization]
>   328 |                 .str = "a_mac_control_frames_transmitted",
>       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c:340:24: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (33 chars into 32 available) [-Wunterminated-string-initialization]
>   340 |                 .str = "a_pause_mac_ctrl_frames_received",
>       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
> index 3f64cdbabfa3..0a8fb9c842d3 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
> @@ -262,7 +262,7 @@ static int mlxsw_sp_port_set_pauseparam(struct net_device *dev,
>  }
>  
>  struct mlxsw_sp_port_hw_stats {
> -	char str[ETH_GSTRING_LEN];
> +	char str[ETH_GSTRING_LEN] __nonstring;
>  	u64 (*getter)(const char *payload);
>  	bool cells_bytes;
>  };

Reviewed-by: Petr Machata <petrm@...dia.com> # for mlxsw