[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN8PR12MB3266989D42B1C5B09E7053A3D3920@BN8PR12MB3266.namprd12.prod.outlook.com>
Date: Wed, 16 Oct 2019 08:55:43 +0000
From: Jose Abreu <Jose.Abreu@...opsys.com>
To: Florian Fainelli <f.fainelli@...il.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
open list <linux-kernel@...r.kernel.org>,
"hkallweit1@...il.com" <hkallweit1@...il.com>,
"bcm-kernel-feedback-list@...adcom.com"
<bcm-kernel-feedback-list@...adcom.com>,
"olteanv@...il.com" <olteanv@...il.com>,
"rmk+kernel@...linux.org.uk" <rmk+kernel@...linux.org.uk>,
"cphealy@...il.com" <cphealy@...il.com>
Subject: RE: [PATCH net-next 2/2] net: phy: Add ability to debug RGMII
connections
From: Florian Fainelli <f.fainelli@...il.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
Powered by blists - more mailing lists