[PATCH v1 net] net: usb: rtl8150: Fix uninit-value access in set_carrier().

[PATCH v1 net] net: usb: rtl8150: Fix uninit-value access in set_carrier().

[Kuniyuki Iwashima]

syzbot reported set_carrier() accesses an uninitialised local var. [0]

get_registers() is a wrapper of usb_control_msg_recv(), which copies
data to the passed buffer only when it returns 0.

Let's check the retval before accessing tmp in set_carrier().

[0]:
BUG: KMSAN: uninit-value in set_carrier drivers/net/usb/rtl8150.c:721 [inline]
BUG: KMSAN: uninit-value in rtl8150_open+0x1131/0x1360 drivers/net/usb/rtl8150.c:758
 set_carrier drivers/net/usb/rtl8150.c:721 [inline]
 rtl8150_open+0x1131/0x1360 drivers/net/usb/rtl8150.c:758
 __dev_open+0x7e9/0xc60 net/core/dev.c:1682
 __dev_change_flags+0x3a8/0x9f0 net/core/dev.c:9549
 netif_change_flags+0x8d/0x1e0 net/core/dev.c:9612
 dev_change_flags+0x18c/0x320 net/core/dev_api.c:68
 devinet_ioctl+0x1186/0x2500 net/ipv4/devinet.c:1200
 inet_ioctl+0x4c0/0x6f0 net/ipv4/af_inet.c:1001
 sock_do_ioctl+0x9c/0x480 net/socket.c:1238
 sock_ioctl+0x70b/0xd60 net/socket.c:1359
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:598 [inline]
 __se_sys_ioctl+0x23c/0x400 fs/ioctl.c:584
 __x64_sys_ioctl+0x97/0xe0 fs/ioctl.c:584
 x64_sys_call+0x1cbc/0x3e20 arch/x86/include/generated/asm/syscalls_64.h:17
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xd9/0x210 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Local variable tmp created at:
 number+0x8a/0x2200 lib/vsprintf.c:469
 vsnprintf+0xd21/0x1bd0 lib/vsprintf.c:2890

CPU: 1 UID: 0 PID: 5461 Comm: dhcpcd Not tainted syzkaller #0 PREEMPT(none)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2025

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+b4d5d8faea6996fd55e3@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/68af933a.a00a0220.2929dc.0007.GAE@google.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/usb/rtl8150.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index ddff6f19ff98..0f45627f6a00 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -717,8 +717,8 @@ static void set_carrier(struct net_device *netdev)
 	rtl8150_t *dev = netdev_priv(netdev);
 	short tmp;
 
-	get_registers(dev, CSCR, 2, &tmp);
-	if (tmp & CSCR_LINK_STATUS)
+	if (!get_registers(dev, CSCR, 2, &tmp) &&
+	    (tmp & CSCR_LINK_STATUS))
 		netif_carrier_on(netdev);
 	else
 		netif_carrier_off(netdev);
-- 
2.51.0.268.g9569e192d0-goog

Re: [PATCH v1 net] net: usb: rtl8150: Fix uninit-value access in set_carrier().

[Andrew Lunn]

On Wed, Aug 27, 2025 at 11:31:02PM +0000, Kuniyuki Iwashima wrote:
> syzbot reported set_carrier() accesses an uninitialised local var. [0]
> 
> get_registers() is a wrapper of usb_control_msg_recv(), which copies
> data to the passed buffer only when it returns 0.
> 
> Let's check the retval before accessing tmp in set_carrier().

	do {
		get_registers(dev, PHYCNT, 1, data);
	} while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));

	if (i <= MII_TIMEOUT) {
		get_registers(dev, PHYDAT, 2, data);
		*reg = data[0] | (data[1] << 8);



	/* Get the CR contents. */
	get_registers(dev, CR, 1, &cr);
	/* Set the WEPROM bit (eeprom write enable). */
	cr |= 0x20;
	set_registers(dev, CR, 1, &cr);


	do {
		get_registers(dev, CR, 1, &data);
	} while ((data & 0x10) && --i);

Don't these also have the same problem?

    Andrew

---
pw-bot: cr

Re: [PATCH v1 net] net: usb: rtl8150: Fix uninit-value access in set_carrier().

[Kuniyuki Iwashima]

On Wed, Aug 27, 2025 at 5:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Aug 27, 2025 at 11:31:02PM +0000, Kuniyuki Iwashima wrote:
> > syzbot reported set_carrier() accesses an uninitialised local var. [0]
> >
> > get_registers() is a wrapper of usb_control_msg_recv(), which copies
> > data to the passed buffer only when it returns 0.
> >
> > Let's check the retval before accessing tmp in set_carrier().
>
>         do {
>                 get_registers(dev, PHYCNT, 1, data);
>         } while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));
>
>         if (i <= MII_TIMEOUT) {
>                 get_registers(dev, PHYDAT, 2, data);
>                 *reg = data[0] | (data[1] << 8);
>
>
>
>         /* Get the CR contents. */
>         get_registers(dev, CR, 1, &cr);
>         /* Set the WEPROM bit (eeprom write enable). */
>         cr |= 0x20;
>         set_registers(dev, CR, 1, &cr);
>
>
>         do {
>                 get_registers(dev, CR, 1, &data);
>         } while ((data & 0x10) && --i);
>
> Don't these also have the same problem?

Exactly, will fix other places.

Thanks!