[PATCH iwl-next] ice: Consistently use ethtool_puts() to copy strings

Simon Horman
2024-09-02T12:47:00Z

ethtool_puts() is the preferred method for copying ethtool strings.
And ethtool_puts() is already used to copy ethtool strings in
igc_ethtool_get_strings(). With this patch igc_ethtool_get_strings()
uses it for all such cases.

In general, the compiler can't use fortification to verify that the
destination buffer isn't over-run when the destination is the first
element of an array, and more than one element of the array is to be
written by memcpy().

For the ETH_SS_PRIV_FLAGS the problem doesn't manifest as there is only
one element in the igc_priv_flags_strings array.

In the ETH_SS_TEST case, there is more than one element of
igc_gstrings_test, and from the compiler's perspective, that element is
overrun. In practice it does not overrun the overall size of the array,
but it is nice to use tooling to help us where possible. In this case
the problem is flagged as follows.

Flagged by clang-18 as:

In file included from drivers/net/ethernet/intel/igc/igc_ethtool.c:5:
In file included from ./include/linux/if_vlan.h:10:
In file included from ./include/linux/netdevice.h:24:
In file included from ./include/linux/timer.h:6:
In file included from ./include/linux/ktime.h:25:
In file included from ./include/linux/jiffies.h:10:
In file included from ./include/linux/time.h:60:
In file included from ./include/linux/time32.h:13:
In file included from ./include/linux/timex.h:67:
In file included from ./arch/x86/include/asm/timex.h:5:
In file included from ./arch/x86/include/asm/processor.h:19:
In file included from ./arch/x86/include/asm/cpuid.h:62:
In file included from ./arch/x86/include/asm/paravirt.h:21:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:13:
In file included from ./include/linux/string.h:374:
.../fortify-string.h:580:4: warning: call to '__read_overflow2_field' declared with 'warning' attribute: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]

And Smatch as:

.../igc_ethtool.c:771 igc_ethtool_get_strings() error: __builtin_memcpy() '*igc_gstrings_test' too small (32 vs 160)

Curiously, not flagged by gcc-14.

Compile tested only.

Signed-off-by: Simon Horman <horms@kernel.org>
---
drivers/net/ethernet/intel/igc/igc_ethtool.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 457b5d7f1610..ccace77c6c2d 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -768,8 +768,8 @@ static void igc_ethtool_get_strings(struct net_device *netdev, u32 stringset,

switch (stringset) {
case ETH_SS_TEST:
- memcpy(data, *igc_gstrings_test,
- IGC_TEST_LEN * ETH_GSTRING_LEN);
+ for (i = 0; i < IGC_TEST_LEN; i++)
+ ethtool_puts(&p, igc_gstrings_test[i]);
break;
case ETH_SS_STATS:
for (i = 0; i < IGC_GLOBAL_STATS_LEN; i++)
@@ -791,8 +791,8 @@ static void igc_ethtool_get_strings(struct net_device *netdev, u32 stringset,
/* BUG_ON(p - data != IGC_STATS_LEN * ETH_GSTRING_LEN); */
break;
case ETH_SS_PRIV_FLAGS:
- memcpy(data, igc_priv_flags_strings,
- IGC_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN);
+ for (i = 0; i < IGC_PRIV_FLAGS_STR_LEN; i++)
+ ethtool_puts(&p, igc_priv_flags_strings[i]);
break;
}
}