* [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe
2019-01-16 10:23 [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup Johan Hovold
@ 2019-01-16 10:23 ` Johan Hovold
2019-01-16 15:00 ` Andrew Lunn
2019-01-16 21:42 ` Hauke Mehrtens
2019-01-16 10:23 ` [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups Johan Hovold
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Johan Hovold @ 2019-01-16 10:23 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel, Johan Hovold, stable
Make sure to disable and deregister the switch on late probe errors to
avoid use-after-free when the device-resource-managed switch is freed.
Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: stable <stable@vger.kernel.org> # 4.20
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/dsa/lantiq_gswip.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 693a67f45bef..b06c41c98de9 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1099,7 +1099,7 @@ static int gswip_probe(struct platform_device *pdev)
dev_err(dev, "wrong CPU port defined, HW only supports port: %i",
priv->hw_info->cpu_port);
err = -EINVAL;
- goto mdio_bus;
+ goto disable_switch;
}
platform_set_drvdata(pdev, priv);
@@ -1109,6 +1109,9 @@ static int gswip_probe(struct platform_device *pdev)
(version & GSWIP_VERSION_MOD_MASK) >> GSWIP_VERSION_MOD_SHIFT);
return 0;
+disable_switch:
+ gswip_mdio_mask(priv, GSWIP_MDIO_GLOB_ENABLE, 0, GSWIP_MDIO_GLOB);
+ dsa_unregister_switch(priv->ds);
mdio_bus:
if (mdio_np)
mdiobus_unregister(priv->ds->slave_mii_bus);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe
2019-01-16 10:23 ` [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe Johan Hovold
@ 2019-01-16 15:00 ` Andrew Lunn
2019-01-16 21:43 ` Hauke Mehrtens
2019-01-16 21:42 ` Hauke Mehrtens
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2019-01-16 15:00 UTC (permalink / raw)
To: Johan Hovold
Cc: Hauke Mehrtens, Vivien Didelot, Florian Fainelli,
David S. Miller, netdev, linux-kernel, stable
On Wed, Jan 16, 2019 at 11:23:33AM +0100, Johan Hovold wrote:
> Make sure to disable and deregister the switch on late probe errors to
> avoid use-after-free when the device-resource-managed switch is freed.
>
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Cc: stable <stable@vger.kernel.org> # 4.20
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> drivers/net/dsa/lantiq_gswip.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 693a67f45bef..b06c41c98de9 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1099,7 +1099,7 @@ static int gswip_probe(struct platform_device *pdev)
> dev_err(dev, "wrong CPU port defined, HW only supports port: %i",
> priv->hw_info->cpu_port);
> err = -EINVAL;
> - goto mdio_bus;
> + goto disable_switch;
> }
>
> platform_set_drvdata(pdev, priv);
> @@ -1109,6 +1109,9 @@ static int gswip_probe(struct platform_device *pdev)
> (version & GSWIP_VERSION_MOD_MASK) >> GSWIP_VERSION_MOD_SHIFT);
> return 0;
>
> +disable_switch:
> + gswip_mdio_mask(priv, GSWIP_MDIO_GLOB_ENABLE, 0, GSWIP_MDIO_GLOB);
> + dsa_unregister_switch(priv->ds);
This is correct. But it would be nice if somebody in the future could
move the disabling of the switch to the inverse of the gswip_setup()
function to make the code symmetrical.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe
2019-01-16 15:00 ` Andrew Lunn
@ 2019-01-16 21:43 ` Hauke Mehrtens
2019-01-16 22:15 ` Andrew Lunn
0 siblings, 1 reply; 13+ messages in thread
From: Hauke Mehrtens @ 2019-01-16 21:43 UTC (permalink / raw)
To: Andrew Lunn, Johan Hovold
Cc: Vivien Didelot, Florian Fainelli, David S. Miller, netdev,
linux-kernel, stable
On 1/16/19 4:00 PM, Andrew Lunn wrote:
> On Wed, Jan 16, 2019 at 11:23:33AM +0100, Johan Hovold wrote:
>> Make sure to disable and deregister the switch on late probe errors to
>> avoid use-after-free when the device-resource-managed switch is freed.
>>
>> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
>> Cc: stable <stable@vger.kernel.org> # 4.20
>> Cc: Hauke Mehrtens <hauke@hauke-m.de>
>> Signed-off-by: Johan Hovold <johan@kernel.org>
>> ---
>> drivers/net/dsa/lantiq_gswip.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
>> index 693a67f45bef..b06c41c98de9 100644
>> --- a/drivers/net/dsa/lantiq_gswip.c
>> +++ b/drivers/net/dsa/lantiq_gswip.c
>> @@ -1099,7 +1099,7 @@ static int gswip_probe(struct platform_device *pdev)
>> dev_err(dev, "wrong CPU port defined, HW only supports port: %i",
>> priv->hw_info->cpu_port);
>> err = -EINVAL;
>> - goto mdio_bus;
>> + goto disable_switch;
>> }
>>
>> platform_set_drvdata(pdev, priv);
>> @@ -1109,6 +1109,9 @@ static int gswip_probe(struct platform_device *pdev)
>> (version & GSWIP_VERSION_MOD_MASK) >> GSWIP_VERSION_MOD_SHIFT);
>> return 0;
>>
>> +disable_switch:
>> + gswip_mdio_mask(priv, GSWIP_MDIO_GLOB_ENABLE, 0, GSWIP_MDIO_GLOB);
>> + dsa_unregister_switch(priv->ds);
>
> This is correct. But it would be nice if somebody in the future could
> move the disabling of the switch to the inverse of the gswip_setup()
> function to make the code symmetrical.
Should we add an uninit callback?
Hauke
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe
2019-01-16 21:43 ` Hauke Mehrtens
@ 2019-01-16 22:15 ` Andrew Lunn
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-01-16 22:15 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Johan Hovold, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel, stable
> > This is correct. But it would be nice if somebody in the future could
> > move the disabling of the switch to the inverse of the gswip_setup()
> > function to make the code symmetrical.
>
> Should we add an uninit callback?
Yes, that would be good.
It looks like lan9303-core.c could use it as well for
lan9303_disable_processing(chip);
Thanks
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe
2019-01-16 10:23 ` [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe Johan Hovold
2019-01-16 15:00 ` Andrew Lunn
@ 2019-01-16 21:42 ` Hauke Mehrtens
1 sibling, 0 replies; 13+ messages in thread
From: Hauke Mehrtens @ 2019-01-16 21:42 UTC (permalink / raw)
To: Johan Hovold
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel, stable
On 1/16/19 11:23 AM, Johan Hovold wrote:
> Make sure to disable and deregister the switch on late probe errors to
> avoid use-after-free when the device-resource-managed switch is freed.
>
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Cc: stable <stable@vger.kernel.org> # 4.20
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> drivers/net/dsa/lantiq_gswip.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 693a67f45bef..b06c41c98de9 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1099,7 +1099,7 @@ static int gswip_probe(struct platform_device *pdev)
> dev_err(dev, "wrong CPU port defined, HW only supports port: %i",
> priv->hw_info->cpu_port);
> err = -EINVAL;
> - goto mdio_bus;
> + goto disable_switch;
> }
>
> platform_set_drvdata(pdev, priv);
> @@ -1109,6 +1109,9 @@ static int gswip_probe(struct platform_device *pdev)
> (version & GSWIP_VERSION_MOD_MASK) >> GSWIP_VERSION_MOD_SHIFT);
> return 0;
>
> +disable_switch:
> + gswip_mdio_mask(priv, GSWIP_MDIO_GLOB_ENABLE, 0, GSWIP_MDIO_GLOB);
> + dsa_unregister_switch(priv->ds);
> mdio_bus:
> if (mdio_np)
> mdiobus_unregister(priv->ds->slave_mii_bus);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups
2019-01-16 10:23 [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup Johan Hovold
2019-01-16 10:23 ` [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe Johan Hovold
@ 2019-01-16 10:23 ` Johan Hovold
2019-01-16 15:02 ` Andrew Lunn
2019-01-16 21:46 ` Hauke Mehrtens
2019-01-16 10:23 ` [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check Johan Hovold
2019-01-17 20:12 ` [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup David Miller
3 siblings, 2 replies; 13+ messages in thread
From: Johan Hovold @ 2019-01-16 10:23 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel, Johan Hovold, stable
Use the new of_get_compatible_child() helper to look up child nodes to
avoid ever matching non-child nodes elsewhere in the tree.
Also fix up the related struct device_node leaks.
Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: stable <stable@vger.kernel.org> # 4.20
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/dsa/lantiq_gswip.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index b06c41c98de9..5792674dd4e4 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1069,10 +1069,10 @@ static int gswip_probe(struct platform_device *pdev)
version = gswip_switch_r(priv, GSWIP_VERSION);
/* bring up the mdio bus */
- gphy_fw_np = of_find_compatible_node(pdev->dev.of_node, NULL,
- "lantiq,gphy-fw");
+ gphy_fw_np = of_get_compatible_child(dev->of_node, "lantiq,gphy-fw");
if (gphy_fw_np) {
err = gswip_gphy_fw_list(priv, gphy_fw_np, version);
+ of_node_put(gphy_fw_np);
if (err) {
dev_err(dev, "gphy fw probe failed\n");
return err;
@@ -1080,13 +1080,12 @@ static int gswip_probe(struct platform_device *pdev)
}
/* bring up the mdio bus */
- mdio_np = of_find_compatible_node(pdev->dev.of_node, NULL,
- "lantiq,xrx200-mdio");
+ mdio_np = of_get_compatible_child(dev->of_node, "lantiq,xrx200-mdio");
if (mdio_np) {
err = gswip_mdio(priv, mdio_np);
if (err) {
dev_err(dev, "mdio probe failed\n");
- goto gphy_fw;
+ goto put_mdio_node;
}
}
@@ -1115,7 +1114,8 @@ static int gswip_probe(struct platform_device *pdev)
mdio_bus:
if (mdio_np)
mdiobus_unregister(priv->ds->slave_mii_bus);
-gphy_fw:
+put_mdio_node:
+ of_node_put(mdio_np);
for (i = 0; i < priv->num_gphy_fw; i++)
gswip_gphy_fw_remove(priv, &priv->gphy_fw[i]);
return err;
@@ -1134,8 +1134,10 @@ static int gswip_remove(struct platform_device *pdev)
dsa_unregister_switch(priv->ds);
- if (priv->ds->slave_mii_bus)
+ if (priv->ds->slave_mii_bus) {
mdiobus_unregister(priv->ds->slave_mii_bus);
+ of_node_put(priv->ds->slave_mii_bus->dev.of_node);
+ }
for (i = 0; i < priv->num_gphy_fw; i++)
gswip_gphy_fw_remove(priv, &priv->gphy_fw[i]);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups
2019-01-16 10:23 ` [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups Johan Hovold
@ 2019-01-16 15:02 ` Andrew Lunn
2019-01-16 21:46 ` Hauke Mehrtens
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-01-16 15:02 UTC (permalink / raw)
To: Johan Hovold
Cc: Hauke Mehrtens, Vivien Didelot, Florian Fainelli,
David S. Miller, netdev, linux-kernel, stable
On Wed, Jan 16, 2019 at 11:23:34AM +0100, Johan Hovold wrote:
> Use the new of_get_compatible_child() helper to look up child nodes to
> avoid ever matching non-child nodes elsewhere in the tree.
>
> Also fix up the related struct device_node leaks.
>
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Cc: stable <stable@vger.kernel.org> # 4.20
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups
2019-01-16 10:23 ` [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups Johan Hovold
2019-01-16 15:02 ` Andrew Lunn
@ 2019-01-16 21:46 ` Hauke Mehrtens
1 sibling, 0 replies; 13+ messages in thread
From: Hauke Mehrtens @ 2019-01-16 21:46 UTC (permalink / raw)
To: Johan Hovold
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel, stable
On 1/16/19 11:23 AM, Johan Hovold wrote:
> Use the new of_get_compatible_child() helper to look up child nodes to
> avoid ever matching non-child nodes elsewhere in the tree.
>
> Also fix up the related struct device_node leaks.
>
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Cc: stable <stable@vger.kernel.org> # 4.20
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> drivers/net/dsa/lantiq_gswip.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check
2019-01-16 10:23 [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup Johan Hovold
2019-01-16 10:23 ` [PATCH 1/3] net: dsa: lantiq_gswip: fix use-after-free on failed probe Johan Hovold
2019-01-16 10:23 ` [PATCH 2/3] net: dsa: lantiq_gswip: fix OF child-node lookups Johan Hovold
@ 2019-01-16 10:23 ` Johan Hovold
2019-01-16 15:02 ` Andrew Lunn
2019-01-16 21:46 ` Hauke Mehrtens
2019-01-17 20:12 ` [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup David Miller
3 siblings, 2 replies; 13+ messages in thread
From: Johan Hovold @ 2019-01-16 10:23 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel, Johan Hovold
The platform-device driver data is set on successful probe and will
never be NULL on remove (or we have much bigger problems).
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/dsa/lantiq_gswip.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 5792674dd4e4..27d092cab40e 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1126,9 +1126,6 @@ static int gswip_remove(struct platform_device *pdev)
struct gswip_priv *priv = platform_get_drvdata(pdev);
int i;
- if (!priv)
- return 0;
-
/* disable the switch */
gswip_mdio_mask(priv, GSWIP_MDIO_GLOB_ENABLE, 0, GSWIP_MDIO_GLOB);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check
2019-01-16 10:23 ` [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check Johan Hovold
@ 2019-01-16 15:02 ` Andrew Lunn
2019-01-16 21:46 ` Hauke Mehrtens
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-01-16 15:02 UTC (permalink / raw)
To: Johan Hovold
Cc: Hauke Mehrtens, Vivien Didelot, Florian Fainelli,
David S. Miller, netdev, linux-kernel
On Wed, Jan 16, 2019 at 11:23:35AM +0100, Johan Hovold wrote:
> The platform-device driver data is set on successful probe and will
> never be NULL on remove (or we have much bigger problems).
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check
2019-01-16 10:23 ` [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check Johan Hovold
2019-01-16 15:02 ` Andrew Lunn
@ 2019-01-16 21:46 ` Hauke Mehrtens
1 sibling, 0 replies; 13+ messages in thread
From: Hauke Mehrtens @ 2019-01-16 21:46 UTC (permalink / raw)
To: Johan Hovold
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel
On 1/16/19 11:23 AM, Johan Hovold wrote:
> The platform-device driver data is set on successful probe and will
> never be NULL on remove (or we have much bigger problems).
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> drivers/net/dsa/lantiq_gswip.c | 3 ---
> 1 file changed, 3 deletions(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup
2019-01-16 10:23 [PATCH 0/3] net: dsa: lantiq_gswip: probe fixes and remove cleanup Johan Hovold
` (2 preceding siblings ...)
2019-01-16 10:23 ` [PATCH 3/3] net: dsa: lantiq_gswip: drop bogus drvdata check Johan Hovold
@ 2019-01-17 20:12 ` David Miller
3 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2019-01-17 20:12 UTC (permalink / raw)
To: johan; +Cc: hauke, andrew, vivien.didelot, f.fainelli, netdev, linux-kernel
From: Johan Hovold <johan@kernel.org>
Date: Wed, 16 Jan 2019 11:23:32 +0100
> This series fix a few issues found through inspection when fixing up new
> bad uses of of_find_compatible_node() that have crept in since 4.19.
>
> Note that these have only been compile tested.
Series applied.
^ permalink raw reply [flat|nested] 13+ messages in thread