linux-kernel - Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume

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: <20250917082457.1200792-1-yicongsrfy@163.com>
Date: Wed, 17 Sep 2025 16:24:57 +0800
From: yicongsrfy@....com
To: andrew@...n.ch,
	Frank.Sae@...or-comm.com
Cc: davem@...emloft.net,
	edumazet@...gle.com,
	hkallweit1@...il.com,
	kuba@...nel.org,
	linux-kernel@...r.kernel.org,
	linux@...linux.org.uk,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	yicong@...inos.cn
Subject: Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume

On Thu, 11 Sep 2025 14:58:32 +0200, Andrew Lunn <andrew@...n.ch> wrote:
>
> > Some PHY chips support two addresses, using address 0 as a broadcast address
> > and address 1 as the hardware address. Both addresses respond to GMAC's MDIO
> > read/write operations. As a result, during 'mdio_scan', both PHY addresses are
> > detected, leading to the creation of two PHY device instances (for example,
> > as in my previous email: xxxxmac_mii_bus-XXXX:00:00 and xxxxmac_mii_bus-XXXX:00:01).
>
> I would say the PHY driver is broken, or at least, not correctly
> handling the situation. When the scan finds the PHY at address 0, the
> PHY driver is probed, and it should look at the strapping and decided,
> if the strapping has put the PHY at address 0, or its the broadcast
> address being used. If it is the broadcast address, turn off broadcast
> address, and return -ENODEV. phylib should then not create a PHY at
> address 0. The scan will continue and find the PHY at its correct
> address. Your problem then goes away, and phylib has a correct
> representation of the hardware.

+To: Frank <Frank.Sae@...or-comm.com> (maintainer:MOTORCOMM PHY DRIVER)

Hi Andrew, thank you again for your reply!

Recently I conducted the following tests:
Following your suggestion, to avoid compatibility issues caused by
disabling broadcast address (after all, we don't know if any vendor
might directly use address 0 as the PHY's hardware address——otherwise
we could simply start scanning from address 1),I modified the logic
only for one specific chip in motorcomm.c, aiming to prevent phylib
from creating a phy_device instance at address 0:

static int yt8521_probe(struct phy_device *phydev) {
    ...
    if (/* phydev->mdio.addr == 0 AND phy_addr0 broadcast enable */)
        return -ENODEV;
    ...
}

However, this had no effect, for the following reasons:
1. `get_phy_device` does not invoke the driver's probe function,
    so it always succeeds.
2. Subsequently, during the `phy_device_register` probe, the entire
   call chain contains calls without return value handling, so we
   cannot rely on the driver's return value to determine whether to
   create the `phy_device`.

An example of `dump_stack()` output:
```
[    4.509504]  yt8521_probe+0x34/0x330 [motorcomm]  ==> checking inside driver
[    4.509513]  phy_probe+0x78/0x2b0 [libphy]
[    4.509529]  really_probe+0x184/0x3d0
[    4.509532]  __driver_probe_device+0x80/0x178
[    4.509534]  driver_probe_device+0x40/0x118
[    4.509536]  __device_attach_driver+0xb8/0x158
[    4.509538]  bus_for_each_drv+0x84/0xe8
[    4.509540]  __device_attach+0xd4/0x1b0
[    4.509542]  device_initial_probe+0x18/0x28     ==> allows asynchronous initialization
[    4.509543]  bus_probe_device+0xa8/0xb8         ==> no return value here
[    4.509545]  device_add+0x510/0x700
[    4.509549]  phy_device_register+0x58/0xb0 [libphy]
[    4.509562]  mdiobus_scan+0x80/0x1a8 [libphy]
[    4.509576]  __mdiobus_register+0x208/0x4b0 [libphy]
```

The existing framework appears unable to handle this situation.
Yet, there are at least two vendors (I have hardware from two
different vendors) whose PHY chips have broadcast addressing on
address 0 enabled by default.

Does this mean we can only force these vendors to disable it by default?