| 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
| ||
|
Message-ID: <ogh66evbiin6ugz2nkhpn33iffsjeo3kvdawe6cig4jjt67ygl@a3nqlqefvzwb>
Date: Thu, 7 Aug 2025 17:10:39 +0800
From: Xu Yang <xu.yang_2@....com>
To: Andrew Lunn <andrew@...n.ch>
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()
Hi Andrew,
On Wed, Aug 06, 2025 at 05:01:22PM +0200, Andrew Lunn wrote:
> > > > 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?
static const struct driver_info ax88772_info = {
.description = "ASIX AX88772 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
.reset = ax88772_reset,
.stop = ax88772_stop,
.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
};
}
// DLink DUB-E100 H/W Ver B1
USB_DEVICE (0x07d1, 0x3c05),
.driver_info = (unsigned long) &ax88772_info,
}
This one.
>
> > > > 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.
I attach the usb bus data about this USB device for reference.
If you search "1:002:0 s c0 07", you will locate AX_CMD_READ_MII_REG (0x07)
transfer.
For address 0x00:
ffff00008598c780 71304432 S Ci:1:002:0 s c0 07 0000 0002 0002 2 <
ffff00008598c780 71304609 C Ci:1:002:0 0 2 = 0000
ffff00008598c780 71306137 S Ci:1:002:0 s c0 07 0000 0003 0002 2 <
ffff00008598c780 71306359 C Ci:1:002:0 0 2 = 0000
...
For address 0x03:
ffff00008598c780 71335993 S Ci:1:002:0 s c0 07 0003 0002 0002 2 <
ffff00008598c780 71336203 C Ci:1:002:0 0 2 = 4302
ffff00008598c780 71337758 S Ci:1:002:0 s c0 07 0003 0003 0002 2 <
ffff00008598c780 71337942 C Ci:1:002:0 0 2 = 540c
...
For address 0x04:
ffff00008598c780 71346488 S Ci:1:002:0 s c0 07 0004 0002 0002 2 <
ffff00008598c780 71346706 C Ci:1:002:0 0 2 = 540c
ffff00008598c780 71348311 S Ci:1:002:0 s c0 07 0004 0003 0002 2 <
ffff00008598c780 71348541 C Ci:1:002:0 0 2 = 540c
So it is indeed returned by this device.
>
> What does asix_read_phy_addr() return?
If you search "1:002:0 s c0 19", you will locate AX_CMD_READ_PHY_ID (0x19) transfer.
ffff00008598c780 71134999 S Ci:1:002:0 s c0 19 0000 0000 0002 2 <
ffff00008598c780 71135082 C Ci:1:002:0 0 2 = e003
So it returns 'e0 03'.
>
> 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.
Because it's ax88772_bind(), so I can't test this. Sorry for this.
Thanks,
Xu Yang
>
> Andrew
View attachment "usbnet.log" of type "text/plain" (49220 bytes)
Powered by blists - more mailing lists