request for 4.4-stable: d733f7542ad4 ("drivers: net: cpsw: fix segfault in case of bad phy-handle")

PATCH SZ Lin (林上智) 2 participants 4 messages Sep 14, 2018 – Sep 24, 2018

Subsystems (3)

Network drivers Stable branch TI Ethernet switch driver (cpsw)

lore

Share & tools

lore

SZ Lin (林上智)

author

2018-09-14 20:39 UTC #0 lore raw

Hi Greg,

This patch is not marked for 4.4-stable, but it's already in 4.9 and 4.14 stable.

Please apply to 4.4-stable.

This patch adds error handling to avoid segfault situation, I've tweaked patch to use original print function of dev_info() instead of phy_attached_info() which is in commit 2220943a21e26d97d7fd8f83c004b947326b469d upstream.

Signature · 1 line

SZ Lin (林上智)

Quoted 1 line

From 194f912f0303146bffe8a1fd0d0c8a5b9208752c Mon Sep 17 00:00:00 2001

From: David Rivshin <[email protected]>
Date: Wed, 27 Apr 2016 21:32:31 -0400
Subject: [PATCH] drivers: net: cpsw: fix segfault in case of bad phy-handle

commit d733f7542ad47cf73e033c90cf55158587e1d060 upstream

If an emac node has a phy-handle property that points to something which is not a phy, then a segmentation fault will occur when the interface is brought up. This is because while phy_connect() will return ERR_PTR() on failure, of_phy_connect() will return NULL. The common error check uses IS_ERR(), and so missed when of_phy_connect() fails. The NULL pointer is then dereferenced.

Also, the common error message referenced slave->data->phy_id, which would be empty in the case of phy-handle. Instead, use the name of the device_node as a useful identifier. And in the phy_id case add the error code for completeness.

Fixes 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")

Signed-off-by David Rivshin <[email protected]>

Signed-off-by David S. Miller <[email protected]>

[SZ Lin (林上智): Tweak the patch to use original print function of dev_info()]

Signed-off-by SZ Lin (林上智) <[email protected]>

---
 drivers/net/ethernet/ti/cpsw.c | 38 +++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 14 deletions(-)

drivers/net/ethernet/ti/cpsw.c +24 ~~-14~~

index c21c80a228d9..c1a4425b54e7 100644
@@ -1164,25 +1164,35 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 		cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
 				   1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
 
-	if (slave->data->phy_node)
+	if (slave->data->phy_node) {
 		slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
 				 &cpsw_adjust_link, 0, slave->data->phy_if);
-	else
+		if (!slave->phy) {
+			dev_err(priv->dev, "phy \"%s\" not found on slave %d\n",
+				slave->data->phy_node->full_name,
+				slave->slave_num);
+			return;
+		}
+	} else {
 		slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
 				 &cpsw_adjust_link, slave->data->phy_if);
-	if (IS_ERR(slave->phy)) {
-		dev_err(priv->dev, "phy %s not found on slave %d\n",
-			slave->data->phy_id, slave->slave_num);
-		slave->phy = NULL;
-	} else {
-		dev_info(priv->dev, "phy found : id is : 0x%x\n",
-			 slave->phy->phy_id);
-		phy_start(slave->phy);
-
-		/* Configure GMII_SEL register */
-		cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface,
-			     slave->slave_num);
+		if (IS_ERR(slave->phy)) {
+			dev_err(priv->dev,
+				"phy \"%s\" not found on slave %d, err %ld\n",
+				slave->data->phy_id, slave->slave_num,
+				PTR_ERR(slave->phy));
+			slave->phy = NULL;
+			return;
+		}
 	}
+
+	dev_info(priv->dev, "phy found : id is : 0x%x\n",
+		 slave->phy->phy_id);
+
+	phy_start(slave->phy);
+
+	/* Configure GMII_SEL register */
+	cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface, slave->slave_num);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)

Signature · 1 line

2.19.0

Greg Kroah-Hartman

maintainer

2018-09-17 16:56 UTC #1 lore raw

Quoted 9 lines · SZ Lin (林上智)

Hi Greg,
This patch is not marked for 4.4-stable, but it's already in 4.9 and 4.14 stable.
Please apply to 4.4-stable.
This patch adds error handling to avoid segfault situation, I've tweaked patch to use original print function of dev_info() instead of phy_attached_info() which is in commit 2220943a21e26d97d7fd8f83c004b947326b469d upstream.

