* [PATCH net-next v5 1/2] net: usb: lan78xx: do not update statistics after a failed read
2026-11-11 1:34 [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Wei Jie Law
@ 2026-11-11 1:34 ` Wei Jie Law
2026-11-11 1:34 ` [PATCH net-next v5 2/2] net: usb: lan78xx: reject short register and statistics transfers Wei Jie Law
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Wei Jie Law @ 2026-11-11 1:34 UTC (permalink / raw)
To: Thangaraj Samynathan, UNGLinuxDriver, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Woojung Huh, netdev, linux-usb, linux-kernel, Wei Jie Law,
Andrew Lunn
lan78xx_update_stats() reads the hardware counters into an on-stack
struct lan78xx_statstage. If lan78xx_read_stats() fails, only the
rollover check is skipped: the counters are still recomputed from the
uninitialized stack buffer and stored in dev->stats.curr_stat, which is
then reported to user space by ethtool -S.
Keep the previous values when the read fails.
Found by code review with LLM assistance. Tested together with the next
patch, which makes short reads fail: with an emulated device, ethtool -S
then keeps reporting the previous counters.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Wei Jie Law <law.wei.jie@gt.tech.gov.sg>
---
drivers/net/usb/lan78xx.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index d73002b2c21a..76c2d98b49df 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -789,11 +789,13 @@ static void lan78xx_update_stats(struct lan78xx_net *dev)
mutex_lock(&dev->stats.access_lock);
- if (lan78xx_read_stats(dev, &lan78xx_stats) > 0)
+ if (lan78xx_read_stats(dev, &lan78xx_stats) > 0) {
lan78xx_check_stat_rollover(dev, &lan78xx_stats);
- for (i = 0; i < (sizeof(lan78xx_stats) / (sizeof(u32))); i++)
- data[i] = (u64)p[i] + ((u64)count[i] * ((u64)max[i] + 1));
+ for (i = 0; i < (sizeof(lan78xx_stats) / (sizeof(u32))); i++)
+ data[i] = (u64)p[i] +
+ ((u64)count[i] * ((u64)max[i] + 1));
+ }
mutex_unlock(&dev->stats.access_lock);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v5 2/2] net: usb: lan78xx: reject short register and statistics transfers
2026-11-11 1:34 [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Wei Jie Law
2026-11-11 1:34 ` [PATCH net-next v5 1/2] net: usb: lan78xx: do not update statistics after a failed read Wei Jie Law
@ 2026-11-11 1:34 ` Wei Jie Law
2026-11-14 0:32 ` [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Jakub Kicinski
2026-11-14 0:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Wei Jie Law @ 2026-11-11 1:34 UTC (permalink / raw)
To: Thangaraj Samynathan, UNGLinuxDriver, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Woojung Huh, netdev, linux-usb, linux-kernel, Wei Jie Law,
Andrew Lunn
lan78xx_read_reg() and lan78xx_read_stats() read from the device into
a freshly kmalloc()ed buffer, but only treat a negative return value
from usb_control_msg() as an error. A device that answers with fewer
bytes than requested leaves part or all of the buffer uninitialized,
and those bytes are then used as the register value or the counters.
Treat a short transfer as -EIO in both helpers. lan78xx_write_reg()
does not use uninitialized data, since it fills its buffer before the
transfer, but make it fail on a short transfer as well, so that all the
register helpers report a partial transfer as an error.
Found by code review with LLM assistance and reproduced with an
emulated USB device (raw-gadget + dummy_hcd).
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Wei Jie Law <law.wei.jie@gt.tech.gov.sg>
---
Notes:
This patch depends on the previous one. Without it, a failed statistics
read would make lan78xx_update_stats() compute the counters from its
uninitialized on-stack buffer, so the leak would move from the heap to the
stack.
drivers/net/usb/lan78xx.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 76c2d98b49df..4a58febcddcf 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -612,6 +612,9 @@ static int lan78xx_read_reg(struct lan78xx_net *dev, u32 index, u32 *data)
USB_VENDOR_REQUEST_READ_REGISTER,
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
0, index, buf, 4, USB_CTRL_GET_TIMEOUT);
+ if (ret >= 0 && ret != sizeof(*buf))
+ ret = -EIO;
+
if (likely(ret >= 0)) {
le32_to_cpus(buf);
*data = *buf;
@@ -645,6 +648,9 @@ static int lan78xx_write_reg(struct lan78xx_net *dev, u32 index, u32 data)
USB_VENDOR_REQUEST_WRITE_REGISTER,
USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
0, index, buf, 4, USB_CTRL_SET_TIMEOUT);
+ if (ret >= 0 && ret != sizeof(*buf))
+ ret = -EIO;
+
if (unlikely(ret < 0) &&
net_ratelimit()) {
netdev_warn(dev->net,
@@ -695,6 +701,9 @@ static int lan78xx_read_stats(struct lan78xx_net *dev,
(void *)stats,
sizeof(*stats),
USB_CTRL_SET_TIMEOUT);
+ if (ret >= 0 && ret != sizeof(*stats))
+ ret = -EIO;
+
if (likely(ret >= 0)) {
src = (u32 *)stats;
dst = (u32 *)data;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads
2026-11-11 1:34 [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Wei Jie Law
2026-11-11 1:34 ` [PATCH net-next v5 1/2] net: usb: lan78xx: do not update statistics after a failed read Wei Jie Law
2026-11-11 1:34 ` [PATCH net-next v5 2/2] net: usb: lan78xx: reject short register and statistics transfers Wei Jie Law
@ 2026-11-14 0:32 ` Jakub Kicinski
2026-11-14 7:18 ` Wei Jie Law
2026-11-14 0:40 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-11-14 0:32 UTC (permalink / raw)
To: Wei Jie Law
Cc: Thangaraj Samynathan, UNGLinuxDriver, Andrew Lunn,
David S . Miller, Eric Dumazet, Paolo Abeni, Woojung Huh, netdev,
linux-usb, linux-kernel, Wei Jie Law
On Wed, 11 Nov 2026 09:34:52 +0800 Wei Jie Law wrote:
> Testing: the series builds without warnings (W=1) on net-next and passes
> checkpatch --strict. It applies unchanged to v6.12.105, where it was
> tested with a LAN7800 emulated by raw-gadget + dummy_hcd, booted with
> init_on_alloc=0:
Why are you testing this on 6.12 if you're just using a gadget and not
a real device?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads
2026-11-14 0:32 ` [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Jakub Kicinski
@ 2026-11-14 7:18 ` Wei Jie Law
0 siblings, 0 replies; 6+ messages in thread
From: Wei Jie Law @ 2026-11-14 7:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Wei Jie Law, Thangaraj Samynathan, UNGLinuxDriver, Andrew Lunn,
David S . Miller, Eric Dumazet, Paolo Abeni, Woojung Huh, netdev,
linux-usb, linux-kernel
On Sat, Nov 14, 2026 at 8:32 AM Jakub Kicinski <kuba@kernel.org> wrote:
> Why are you testing this on 6.12 if you're just using a gadget and not
> a real device?
I have noted that the patch was applied but to clarify, the reason for
the tests being on a 6.12 kernel is that I am mirroring my Linux laptop's
kernel version as part of bug hunting and testing (against real hardware)
with a malicious USB device.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads
2026-11-11 1:34 [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Wei Jie Law
` (2 preceding siblings ...)
2026-11-14 0:32 ` [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads Jakub Kicinski
@ 2026-11-14 0:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-11-14 0:40 UTC (permalink / raw)
To: Wei Jie Law
Cc: Thangaraj.S, UNGLinuxDriver, andrew+netdev, davem, edumazet, kuba,
pabeni, woojung.huh, netdev, linux-usb, linux-kernel, 98lawweijie
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 11 Nov 2026 09:34:52 +0800 you wrote:
> lan78xx_read_reg() and lan78xx_read_stats() accept a control transfer
> that returns fewer bytes than requested and then use the uninitialized
> rest of their kmalloc()ed buffers. A device that sends short replies can
> get stale heap contents used as the MAC address of the interface, and makes
> ethtool -S report them to unprivileged users, including kernel text and
> direct map pointers.
>
> [...]
Here is the summary with links:
- [net-next,v5,1/2] net: usb: lan78xx: do not update statistics after a failed read
https://git.kernel.org/netdev/net-next/c/faf25c2bc6af
- [net-next,v5,2/2] net: usb: lan78xx: reject short register and statistics transfers
https://git.kernel.org/netdev/net-next/c/ecb8113b5b32
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] 6+ messages in thread