Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jose Abreu <Jose.Abreu@synopsys.com>
To:     Florian Fainelli <f.fainelli@gmail.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
CC:     Andrew Lunn <andrew@lunn.ch>,
        "David S. Miller" <davem@davemloft.net>,
        open list <linux-kernel@vger.kernel.org>,
        "hkallweit1@gmail.com" <hkallweit1@gmail.com>,
        "bcm-kernel-feedback-list@broadcom.com" 
        <bcm-kernel-feedback-list@broadcom.com>,
        "olteanv@gmail.com" <olteanv@gmail.com>,
        "rmk+kernel@armlinux.org.uk" <rmk+kernel@armlinux.org.uk>,
        "cphealy@gmail.com" <cphealy@gmail.com>
Subject: RE: [PATCH net-next 2/2] net: phy: Add ability to debug RGMII
 connections
Thread-Topic: [PATCH net-next 2/2] net: phy: Add ability to debug RGMII
 connections
Thread-Index: AQHVg6sD/Z3YV8nANkekqPlmkekI5adc9FFA
Date:   Wed, 16 Oct 2019 08:55:43 +0000
Message-ID: <BN8PR12MB3266989D42B1C5B09E7053A3D3920@BN8PR12MB3266.namprd12.prod.outlook.com>
References: <20191015224953.24199-1-f.fainelli@gmail.com>
 <20191015224953.24199-3-f.fainelli@gmail.com>
In-Reply-To: <20191015224953.24199-3-f.fainelli@gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: synopsys.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 68241eb2-c15e-4785-be81-08d75216a075
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Oct 2019 08:55:43.0344
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: c33c9f88-1eb7-4099-9700-16013fd9e8aa
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: uzhKSUmNMw68DyXE6w1SMolTkY4TMgaZpTcPAOmDw86pXbRgbYG8POHyKPLZafrSwerLmN6uTPrU18gc/TtzKA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR12MB2851
X-OriginatorOrg: synopsys.com
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Florian Fainelli <f.fainelli@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 =3D phydev->attached_dev;
> +	unsigned char operstate =3D ndev->operstate;
> +	phy_interface_t rgmii_modes[4] =3D {

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

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

---
Thanks,
Jose Miguel Abreu