[PATCH net-next 1/2] net: mdio: of: fix potential NULL pointer derefernce

[PATCH net-next 1/2] net: mdio: of: fix potential NULL pointer derefernce

[Michael Walle]

of_find_mii_timestamper() returns NULL if no timestamper is found.
Therefore, guard the unregister_mii_timestamper() calls.

Fixes: 1dca22b18421 ("net: mdio: of: Register discovered MII time stampers.")
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/of/of_mdio.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index f5c2a5487761..db0ed5879803 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -81,13 +81,15 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
 	else
 		phy = get_phy_device(mdio, addr, is_c45);
 	if (IS_ERR(phy)) {
-		unregister_mii_timestamper(mii_ts);
+		if (mii_ts)
+			unregister_mii_timestamper(mii_ts);
 		return PTR_ERR(phy);
 	}
 
 	rc = of_irq_get(child, 0);
 	if (rc == -EPROBE_DEFER) {
-		unregister_mii_timestamper(mii_ts);
+		if (mii_ts)
+			unregister_mii_timestamper(mii_ts);
 		phy_device_free(phy);
 		return rc;
 	}
@@ -116,7 +118,8 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
 	 * register it */
 	rc = phy_device_register(phy);
 	if (rc) {
-		unregister_mii_timestamper(mii_ts);
+		if (mii_ts)
+			unregister_mii_timestamper(mii_ts);
 		phy_device_free(phy);
 		of_node_put(child);
 		return rc;
-- 
2.20.1

[PATCH net-next 2/2] net: mii_timestamper: fix static allocation by PHY driver

[Michael Walle]

If phydev->mii_ts is set by the PHY driver, it will always be
overwritten in of_mdiobus_register_phy(). Fix it. Also make sure, that
the unregister() doesn't do anything if the mii_timestamper was provided by
the PHY driver.

Fixes: 1dca22b18421 ("net: mdio: of: Register discovered MII time stampers.")
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/mii_timestamper.c | 7 +++++++
 drivers/of/of_mdio.c              | 8 +++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mii_timestamper.c b/drivers/net/phy/mii_timestamper.c
index 2f12c5d901df..b71b7456462d 100644
--- a/drivers/net/phy/mii_timestamper.c
+++ b/drivers/net/phy/mii_timestamper.c
@@ -111,6 +111,13 @@ void unregister_mii_timestamper(struct mii_timestamper *mii_ts)
 	struct mii_timestamping_desc *desc;
 	struct list_head *this;
 
+	/* mii_timestamper statically registered by the PHY driver won't use the
+	 * register_mii_timestamper() and thus don't have ->device set. Don't
+	 * try to unregister these.
+	 */
+	if (!mii_ts->device)
+		return;
+
 	mutex_lock(&tstamping_devices_lock);
 	list_for_each(this, &mii_timestamping_devices) {
 		desc = list_entry(this, struct mii_timestamping_desc, list);
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index db0ed5879803..8270bbf505fb 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -124,7 +124,13 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
 		of_node_put(child);
 		return rc;
 	}
-	phy->mii_ts = mii_ts;
+
+	/* phy->mii_ts may already be defined by the PHY driver. A
+	 * mii_timestamper probed via the device tree will still have
+	 * precedence.
+	 */
+	if (mii_ts)
+		phy->mii_ts = mii_ts;
 
 	dev_dbg(&mdio->dev, "registered phy %pOFn at address %i\n",
 		child, addr);
-- 
2.20.1

Re: [PATCH net-next 2/2] net: mii_timestamper: fix static allocation by PHY driver

[Richard Cochran]

On Thu, Jan 30, 2020 at 06:44:51PM +0100, Michael Walle wrote:
> If phydev->mii_ts is set by the PHY driver, it will always be
> overwritten in of_mdiobus_register_phy(). Fix it. Also make sure, that
> the unregister() doesn't do anything if the mii_timestamper was provided by
> the PHY driver.
> 
> Fixes: 1dca22b18421 ("net: mdio: of: Register discovered MII time stampers.")
> Signed-off-by: Michael Walle <michael@walle.cc>

Thanks for the fix.

Acked-by: Richard Cochran <richardcochran@gmail.com>

Re: [PATCH net-next 1/2] net: mdio: of: fix potential NULL pointer derefernce

[Richard Cochran]

On Thu, Jan 30, 2020 at 06:44:50PM +0100, Michael Walle wrote:
> of_find_mii_timestamper() returns NULL if no timestamper is found.
> Therefore, guard the unregister_mii_timestamper() calls.
> 
> Fixes: 1dca22b18421 ("net: mdio: of: Register discovered MII time stampers.")
> Signed-off-by: Michael Walle <michael@walle.cc>

Good catch.

Acked-by: Richard Cochran <richardcochran@gmail.com>

Re: [PATCH net-next 1/2] net: mdio: of: fix potential NULL pointer derefernce

[Jakub Kicinski]

On Thu, 30 Jan 2020 18:44:50 +0100, Michael Walle wrote:
> of_find_mii_timestamper() returns NULL if no timestamper is found.
> Therefore, guard the unregister_mii_timestamper() calls.
> 
> Fixes: 1dca22b18421 ("net: mdio: of: Register discovered MII time stampers.")
> Signed-off-by: Michael Walle <michael@walle.cc>

Applied both, thank you.