2023-11-04 12:44:11

by Linus Walleij

Subject: [PATCH net 3/4] net: ethernet: cortina: Protect against oversized frames

The max size of a transfer no matter the MTU is 64KB-1 so immediately
bail out if the skb exceeds that.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <[email protected]>
---
drivers/net/ethernet/cortina/gemini.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index fd08f098850b..23723c9c0f93 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1156,6 +1156,12 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
mtu = MTU_SIZE_BIT_MASK;
}

+ if (skb->len > 65535) {
+ /* The field for length is only 16 bits */
+ netdev_err(netdev, "%s: frame too big, max size 65535 bytes\n", __func__);
+ return -EINVAL;
+ }
+
word1 = skb->len;
word3 = SOF_BIT;


--
2.34.1

2023-11-04 14:46:40

by Andrew Lunn

Subject: Re: [PATCH net 3/4] net: ethernet: cortina: Protect against oversized frames

On Sat, Nov 04, 2023 at 01:43:50PM +0100, Linus Walleij wrote:
> The max size of a transfer no matter the MTU is 64KB-1 so immediately
> bail out if the skb exceeds that.
>
> Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> drivers/net/ethernet/cortina/gemini.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index fd08f098850b..23723c9c0f93 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -1156,6 +1156,12 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
> mtu = MTU_SIZE_BIT_MASK;
> }
>
> + if (skb->len > 65535) {
> + /* The field for length is only 16 bits */
> + netdev_err(netdev, "%s: frame too big, max size 65535 bytes\n", __func__);
> + return -EINVAL;
> + }
> +

The caller of gmac_map_tx_bufs() is a but funky:

if (gmac_map_tx_bufs(netdev, skb, txq, &w)) {
if (skb_linearize(skb))
goto out_drop;

u64_stats_update_begin(&port->tx_stats_syncp);
port->tx_frags_linearized++;
u64_stats_update_end(&port->tx_stats_syncp);

if (gmac_map_tx_bufs(netdev, skb, txq, &w))
goto out_drop_free;
}

So return -EINVAL is going to cause the skb to be linearised, and then
re-tried. Maybe you want to check the error code here, and go straight
to out_drop?

Andrew