netdev - Re: [RESEND] net: phy: fix NULL pointer dereference in phy_polling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b9140415-2478-4264-a674-c158ca14eb07@lunn.ch>
Date: Wed, 6 Aug 2025 17:01:22 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Xu Yang <xu.yang_2@....com>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>, hkallweit1@...il.com,
	o.rempel@...gutronix.de, pabeni@...hat.com, netdev@...r.kernel.org,
	imx@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [RESEND] net: phy: fix NULL pointer dereference in
 phy_polling_mode()

> > > Reproduce step is simple:
> > > 
> > > 1. connect an USB to Ethernet device to USB port, I'm using "D-Link Corp.
> > >    DUB-E100 Fast Ethernet Adapter".

static const struct driver_info dlink_dub_e100_info = {
        .description = "DLink DUB-E100 USB Ethernet",
        .bind = ax88172_bind,
        .status = asix_status,
        .link_reset = ax88172_link_reset,
        .reset = ax88172_link_reset,
        .flags =  FLAG_ETHER | FLAG_LINK_INTR,
        .data = 0x009f9d9f,
};

{
        // DLink DUB-E100
        USB_DEVICE (0x2001, 0x1a00),
        .driver_info =  (unsigned long) &dlink_dub_e100_info,
}, {

Is this the device you have?

> > > 2. the asix driver (drivers/net/usb/asix_devices.c) will bind to this USB
> > >    device.
> > > 
> > > root@...95evk:~# lsusb -t
> > > /:  Bus 001.Port 001: Dev 001, Class=root_hub, Driver=ci_hdrc/1p, 480M
> > >     |__ Port 001: Dev 003, If 0, Class=Vendor Specific Class, Driver=asix, 480M
> > > 
> > > 3. then the driver will create many mdio devices. 
> > > 
> > > root@...95evk:/sys/bus/mdio_bus# ls -d devices/usb*
> > > devices/usb-001:005:00  devices/usb-001:005:04  devices/usb-001:005:08  devices/usb-001:005:0c  devices/usb-001:005:10  devices/usb-001:005:14  devices/usb-001:005:18  devices/usb-001:005:1c
> > > devices/usb-001:005:01  devices/usb-001:005:05  devices/usb-001:005:09  devices/usb-001:005:0d  devices/usb-001:005:11  devices/usb-001:005:15  devices/usb-001:005:19  devices/usb-001:005:1d
> > > devices/usb-001:005:02  devices/usb-001:005:06  devices/usb-001:005:0a  devices/usb-001:005:0e  devices/usb-001:005:12  devices/usb-001:005:16  devices/usb-001:005:1a  devices/usb-001:005:1e
> > > devices/usb-001:005:03  devices/usb-001:005:07  devices/usb-001:005:0b  devices/usb-001:005:0f  devices/usb-001:005:13  devices/usb-001:005:17  devices/usb-001:005:1b  devices/usb-001:005:1f
> > 
> > This looks broken - please check what
> > /sys/bus/mdio_bus/devices/usb*/phy_id contains.
> 
> root@...95evk:~# cat /sys/bus/mdio_bus/devices/usb*/phy_id
> 0x00000000
> 0x00000000
> 0x00000000
> 0x02430c54
> 0x0c540c54
> 0x0c540c54
> 0x0c540c54
> 0x0c540c54

This suggests which version of the asix device has broken MDIO bus
access.

The first three 0x00000000 are odd. If there is no device at an
address you expect to read 0xffffffff. phylib will ignore 0xffffffff
and not create a device. 0x00000000 suggests something actually is on
the bus, and is responding to reads of registers 2 and 3, but
returning 0x0000 is not expected.

And then 0x02430c54 for all other addresses suggests the device is not
correctly handling the bus address, and is mapping the address
parameter to a single bus address.

What does asix_read_phy_addr() return?

This is completely untested, not even compiled:

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 9b0318fb50b5..e136b25782d9 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -260,13 +260,20 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
        dev->mii.dev = dev->net;
        dev->mii.mdio_read = asix_mdio_read;
        dev->mii.mdio_write = asix_mdio_write;
-       dev->mii.phy_id_mask = 0x3f;
        dev->mii.reg_num_mask = 0x1f;
 
        dev->mii.phy_id = asix_read_phy_addr(dev, true);
        if (dev->mii.phy_id < 0)
                return dev->mii.phy_id;
 
+       if (dev->mii.phy_id > 31) {
+               netdev_err(dev->net, "Invalid PHY address %d\n",
+                          dev->mii.phy_id);
+               return -EINVAL;
+       }
+
+       dev->mii.phy_id_mask = BIT(dev->mii.phy_id);
+
        dev->net->netdev_ops = &ax88172_netdev_ops;
        dev->net->ethtool_ops = &ax88172_ethtool_ops;
        dev->net->needed_headroom = 4; /* cf asix_tx_fixup() */

The idea is to limit the scanning of the bus to just the address where
we expect the PHY to be.  See if this gives you a single PHY, and that
PHY actually works.

	Andrew