| Message ID | 20240521-icssg-prueth-fix-v1-1-b4b17b1433e9@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b31c7e78086127a7fcaa761e8d336ee855a920c6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: ti: icssg_prueth: Fix NULL pointer dereference in prueth_probe() | expand |
+ Andrew Lunn, Diogo Ivo, Vignesh Raghavendra Not trimming reply to provide context for these people On Tue, May 21, 2024 at 02:44:11PM +0200, Romain Gantois wrote: > In the prueth_probe() function, if one of the calls to emac_phy_connect() > fails due to of_phy_connect() returning NULL, then the subsequent call to > phy_attached_info() will dereference a NULL pointer. > > Check the return code of emac_phy_connect and fail cleanly if there is an > error. > > Fixes: 128d5874c082 ("net: ti: icssg-prueth: Add ICSSG ethernet driver") > Cc: stable@vger.kernel.org > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> For Networking patches, please consider seeding the CC list using ./scripts/get_maintainer.pl this.patch. I've added the people who seemed to be missing. The patch itself looks good to me. Reviewed-by: Simon Horman <horms@kernel.org> > --- > Hello everyone, > > There is a possible NULL pointer dereference in the prueth_probe() function of > the icssg_prueth driver. I discovered this while testing a platform with one > PRUETH MAC enabled out of the two available. > > These are the requirements to reproduce the bug: > > prueth_probe() is called > either eth0_node or eth1_node is not NULL > in emac_phy_connect: of_phy_connect() returns NULL > > Then, the following leads to the NULL pointer dereference: > > prueth->emac[PRUETH_MAC0]->ndev->phydev is set to NULL > prueth->emac[PRUETH_MAC0]->ndev->phydev is passed to phy_attached_info() > -> phy_attached_print() dereferences phydev which is NULL > > This series provides a fix by checking the return code of emac_phy_connect(). > > Best Regards, > > Romain > --- > drivers/net/ethernet/ti/icssg/icssg_prueth.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c > index 7c9e9518f555a..1ea3fbd5e954e 100644 > --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c > +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c > @@ -1039,7 +1039,12 @@ static int prueth_probe(struct platform_device *pdev) > > prueth->registered_netdevs[PRUETH_MAC0] = prueth->emac[PRUETH_MAC0]->ndev; > > - emac_phy_connect(prueth->emac[PRUETH_MAC0]); > + ret = emac_phy_connect(prueth->emac[PRUETH_MAC0]); > + if (ret) { > + dev_err(dev, > + "can't connect to MII0 PHY, error -%d", ret); > + goto netdev_unregister; > + } > phy_attached_info(prueth->emac[PRUETH_MAC0]->ndev->phydev); > } > > @@ -1051,7 +1056,12 @@ static int prueth_probe(struct platform_device *pdev) > } > > prueth->registered_netdevs[PRUETH_MAC1] = prueth->emac[PRUETH_MAC1]->ndev; > - emac_phy_connect(prueth->emac[PRUETH_MAC1]); > + ret = emac_phy_connect(prueth->emac[PRUETH_MAC1]); > + if (ret) { > + dev_err(dev, > + "can't connect to MII1 PHY, error %d", ret); > + goto netdev_unregister; > + } > phy_attached_info(prueth->emac[PRUETH_MAC1]->ndev->phydev); > } > > > --- > base-commit: e4a87abf588536d1cdfb128595e6e680af5cf3ed > change-id: 20240521-icssg-prueth-fix-03b03064c5ce > > Best regards, > -- > Romain Gantois <romain.gantois@bootlin.com> > >
On 21/05/24 6:14 pm, Romain Gantois wrote: > In the prueth_probe() function, if one of the calls to emac_phy_connect() > fails due to of_phy_connect() returning NULL, then the subsequent call to > phy_attached_info() will dereference a NULL pointer. > > Check the return code of emac_phy_connect and fail cleanly if there is an > error. > > Fixes: 128d5874c082 ("net: ti: icssg-prueth: Add ICSSG ethernet driver") > Cc: stable@vger.kernel.org > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> Reviewed-by: MD Danish Anwar <danishanwar@ti.com>
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 21 May 2024 14:44:11 +0200 you wrote: > In the prueth_probe() function, if one of the calls to emac_phy_connect() > fails due to of_phy_connect() returning NULL, then the subsequent call to > phy_attached_info() will dereference a NULL pointer. > > Check the return code of emac_phy_connect and fail cleanly if there is an > error. > > [...] Here is the summary with links: - [net] net: ti: icssg_prueth: Fix NULL pointer dereference in prueth_probe() https://git.kernel.org/netdev/net/c/b31c7e780861 You are awesome, thank you!
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c index 7c9e9518f555a..1ea3fbd5e954e 100644 --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c @@ -1039,7 +1039,12 @@ static int prueth_probe(struct platform_device *pdev) prueth->registered_netdevs[PRUETH_MAC0] = prueth->emac[PRUETH_MAC0]->ndev; - emac_phy_connect(prueth->emac[PRUETH_MAC0]); + ret = emac_phy_connect(prueth->emac[PRUETH_MAC0]); + if (ret) { + dev_err(dev, + "can't connect to MII0 PHY, error -%d", ret); + goto netdev_unregister; + } phy_attached_info(prueth->emac[PRUETH_MAC0]->ndev->phydev); } @@ -1051,7 +1056,12 @@ static int prueth_probe(struct platform_device *pdev) } prueth->registered_netdevs[PRUETH_MAC1] = prueth->emac[PRUETH_MAC1]->ndev; - emac_phy_connect(prueth->emac[PRUETH_MAC1]); + ret = emac_phy_connect(prueth->emac[PRUETH_MAC1]); + if (ret) { + dev_err(dev, + "can't connect to MII1 PHY, error %d", ret); + goto netdev_unregister; + } phy_attached_info(prueth->emac[PRUETH_MAC1]->ndev->phydev); }
In the prueth_probe() function, if one of the calls to emac_phy_connect() fails due to of_phy_connect() returning NULL, then the subsequent call to phy_attached_info() will dereference a NULL pointer. Check the return code of emac_phy_connect and fail cleanly if there is an error. Fixes: 128d5874c082 ("net: ti: icssg-prueth: Add ICSSG ethernet driver") Cc: stable@vger.kernel.org Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> --- Hello everyone, There is a possible NULL pointer dereference in the prueth_probe() function of the icssg_prueth driver. I discovered this while testing a platform with one PRUETH MAC enabled out of the two available. These are the requirements to reproduce the bug: prueth_probe() is called either eth0_node or eth1_node is not NULL in emac_phy_connect: of_phy_connect() returns NULL Then, the following leads to the NULL pointer dereference: prueth->emac[PRUETH_MAC0]->ndev->phydev is set to NULL prueth->emac[PRUETH_MAC0]->ndev->phydev is passed to phy_attached_info() -> phy_attached_print() dereferences phydev which is NULL This series provides a fix by checking the return code of emac_phy_connect(). Best Regards, Romain --- drivers/net/ethernet/ti/icssg/icssg_prueth.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) --- base-commit: e4a87abf588536d1cdfb128595e6e680af5cf3ed change-id: 20240521-icssg-prueth-fix-03b03064c5ce Best regards,