[PATCH net] net: sundance: fix sign extension of the octet counters

[PATCH net] net: sundance: fix sign extension of the octet counters

[Rand Deeb]

get_stats() adds the upper halves of the 32-bit TxOctets and RxOctets
hardware counters to the byte statistics with

	dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;

On architectures that use the ioread16() from asm-generic/io.h, which
returns u16, the value is promoted to int before the shift. When the
high word is 0x8000 or more, the result does not fit in a positive int
and ends up negative, and adding it to the unsigned long counter
sign-extends it. On 64-bit kernels tx_bytes and rx_bytes then end up
2^32 lower than they should be. x86 and m68k are not affected, because
their ioread16() comes from asm-generic/iomap.h and returns unsigned int.

Cast the high word to u32 before shifting, so the result is the same on
all architectures.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Rand Deeb <rand.sec96@gmail.com>
---
 drivers/net/ethernet/dlink/sundance.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/dlink/sundance.c b/drivers/net/ethernet/dlink/sundance.c
index 58873fcd3..cbb1f1fe1 100644
--- a/drivers/net/ethernet/dlink/sundance.c
+++ b/drivers/net/ethernet/dlink/sundance.c
@@ -1545,9 +1545,9 @@ static struct net_device_stats *get_stats(struct net_device *dev)
 	np->xstats.rx_mcasts += ioread8(ioaddr + StatsMcastRx);
 
 	dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsLow);
-	dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;
+	dev->stats.tx_bytes += (u32)ioread16(ioaddr + TxOctetsHigh) << 16;
 	dev->stats.rx_bytes += ioread16(ioaddr + RxOctetsLow);
-	dev->stats.rx_bytes += ioread16(ioaddr + RxOctetsHigh) << 16;
+	dev->stats.rx_bytes += (u32)ioread16(ioaddr + RxOctetsHigh) << 16;
 
 	spin_unlock_irqrestore(&np->statlock, flags);
 

base-commit: 4cbdd58dfa7aaad679bf62f81cdbc96a6cb7c6ae
-- 
2.39.5

Re: [PATCH net] net: sundance: fix sign extension of the octet counters

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: sundance: fix sign extension of the octet counters

[Jakub Kicinski]

On Wed, 21 Oct 2026 02:44:34 +0300 Rand Deeb wrote:
> get_stats() adds the upper halves of the 32-bit TxOctets and RxOctets
> hardware counters to the byte statistics with
> 
> 	dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;
> 
> On architectures that use the ioread16() from asm-generic/io.h, which
> returns u16, the value is promoted to int before the shift. When the
> high word is 0x8000 or more, the result does not fit in a positive int
> and ends up negative, and adding it to the unsigned long counter
> sign-extends it. On 64-bit kernels tx_bytes and rx_bytes then end up
> 2^32 lower than they should be. x86 and m68k are not affected, because
> their ioread16() comes from asm-generic/iomap.h and returns unsigned int.
> 
> Cast the high word to u32 before shifting, so the result is the same on
> all architectures.

We don't take LLM slop fixes for ancient hw
-- 
pw-bot: reject

Re: [PATCH net] net: sundance: fix sign extension of the octet counters

[Rand Deeb]

On Wed, Oct 21, 2026 at 4:11 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> We don't take LLM slop fixes for ancient hw

The change itself is a simple cast, so I was surprised by the "LLM
slop" remark. The same kind of fix was applied in 2776d2320ac1
(net: dsa: fix unintended sign extension on a u16 left shift), which
you committed in 2020. If it was the commit message that gave that
impression, I can make it shorter next time.

The issue was reported by SVACE static analysis, on architectures where
ioread16() returns u16, the high word is promoted to int and sign
extended when it is added to the unsigned long counter, so
tx_bytes, rx_bytes end up wrong on 64-bit.

The issue is theoretical and the change was not tested on real
hardware, as I don't have the hardware.
If you don't want changes to this driver without hardware
testing, that's your call. The issue is real and the fix is correct,
it is not security related, but it is still a bug on those
architectures.

Regards,
Rand