Skip to contentSkip to conversation

net/sun3_82586: Avoid reading past buffer in debug output

PATCH Applied Kees Cook 4 participants 4 messages Feb 6, 2024 – Feb 9, 2024

Subsystems (3)
Networking Network drivers SUN3/3X
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 net/sun3_82586: Avoid reading past buffer in debug output

Since NUM_XMIT_BUFFS is always 1, building m68k with sun3_defconfig and -Warraybounds, this build warning is visible[1]:

drivers/net/ethernet/i825xx/sun3_82586.c: In function 'sun3_82586_timeout':
drivers/net/ethernet/i825xx/sun3_82586.c:990:122: warning: array subscript 1 is above array bounds of 'volatile struct transmit_cmd_struct *[1]' [-Warray-bounds=]
990 | printk("%s: command-stats: %04x %04x\n",dev->name,swab16(p->xmit_cmds[0]->cmd_status),swab16(p->xmit_cmds[1]->cmd_status));
| ~~~~~~~~~~~~^~~
...
drivers/net/ethernet/i825xx/sun3_82586.c:156:46: note: while referencing 'xmit_cmds'
156 | volatile struct transmit_cmd_struct *xmit_cmds[NUM_XMIT_BUFFS];

Avoid accessing index 1 since it doesn't exist.

Link https://github.com/KSPP/linux/issues/325 [1]
Cc Sam Creasey <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc Gustavo A. R. Silva <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
 drivers/net/ethernet/i825xx/sun3_82586.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/i825xx/sun3_82586.c +1 -1
index 5e27470c6b1e..f2d4669c81cf 100644
@@ -987,7 +987,7 @@ static void sun3_82586_timeout(struct net_device *dev, unsigned int txqueue)
 	{
 #ifdef DEBUG
 		printk("%s: xmitter timed out, try to restart! stat: %02x\n",dev->name,p->scb->cus);
-		printk("%s: command-stats: %04x %04x\n",dev->name,swab16(p->xmit_cmds[0]->cmd_status),swab16(p->xmit_cmds[1]->cmd_status));
+		printk("%s: command-stats: %04x\n", dev->name, swab16(p->xmit_cmds[0]->cmd_status));
 		printk("%s: check, whether you set the right interrupt number!\n",dev->name);
 #endif
 		sun3_82586_close(dev);
Signature · 1 line
2.34.1
reviewer Reviewed-by Tested-by on v1 1/1
#1 lore raw
Quoted 13 lines · Kees Cook

Since NUM_XMIT_BUFFS is always 1, building m68k with sun3_defconfig and -Warraybounds, this build warning is visible[1]:

drivers/net/ethernet/i825xx/sun3_82586.c: In function 'sun3_82586_timeout':
drivers/net/ethernet/i825xx/sun3_82586.c:990:122: warning: array subscript 1 is above array bounds of 'volatile struct transmit_cmd_struct *[1]' [-Warray-bounds=]
990 | printk("%s: command-stats: %04x %04x\n",dev->name,swab16(p->xmit_cmds[0]->cmd_status),swab16(p->xmit_cmds[1]->cmd_status));
| ~~~~~~~~~~~~^~~
...
drivers/net/ethernet/i825xx/sun3_82586.c:156:46: note: while referencing 'xmit_cmds'
156 | volatile struct transmit_cmd_struct *xmit_cmds[NUM_XMIT_BUFFS];

Avoid accessing index 1 since it doesn't exist.

Quoted 9 lines
Link https://github.com/KSPP/linux/issues/325 [1]
Cc Sam Creasey <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc Gustavo A. R. Silva <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
Reviewed-by Simon Horman <[email protected]>
Tested-by Simon Horman <[email protected]> # build-tested

...

Reviewed-by on v1 1/1
#2 lore raw
In reply to Kees Cook
Quoted 13 lines · Kees Cook

Since NUM_XMIT_BUFFS is always 1, building m68k with sun3_defconfig and -Warraybounds, this build warning is visible[1]:

drivers/net/ethernet/i825xx/sun3_82586.c: In function 'sun3_82586_timeout':
drivers/net/ethernet/i825xx/sun3_82586.c:990:122: warning: array subscript 1 is above array bounds of 'volatile struct transmit_cmd_struct *[1]' [-Warray-bounds=]
990 | printk("%s: command-stats: %04x %04x\n",dev->name,swab16(p->xmit_cmds[0]->cmd_status),swab16(p->xmit_cmds[1]->cmd_status));
| ~~~~~~~~~~~~^~~
...
drivers/net/ethernet/i825xx/sun3_82586.c:156:46: note: while referencing 'xmit_cmds'
156 | volatile struct transmit_cmd_struct *xmit_cmds[NUM_XMIT_BUFFS];

Avoid accessing index 1 since it doesn't exist.

Quoted 9 lines
Link https://github.com/KSPP/linux/issues/325 [1]
Cc Sam Creasey <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc Gustavo A. R. Silva <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
Reviewed-by Gustavo A. R. Silva <[email protected]>

Thanks!

Signature · 1 line
Gustavo
Quoted 17 lines
---
  drivers/net/ethernet/i825xx/sun3_82586.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/i825xx/sun3_82586.c +1 -1
index 5e27470c6b1e..f2d4669c81cf 100644
@@ -987,7 +987,7 @@ static void sun3_82586_timeout(struct net_device *dev, unsigned int txqueue)
  	{
  #ifdef DEBUG
  		printk("%s: xmitter timed out, try to restart! stat: %02x\n",dev->name,p->scb->cus);
-		printk("%s: command-stats: %04x %04x\n",dev->name,swab16(p->xmit_cmds[0]->cmd_status),swab16(p->xmit_cmds[1]->cmd_status));
+		printk("%s: command-stats: %04x\n", dev->name, swab16(p->xmit_cmds[0]->cmd_status));
  		printk("%s: check, whether you set the right interrupt number!\n",dev->name);
  #endif
  		sun3_82586_close(dev);
CI Applied
#3 lore raw
In reply to Kees Cook
This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 11 lines · On Tue, 6 Feb 2024 08:16:54 -0800 you wrote:

Since NUM_XMIT_BUFFS is always 1, building m68k with sun3_defconfig and -Warraybounds, this build warning is visible[1]:

drivers/net/ethernet/i825xx/sun3_82586.c: In function 'sun3_82586_timeout':
drivers/net/ethernet/i825xx/sun3_82586.c:990:122: warning: array subscript 1 is above array bounds of 'volatile struct transmit_cmd_struct *[1]' [-Warray-bounds=]
990 | printk("%s: command-stats: %04x %04x\n",dev->name,swab16(p->xmit_cmds[0]->cmd_status),swab16(p->xmit_cmds[1]->cmd_status));
| ~~~~~~~~~~~~^~~
...
drivers/net/ethernet/i825xx/sun3_82586.c:156:46: note: while referencing 'xmit_cmds'
156 | volatile struct transmit_cmd_struct *xmit_cmds[NUM_XMIT_BUFFS];

Quoted 1 line

[...]

Here is the summary with links:
- net/sun3_82586: Avoid reading past buffer in debug output

    https://git.kernel.org/netdev/net-next/c/4bea747f3fbe

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html