This patch does not apply against the latest 4.4.y tree. Can you please rebase/refresh it and resend?

thanks,

greg k-h

SZ Lin (林上智)

author

2018-09-20 15:29 UTC #2 lore raw

Hi Greg,

Quoted 5 lines · Greg Kroah-Hartman

Quoted 4 lines · SZ Lin (林上智)
Hi Greg,
This patch is not marked for 4.4-stable, but it's already in 4.9 and 4.14 stable.

Quoted 8 lines

Quoted 5 lines
Please apply to 4.4-stable.
This patch adds error handling to avoid segfault situation, I've tweaked patch to use original print function of dev_info() instead of phy_attached_info() which is in commit 2220943a21e26d97d7fd8f83c004b947326b469d upstream.
This patch does not apply against the latest 4.4.y tree. Can you please rebase/refresh it and resend?

I've rebased the attached patch based on 4.4.157, please apply it.

Thanks.

Quoted 4 lines

thanks,
greg k-h

Signature · 1 line

SZ Lin (林上智)

Quoted 1 line

From 76aa771afa9068449b4c1974bcb63e188d6bc180 Mon Sep 17 00:00:00 2001

From: David Rivshin <[email protected]>
Date: Wed, 27 Apr 2016 21:32:31 -0400
Subject: [PATCH] drivers: net: cpsw: fix segfault in case of bad phy-handle

commit d733f7542ad47cf73e033c90cf55158587e1d060 upstream

Fixes 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")

Signed-off-by David Rivshin <[email protected]>

Signed-off-by David S. Miller <[email protected]>

[SZ Lin (林上智): Tweak the patch to use original print function of dev_info()]

Signed-off-by SZ Lin (林上智) <[email protected]>

---
 drivers/net/ethernet/ti/cpsw.c | 37 +++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 14 deletions(-)

drivers/net/ethernet/ti/cpsw.c +23 ~~-14~~

index c2e110b2549b..c1217a87d535 100644
@@ -1164,25 +1164,34 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 		cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
 				   1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
 
-	if (slave->data->phy_node)
+	if (slave->data->phy_node) {
 		slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
 				 &cpsw_adjust_link, 0, slave->data->phy_if);
-	else
+		if (!slave->phy) {
+			dev_err(priv->dev, "phy \"%s\" not found on slave %d\n",
+				slave->data->phy_node->full_name,
+				slave->slave_num);
+			return;
+		}
+	} else {
 		slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
 				 &cpsw_adjust_link, slave->data->phy_if);
-	if (IS_ERR(slave->phy)) {
-		dev_err(priv->dev, "phy %s not found on slave %d\n",
-			slave->data->phy_id, slave->slave_num);
-		slave->phy = NULL;
-	} else {
-		dev_info(priv->dev, "phy found : id is : 0x%x\n",
-			 slave->phy->phy_id);
-		phy_start(slave->phy);
-
-		/* Configure GMII_SEL register */
-		cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface,
-			     slave->slave_num);
+		if (IS_ERR(slave->phy)) {
+			dev_err(priv->dev,
+				"phy \"%s\" not found on slave %d, err %ld\n",
+				slave->data->phy_id, slave->slave_num,
+				PTR_ERR(slave->phy));
+			slave->phy = NULL;
+			return;
+		}
 	}
+
+	dev_info(priv->dev, "phy found : id is : 0x%x\n", slave->phy->phy_id);
+
+	phy_start(slave->phy);
+
+	/* Configure GMII_SEL register */
+	cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface, slave->slave_num);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)

Signature · 1 line

2.19.0

Greg Kroah-Hartman

maintainer

2018-09-24 16:58 UTC #3 lore raw

Quoted 10 lines · SZ Lin (林上智)

Hi Greg,
Quoted 7 lines · Greg Kroah-Hartman
Quoted 6 lines · SZ Lin (林上智)
Hi Greg,
This patch is not marked for 4.4-stable, but it's already in 4.9 and 4.14 stable.
Please apply to 4.4-stable.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
This patch adds error handling to avoid segfault situation, I've tweaked patch to use original print function of dev_info() instead of phy_attached_info() which is in commit 2220943a21e26d97d7fd8f83c004b947326b469d upstream.
This patch does not apply against the latest 4.4.y tree. Can you please rebase/refresh it and resend?
I've rebased the attached patch based on 4.4.157, please apply it.

Thanks, that worked!

greg k-h