On 20. 12. 18 14:13, Stefan Roese wrote:

On 19.12.18 16:57, Michal Simek wrote:

...

When fixed-link phy is used subnode offset is used as phy address. This number is bigger then space allocated for bus structure (allocated via mdio_alloc). bus->phymap[] array has PHY_MAX_ADDR size (32). That's why writing bus->phymap[addr] where addr is < 0 or > PHY_MAX_ADDR is causing write to memory which can caused full U-Boot crash.

The patch is checking if address is in correct range.

Signed-off-by: Michal Simek michal.simek@xilinx.com

search_for_existing_phy() is using this array but there is if check already that's why it shouldn't be a big deal.

---

drivers/net/phy/phy.c | 3 ++-   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index e837eb7688cc..cda4caa8034d 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -656,7 +656,8 @@ static struct phy_device *phy_device_create(struct mii_dev *bus, int addr,         phy_probe(dev);   -    bus->phymap[addr] = dev; +    if (addr >= 0 && addr < PHY_MAX_ADDR) +        bus->phymap[addr] = dev;

How about adding some error / warning print in the else path here?

We can add it but I am not quite sure how that error should look like in this case. Because addr is filled by node offset got from sn = fdt_first_subnode(gd->fdt_blob, dev_of_offset(dev));

It means this error/warning will be shown all the time which is also weird.

The solution could be to save link to phy DT node in phy_device structure and then read it on driver not like this int ofnode = phydev->addr; which is done in fixed.c

But anyway still that above fix make sense to make sure that we are not overwriting stuff.

Thanks, Michal