[PATCH] net: b44: use ethtool_puts

[PATCH] net: b44: use ethtool_puts

[Rosen Penev]

There's a subtle error with the memcpy here, where b44_gstrings should
not be dereferenced. Dereferening causes the following error with W=1:

In file included from drivers/net/ethernet/broadcom/b44.c:17:
In file included from ./include/linux/module.h:18:
In file included from ./include/linux/kmod.h:9:
In file included from ./include/linux/umh.h:4:
In file included from ./include/linux/gfp.h:7:
In file included from ./include/linux/mmzone.h:8:
In file included from ./include/linux/spinlock.h:56:
In file included from ./include/linux/preempt.h:79:
In file included from ./arch/powerpc/include/asm/preempt.h:5:
In file included from ./include/asm-generic/preempt.h:5:
In file included from ./include/linux/thread_info.h:23:
In file included from ./arch/powerpc/include/asm/current.h:13:
In file included from ./arch/powerpc/include/asm/paca.h:16:
In file included from ./include/linux/string.h:386:
./include/linux/fortify-string.h:578:4: error: call to
'__read_overflow2_field' declared with 'warning' attribute: detected read
beyond size of field (2nd parameter); maybe use>
  578 |                        __read_overflow2_field(q_size_field, size);
      |                         ^

Instead of fixing the memcpy, use ethtool_puts, which is the proper
helper for printing ethtool gstrings.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/broadcom/b44.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 90df02e0039c..f994636fbd5f 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2031,11 +2031,11 @@ static int b44_set_pauseparam(struct net_device *dev,
 
 static void b44_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 {
-	switch(stringset) {
-	case ETH_SS_STATS:
-		memcpy(data, *b44_gstrings, sizeof(b44_gstrings));
-		break;
-	}
+	if (stringset != ETH_SS_STATS)
+		return;
+
+	for (int i = 0; i < ARRAY_SIZE(b44_gstrings); i++)
+		ethtool_puts(&data, b44_gstrings[i]);
 }
 
 static int b44_get_sset_count(struct net_device *dev, int sset)
-- 
2.54.0

Re: [PATCH] net: b44: use ethtool_puts

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 30 May 2026 17:03:34 -0700 you wrote:
> There's a subtle error with the memcpy here, where b44_gstrings should
> not be dereferenced. Dereferening causes the following error with W=1:
> 
> In file included from drivers/net/ethernet/broadcom/b44.c:17:
> In file included from ./include/linux/module.h:18:
> In file included from ./include/linux/kmod.h:9:
> In file included from ./include/linux/umh.h:4:
> In file included from ./include/linux/gfp.h:7:
> In file included from ./include/linux/mmzone.h:8:
> In file included from ./include/linux/spinlock.h:56:
> In file included from ./include/linux/preempt.h:79:
> In file included from ./arch/powerpc/include/asm/preempt.h:5:
> In file included from ./include/asm-generic/preempt.h:5:
> In file included from ./include/linux/thread_info.h:23:
> In file included from ./arch/powerpc/include/asm/current.h:13:
> In file included from ./arch/powerpc/include/asm/paca.h:16:
> In file included from ./include/linux/string.h:386:
> ./include/linux/fortify-string.h:578:4: error: call to
> '__read_overflow2_field' declared with 'warning' attribute: detected read
> beyond size of field (2nd parameter); maybe use>
>   578 |                        __read_overflow2_field(q_size_field, size);
>       |                         ^
> 
> [...]

Here is the summary with links:
  - net: b44: use ethtool_puts
    https://git.kernel.org/netdev/net-next/c/a87533a80831

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] net: b44: use ethtool_puts

[David Laight]

On Sat, 30 May 2026 17:03:34 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> There's a subtle error with the memcpy here, where b44_gstrings should
> not be dereferenced. Dereferening causes the following error with W=1:

The definition is:
static const char b44_gstrings[][ETH_GSTRING_LEN] = ...

There is nothing wrong with using memcpy().

> 
> In file included from drivers/net/ethernet/broadcom/b44.c:17:
> In file included from ./include/linux/module.h:18:
> In file included from ./include/linux/kmod.h:9:
> In file included from ./include/linux/umh.h:4:
> In file included from ./include/linux/gfp.h:7:
> In file included from ./include/linux/mmzone.h:8:
> In file included from ./include/linux/spinlock.h:56:
> In file included from ./include/linux/preempt.h:79:
> In file included from ./arch/powerpc/include/asm/preempt.h:5:
> In file included from ./include/asm-generic/preempt.h:5:
> In file included from ./include/linux/thread_info.h:23:
> In file included from ./arch/powerpc/include/asm/current.h:13:
> In file included from ./arch/powerpc/include/asm/paca.h:16:
> In file included from ./include/linux/string.h:386:
> ./include/linux/fortify-string.h:578:4: error: call to
> '__read_overflow2_field' declared with 'warning' attribute: detected read
> beyond size of field (2nd parameter); maybe use>
>   578 |                        __read_overflow2_field(q_size_field, size);
>       |                         ^
> 
> Instead of fixing the memcpy, use ethtool_puts, which is the proper
> helper for printing ethtool gstrings.

ethtool_puts() lets you pass short strings as well as ones that don't
contain a '\0' terminator.
It isn't needed here because all the strings are already padded.

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/b44.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
> index 90df02e0039c..f994636fbd5f 100644
> --- a/drivers/net/ethernet/broadcom/b44.c
> +++ b/drivers/net/ethernet/broadcom/b44.c
> @@ -2031,11 +2031,11 @@ static int b44_set_pauseparam(struct net_device *dev,
>  
>  static void b44_get_strings(struct net_device *dev, u32 stringset, u8 *data)
>  {
> -	switch(stringset) {
> -	case ETH_SS_STATS:
> -		memcpy(data, *b44_gstrings, sizeof(b44_gstrings));

Just remove the * or replace with an & - either is valid.
The existing code is reading b44_gstrings[0] so overruns into the following
strings.

-- David

> -		break;
> -	}
> +	if (stringset != ETH_SS_STATS)
> +		return;
> +
> +	for (int i = 0; i < ARRAY_SIZE(b44_gstrings); i++)
> +		ethtool_puts(&data, b44_gstrings[i]);
>  }
>  
>  static int b44_get_sset_count(struct net_device *dev, int sset)