2020-12-18 10:59:40

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH net v2 0/3] ucc_geth fixes

This is three bug fixes that fell out of a series of cleanups of the
ucc_geth driver. Please consider applying via the net tree.

v2: reorder and split off from larger series; add Andrew's R-b to
patch 1; only move the free_netdev() call in patch 3.

Rasmus Villemoes (3):
ethernet: ucc_geth: set dev->max_mtu to 1518
ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram
ethernet: ucc_geth: fix use-after-free in ucc_geth_remove()

drivers/net/ethernet/freescale/ucc_geth.c | 3 ++-
drivers/net/ethernet/freescale/ucc_geth.h | 9 ++++++++-
2 files changed, 10 insertions(+), 2 deletions(-)

--
2.23.0


2020-12-18 11:00:43

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH net v2 3/3] ethernet: ucc_geth: fix use-after-free in ucc_geth_remove()

ugeth is the netdiv_priv() part of the netdevice. Accessing the memory
pointed to by ugeth (such as done by ucc_geth_memclean() and the two
of_node_puts) after free_netdev() is thus use-after-free.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 380c1f09adaf..3f9fca061cc0 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3935,12 +3935,12 @@ static int ucc_geth_remove(struct platform_device* ofdev)
struct device_node *np = ofdev->dev.of_node;

unregister_netdev(dev);
- free_netdev(dev);
ucc_geth_memclean(ugeth);
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
of_node_put(ugeth->ug_info->tbi_node);
of_node_put(ugeth->ug_info->phy_node);
+ free_netdev(dev);

return 0;
}
--
2.23.0

2020-12-18 11:01:13

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH net v2 1/3] ethernet: ucc_geth: set dev->max_mtu to 1518

All the buffers and registers are already set up appropriately for an
MTU slightly above 1500, so we just need to expose this to the
networking stack. AFAICT, there's no need to implement .ndo_change_mtu
when the receive buffers are always set up to support the max_mtu.

This fixes several warnings during boot on our mpc8309-board with an
embedded mv88e6250 switch:

mv88e6085 mdio@e0102120:10: nonfatal error -34 setting MTU 1500 on port 0
...
mv88e6085 mdio@e0102120:10: nonfatal error -34 setting MTU 1500 on port 4
ucc_geth e0102000.ethernet eth1: error -22 setting MTU to 1504 to include DSA overhead

The last line explains what the DSA stack tries to do: achieving an MTU
of 1500 on-the-wire requires that the master netdevice connected to
the CPU port supports an MTU of 1500+the tagging overhead.

Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports")
Cc: Vladimir Oltean <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
---
drivers/net/ethernet/freescale/ucc_geth.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 714b501be7d0..380c1f09adaf 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3889,6 +3889,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
INIT_WORK(&ugeth->timeout_work, ucc_geth_timeout_work);
netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, 64);
dev->mtu = 1500;
+ dev->max_mtu = 1518;

ugeth->msg_enable = netif_msg_init(debug.msg_enable, UGETH_MSG_DEFAULT);
ugeth->phy_interface = phy_interface;
--
2.23.0

2020-12-20 08:08:44

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net v2 1/3] ethernet: ucc_geth: set dev->max_mtu to 1518

On Fri, Dec 18, 2020 at 11:55:36AM +0100, Rasmus Villemoes wrote:
> All the buffers and registers are already set up appropriately for an
> MTU slightly above 1500, so we just need to expose this to the
> networking stack. AFAICT, there's no need to implement .ndo_change_mtu
> when the receive buffers are always set up to support the max_mtu.
>
> This fixes several warnings during boot on our mpc8309-board with an
> embedded mv88e6250 switch:
>
> mv88e6085 mdio@e0102120:10: nonfatal error -34 setting MTU 1500 on port 0
> ...
> mv88e6085 mdio@e0102120:10: nonfatal error -34 setting MTU 1500 on port 4
> ucc_geth e0102000.ethernet eth1: error -22 setting MTU to 1504 to include DSA overhead
>
> The last line explains what the DSA stack tries to do: achieving an MTU
> of 1500 on-the-wire requires that the master netdevice connected to
> the CPU port supports an MTU of 1500+the tagging overhead.
>
> Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports")
> Cc: Vladimir Oltean <[email protected]>
> Reviewed-by: Andrew Lunn <[email protected]>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2020-12-22 01:42:11

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v2 0/3] ucc_geth fixes

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Fri, 18 Dec 2020 11:55:35 +0100 you wrote:
> This is three bug fixes that fell out of a series of cleanups of the
> ucc_geth driver. Please consider applying via the net tree.
>
> v2: reorder and split off from larger series; add Andrew's R-b to
> patch 1; only move the free_netdev() call in patch 3.
>
> Rasmus Villemoes (3):
> ethernet: ucc_geth: set dev->max_mtu to 1518
> ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram
> ethernet: ucc_geth: fix use-after-free in ucc_geth_remove()
>
> [...]

Here is the summary with links:
- [net,v2,1/3] ethernet: ucc_geth: set dev->max_mtu to 1518
https://git.kernel.org/netdev/net/c/1385ae5c30f2
- [net,v2,2/3] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram
https://git.kernel.org/netdev/net/c/887078de2a23
- [net,v2,3/3] ethernet: ucc_geth: fix use-after-free in ucc_geth_remove()
https://git.kernel.org/netdev/net/c/e925e0cd2a70

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html