[PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads

[PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads

[Wei Jie Law]

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.

Patch 2 makes the register and statistics helpers fail on short transfers. Patch 1 comes first so
that a failing statistics read does not report uninitialized stack data
instead.

The bugs need a malicious or emulated USB device, which
Documentation/process/threat-model.rst treats as a regular bug rather
than a vulnerability, so they are posted publicly.

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:

- Zero-length statistics replies. Before: 40 to 88 of the 235 ethtool -S
  counters are non-zero, including usb_api_blocking_completion() and
  direct map pointers. After: every read fails with -EIO and all
  counters stay zero.

- 100 byte statistics replies. Before: 110 of the 235 counters are
  non-zero. With patch 1 only: 29 are, so the stale tail of the buffer
  still gets through. With both patches: the read fails with -EIO and
  all counters stay zero.

- Zero-length RX_ADDRL/RX_ADDRH replies. Before: the MAC address
  differs on every probe, is programmed into the MAC filter and is
  used as the address of the interface. After: the
  probe fails with -EIO.

Both bugs were found, and the patches written, with LLM assistance.
v1 to v4 carried Assisted-by tags; v5 drops them following commit
1e035297ffda ("docs: remove Assisted-by references").

Changes in v5:
- retarget to net-next and drop the Fixes: and Cc: stable tags, since
  the bugs need a misbehaving device (Andrew Lunn)
- add Andrew's Reviewed-by
- drop the Assisted-by tags (see above) and rebase on current net-next
v4: https://lore.kernel.org/all/20261020001621.2920-1-law.wei.jie@gt.tech.gov.sg/
Changes in v4:
- 2/2: drop the examples from the commit message and move the note that
  it depends on 1/2 below the --- line (Andrew Lunn)
- 2/2: also fail on a short transfer in lan78xx_write_reg(), for
  symmetry with the read helpers (Andrew Lunn)
v3: https://lore.kernel.org/all/20261018062419.21921-1-law.wei.jie@gt.tech.gov.sg/
- add the missing Signed-off-by; no other changes
v2: https://lore.kernel.org/all/20261018061734.19488-1-law.wei.jie@gt.tech.gov.sg/
- 2/2: the commit message now says that with patch 1 only a partial
  statistics reply (1 to 187 bytes) still leaks, and that patch 2 depends on
  patch 1; the MAC address paragraph no longer claims a write-back to
  RX_ADDRL/RX_ADDRH, which happens only for an invalid address. The cover
  letter gains a partial-reply test. No code change.
v1: https://lore.kernel.org/all/20261014025602.26334-1-law.wei.jie@gt.tech.gov.sg/

Wei Jie Law (2):
  net: usb: lan78xx: do not update statistics after a failed read
  net: usb: lan78xx: reject short register and statistics transfers

 drivers/net/usb/lan78xx.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)


base-commit: 099e433a8bf4914a727b46e0c8a1a3d96c94b6a2
-- 
2.43.0

[PATCH net-next v5 1/2] net: usb: lan78xx: do not update statistics after a failed read

[Wei Jie Law]

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

[PATCH net-next v5 2/2] net: usb: lan78xx: reject short register and statistics transfers

[Wei Jie Law]

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

Re: [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads

[Jakub Kicinski]

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?

Re: [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads

[patchwork-bot+netdevbpf]

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

Re: [PATCH net-next v5 0/2] net: usb: lan78xx: do not use uninitialized data after short reads

[Wei Jie Law]

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.

Re: [PATCH net-next v5 1/2] net: usb: lan78xx: do not update statistics after a failed read

[David Laight]

On Wed, 11 Nov 2026 09:34:53 +0800
Wei Jie Law <law.wei.jie@gt.tech.gov.sg> wrote:

> 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));

You only need to promote one of the values to u64.
Even just changing the 1 to 1uLL will do.

That made be look at the related code.

This function has a large on-stack buffer (about 800 bytes from a quick
count). But lan78xx_read_stats() (now) calls kmalloc() - probably because
it cant use an on-stack buffer for the usb transfer.
If you need to call kmalloc() it might as well be for the larger structure.

There are a lot of stats, and two structures (one u32 and on u64) that have
to match, not only that but a lot (but not all) the code ends up treating
the structures as arrays.
It is probably worth just changing them to arrays and using an enum for
the indexes.

Oh - glad to see you fixed (finally) the massive 'truesize' bug.
When I was testing this driver (may years ago, and the product that would
have used it got cancelled) I found a bug with one of the usb3 host
controllers that it only processed one request from one of the control
rings for each 'doorbell' write. The setup code could send them too fast
and it all got very confusing - don't know if that got fixed.
I've actually got the host that failed, but I don't think I've got the
usb ethernet device.

David



> +	}
>  
>  	mutex_unlock(&dev->stats.access_lock);
>