| 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: <aPtiLLlLu-KbnCz_@stanley.mountain>
Date: Fri, 24 Oct 2025 14:25:32 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: netdev@...r.kernel.org
Subject: [bug report] net: stmmac: mdio: use phy_find_first to simplify
stmmac_mdio_register
Hello Heiner Kallweit,
Commit 4a107a0e8361 ("net: stmmac: mdio: use phy_find_first to
simplify stmmac_mdio_register") from Oct 18, 2025 (linux-next), leads
to the following (unpublished) Smatch static checker warning:
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c:684 stmmac_mdio_register()
warn: array off by one? 'new_bus->irq[phydev->mdio.addr]'
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
577 int stmmac_mdio_register(struct net_device *ndev)
578 {
579 int err = 0;
580 struct mii_bus *new_bus;
581 struct stmmac_priv *priv = netdev_priv(ndev);
582 struct stmmac_mdio_bus_data *mdio_bus_data = priv->plat->mdio_bus_data;
583 struct device_node *mdio_node = priv->plat->mdio_node;
584 struct device *dev = ndev->dev.parent;
585 struct fwnode_handle *fixed_node;
586 struct fwnode_handle *fwnode;
587 struct phy_device *phydev;
588 int max_addr;
589
590 if (!mdio_bus_data)
591 return 0;
592
593 stmmac_mdio_bus_config(priv);
594
595 new_bus = mdiobus_alloc();
596 if (!new_bus)
597 return -ENOMEM;
598
599 if (mdio_bus_data->irqs)
600 memcpy(new_bus->irq, mdio_bus_data->irqs, sizeof(new_bus->irq));
601
602 new_bus->name = "stmmac";
603
604 if (priv->plat->has_xgmac) {
605 new_bus->read = &stmmac_xgmac2_mdio_read_c22;
606 new_bus->write = &stmmac_xgmac2_mdio_write_c22;
607 new_bus->read_c45 = &stmmac_xgmac2_mdio_read_c45;
608 new_bus->write_c45 = &stmmac_xgmac2_mdio_write_c45;
609
610 if (priv->synopsys_id < DWXGMAC_CORE_2_20) {
611 /* Right now only C22 phys are supported */
612 max_addr = MII_XGMAC_MAX_C22ADDR + 1;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
max_addr set here. This is the line which gave me pause.
MII_XGMAC_MAX_C22ADDR is 3, but why do we do a "+ 1"?
613
614 /* Check if DT specified an unsupported phy addr */
615 if (priv->plat->phy_addr > MII_XGMAC_MAX_C22ADDR)
616 dev_err(dev, "Unsupported phy_addr (max=%d)\n",
617 MII_XGMAC_MAX_C22ADDR);
618 } else {
619 /* XGMAC version 2.20 onwards support 32 phy addr */
620 max_addr = PHY_MAX_ADDR;
^^^^^^^^^^^^^^^^^^^^^^^^
621 }
622 } else {
623 new_bus->read = &stmmac_mdio_read_c22;
624 new_bus->write = &stmmac_mdio_write_c22;
625 if (priv->plat->has_gmac4) {
626 new_bus->read_c45 = &stmmac_mdio_read_c45;
627 new_bus->write_c45 = &stmmac_mdio_write_c45;
628 }
629
630 max_addr = PHY_MAX_ADDR;
^^^^^^^^^^^^^^^^^^^^^^^
631 }
632
633 if (mdio_bus_data->needs_reset)
634 new_bus->reset = &stmmac_mdio_reset;
635
636 snprintf(new_bus->id, MII_BUS_ID_SIZE, "%s-%x",
637 new_bus->name, priv->plat->bus_id);
638 new_bus->priv = ndev;
639 new_bus->phy_mask = mdio_bus_data->phy_mask | mdio_bus_data->pcs_mask;
640 new_bus->parent = priv->device;
641
642 err = of_mdiobus_register(new_bus, mdio_node);
643 if (err == -ENODEV) {
644 err = 0;
645 dev_info(dev, "MDIO bus is disabled\n");
646 goto bus_register_fail;
647 } else if (err) {
648 dev_err_probe(dev, err, "Cannot register the MDIO bus\n");
649 goto bus_register_fail;
650 }
651
652 /* Looks like we need a dummy read for XGMAC only and C45 PHYs */
653 if (priv->plat->has_xgmac)
654 stmmac_xgmac2_mdio_read_c45(new_bus, 0, 0, 0);
655
656 /* If fixed-link is set, skip PHY scanning */
657 fwnode = priv->plat->port_node;
658 if (!fwnode)
659 fwnode = dev_fwnode(priv->device);
660
661 if (fwnode) {
662 fixed_node = fwnode_get_named_child_node(fwnode, "fixed-link");
663 if (fixed_node) {
664 fwnode_handle_put(fixed_node);
665 goto bus_register_done;
666 }
667 }
668
669 if (priv->plat->phy_node || mdio_node)
670 goto bus_register_done;
671
672 phydev = phy_find_first(new_bus);
673 if (!phydev || phydev->mdio.addr > max_addr) {
This is an unpublished check because it assumes that every > comparison
is off by one unless proven otherwise. In this case
max_addr is PHY_MAX_ADDR (32) and phydev->mdio.addr is also 32.
674 dev_warn(dev, "No PHY found\n");
675 err = -ENODEV;
676 goto no_phy_found;
677 }
678
679 /*
680 * If an IRQ was provided to be assigned after
681 * the bus probe, do it here.
682 */
683 if (!mdio_bus_data->irqs && mdio_bus_data->probed_phy_irq > 0) {
--> 684 new_bus->irq[phydev->mdio.addr] = mdio_bus_data->probed_phy_irq;
^^^^^^^^^^^^^^^^^
Then this will corrupt memory.
685 phydev->irq = mdio_bus_data->probed_phy_irq;
686 }
687
688 /*
689 * If we're going to bind the MAC to this PHY bus, and no PHY number
690 * was provided to the MAC, use the one probed here.
691 */
692 if (priv->plat->phy_addr == -1)
693 priv->plat->phy_addr = phydev->mdio.addr;
694
695 phy_attached_info(phydev);
696
697 bus_register_done:
698 priv->mii = new_bus;
699
700 return 0;
701
702 no_phy_found:
703 mdiobus_unregister(new_bus);
704 bus_register_fail:
705 mdiobus_free(new_bus);
706 return err;
707 }
regards,
dan carpenter
Powered by blists - more mailing lists