From: Florian Fainelli <f.faine...@gmail.com>
Date: Oct/15/2019, 23:49:53 (UTC+00:00)

> The function phy_rgmii_debug_probe() could also be used by an Ethernet
> controller during its selftests routines instead of open-coding that
> part.

I can add it to stmmac selftests then :)

> +int phy_rgmii_debug_probe(struct phy_device *phydev)
> +{
> +     struct net_device *ndev = phydev->attached_dev;
> +     unsigned char operstate = ndev->operstate;
> +     phy_interface_t rgmii_modes[4] = {

This can be static.

> +             PHY_INTERFACE_MODE_RGMII,
> +             PHY_INTERFACE_MODE_RGMII_ID,
> +             PHY_INTERFACE_MODE_RGMII_RXID,
> +             PHY_INTERFACE_MODE_RGMII_TXID
> +     };
> +     struct phy_rgmii_debug_priv *priv;
> +     unsigned int i, count;
> +     int ret;
> +
> +     ret = phy_rgmii_can_debug(phydev);
> +     if (ret <= 0)
> +             return ret;
> +
> +     priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +     if (!priv)
> +             return -ENOMEM;
> +
> +     if (phy_rgmii_probes_type.af_packet_priv)
> +             return -EBUSY;

You are leaking "priv" here.

> +
> +     phy_rgmii_probes_type.af_packet_priv = priv;
> +     priv->phydev = phydev;
> +     INIT_WORK(&priv->work, phy_rgmii_probe_xmit_work);
> +     init_completion(&priv->compl);
> +
> +     /* We are now testing this network device */
> +     ndev->operstate = IF_OPER_TESTING;
> +
> +     dev_add_pack(&phy_rgmii_probes_type);
> +
> +     /* Determine where to start */
> +     for (i = 0; i < ARRAY_SIZE(rgmii_modes); i++) {
> +             if (phydev->interface == rgmii_modes[i])
> +                     break;
> +     }
> +
> +     /* Now probe all modes */
> +     for (count = 0; count < ARRAY_SIZE(rgmii_modes); count++) {
> +             ret = phy_rgmii_probe_interface(priv, rgmii_modes[i]);
> +             if (ret == 0) {
> +                     netdev_info(ndev, "Determined \"%s\" to be correct\n",
> +                                 phy_modes(rgmii_modes[i]));
> +                     break;
> +             }
> +             i = (i + 1) % ARRAY_SIZE(rgmii_modes);
> +     }
> +
> +     dev_remove_pack(&phy_rgmii_probes_type);
> +     kfree(priv);
> +     phy_rgmii_probes_type.af_packet_priv = NULL;

I think you should set af_packet_priv to NULL before freeing "priv" 
because of the "if ([...].af_packet_priv)" test, otherwise you can get 
use-after-free.

---
Thanks,
Jose Miguel Abreu

Reply via email to