public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: dsa: mxl862xx: don't read out-of-bounds
@ 2026-03-18  3:07 Daniel Golle
  2026-03-19 16:44 ` Simon Horman
  2026-03-21  1:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Daniel Golle @ 2026-03-18  3:07 UTC (permalink / raw)
  To: Daniel Golle, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel

The write loop in mxl862xx_api_wrap() computes the word count as
(size + 1) / 2, rounding up for odd-sized structs.

On the last iteration of an odd-sized buffer it reads a full __le16
from data[i], accessing one byte past the end of the caller's struct.
KASAN catches this as a stack-out-of-bounds read during probe (e.g.
from mxl862xx_bridge_config_fwd() because of the odd length of
sizeof(struct mxl862xx_bridge_config) == 49).

The read-back loop already handles this case, it writes only a single
byte when (i * 2 + 1) == size. The write loop lacked the same guard.

In practice the over-read is harmless: the extra stack byte is sent to
the firmware which ignores trailing data beyond the command's declared
payload size.

Apply the same odd-size last-byte handling to the write path: when the
final word contains only one valid byte, send *(u8 *)&data[i] instead
of le16_to_cpu(data[i]). This is endian-safe because data is
__le16-encoded and the low byte is always at the lowest address
regardless of host byte order.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/dsa/mxl862xx/mxl862xx-host.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-host.c b/drivers/net/dsa/mxl862xx/mxl862xx-host.c
index 8c55497a0ce89..4eefd2a759a7d 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx-host.c
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-host.c
@@ -175,8 +175,14 @@ int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data,
 				goto out;
 		}
 
-		ret = mxl862xx_reg_write(priv, MXL862XX_MMD_REG_DATA_FIRST + off,
-					 le16_to_cpu(data[i]));
+		if ((i * 2 + 1) == size)
+			ret = mxl862xx_reg_write(priv,
+						 MXL862XX_MMD_REG_DATA_FIRST + off,
+						 *(u8 *)&data[i]);
+		else
+			ret = mxl862xx_reg_write(priv,
+						 MXL862XX_MMD_REG_DATA_FIRST + off,
+						 le16_to_cpu(data[i]));
 		if (ret < 0)
 			goto out;
 	}
-- 
2.53.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: dsa: mxl862xx: don't read out-of-bounds
  2026-03-18  3:07 [PATCH net-next] net: dsa: mxl862xx: don't read out-of-bounds Daniel Golle
@ 2026-03-19 16:44 ` Simon Horman
  2026-03-21  1:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2026-03-19 16:44 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-kernel

On Wed, Mar 18, 2026 at 03:07:52AM +0000, Daniel Golle wrote:
> The write loop in mxl862xx_api_wrap() computes the word count as
> (size + 1) / 2, rounding up for odd-sized structs.
> 
> On the last iteration of an odd-sized buffer it reads a full __le16
> from data[i], accessing one byte past the end of the caller's struct.
> KASAN catches this as a stack-out-of-bounds read during probe (e.g.
> from mxl862xx_bridge_config_fwd() because of the odd length of
> sizeof(struct mxl862xx_bridge_config) == 49).
> 
> The read-back loop already handles this case, it writes only a single
> byte when (i * 2 + 1) == size. The write loop lacked the same guard.
> 
> In practice the over-read is harmless: the extra stack byte is sent to
> the firmware which ignores trailing data beyond the command's declared
> payload size.
> 
> Apply the same odd-size last-byte handling to the write path: when the
> final word contains only one valid byte, send *(u8 *)&data[i] instead
> of le16_to_cpu(data[i]). This is endian-safe because data is
> __le16-encoded and the low byte is always at the lowest address
> regardless of host byte order.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: dsa: mxl862xx: don't read out-of-bounds
  2026-03-18  3:07 [PATCH net-next] net: dsa: mxl862xx: don't read out-of-bounds Daniel Golle
  2026-03-19 16:44 ` Simon Horman
@ 2026-03-21  1:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-03-21  1:40 UTC (permalink / raw)
  To: Daniel Golle
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel

Hello:

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

On Wed, 18 Mar 2026 03:07:52 +0000 you wrote:
> The write loop in mxl862xx_api_wrap() computes the word count as
> (size + 1) / 2, rounding up for odd-sized structs.
> 
> On the last iteration of an odd-sized buffer it reads a full __le16
> from data[i], accessing one byte past the end of the caller's struct.
> KASAN catches this as a stack-out-of-bounds read during probe (e.g.
> from mxl862xx_bridge_config_fwd() because of the odd length of
> sizeof(struct mxl862xx_bridge_config) == 49).
> 
> [...]

Here is the summary with links:
  - [net-next] net: dsa: mxl862xx: don't read out-of-bounds
    https://git.kernel.org/netdev/net-next/c/6b5f49176a08

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-21  1:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-18  3:07 [PATCH net-next] net: dsa: mxl862xx: don't read out-of-bounds Daniel Golle
2026-03-19 16:44 ` Simon Horman
2026-03-21  1:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox