[PATCH net] net: atl1c: drop oversized RX frames instead of panicking

[PATCH net] net: atl1c: drop oversized RX frames instead of panicking

[Federico Musto]

atl1c sizes its receive buffers from the netdev MTU plus a fixed set of
L2 overheads, which comes to 1522 bytes at the default MTU.  A link
partner that sends a longer frame - in practice a MikroTik
CCR2004-1G-2XS-PCIe, whose RouterOS side defaults to l2mtu 1600 while
the host stays at MTU 1500 - makes the length reported by the RX return
descriptor exceed skb_tailroom(), and skb_put() then trips
skb_over_panic and takes the host down.

Validate the descriptor length against skb_tailroom() before calling
skb_put() and drop, counting it in rx_length_errors/rx_dropped, any
frame that does not fit.  Such a frame cannot be delivered with the
current MTU anyway - the buffers were sized for it - but losing the
frame is preferable to losing the host.

Tested on a MikroTik CCR2004-1G-2XS-PCIe (4x AR8151) in a Proxmox VE
host, with atl1c built from a DKMS tree carrying this change: the
l2mtu-1600 traffic that panics the stock driver is dropped and counted
with the patch applied, and received normally once the host MTU is
raised to match.

This complements 36c2009d90f2 ("net: atl1c: fix soft lockup on
out-of-range tpd_cons read"), which fixed the TX side of the same
"host dies while the card resets" problem.

Fixes: 43250ddd75a3 ("atl1c: Atheros L1C Gigabit Ethernet driver")
Cc: stable@vger.kernel.org
Signed-off-by: Federico Musto <federico@rimpianto.com>
---
Notes for reviewers (not part of the commit message):

This is my first kernel patch, so process nitpicks are as welcome as
technical ones.

Scope: this is the minimal version of the change - only the descriptor
bounds check.  My local patch also touched atl1c_set_rxbufsize() and
atl1c_change_mtu(); both are independent of the panic and I can send
them separately if there is interest (raising the host MTU to 1600 is
what actually lets such frames through, and the driver handles that
today without changes).

Reproduction data, kernel logs and the DKMS package used for testing:
https://github.com/rimpianto/mikrotik.CCR2004-1G-2XS-PCIe
---
 .../net/ethernet/atheros/atl1c/atl1c_main.c    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index e58f1d2..11de643 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -1934,6 +1934,24 @@ rrs_checked:
 			break;
 		}
 		atl1c_clean_rfd(rfd_ring, rrs, rfd_num);
+
+		/* A peer with a larger L2 MTU than ours can hand us a frame
+		 * that does not fit the buffer allocated for this MTU.  The
+		 * driver used to skb_put() it anyway, which trips
+		 * skb_over_panic and takes the host down; drop it instead.
+		 */
+		if (unlikely(length < ETH_HLEN + ETH_FCS_LEN) ||
+		    unlikely(length - ETH_FCS_LEN > skb_tailroom(skb))) {
+			netdev->stats.rx_length_errors++;
+			netdev->stats.rx_dropped++;
+			if (netif_msg_rx_err(adapter))
+				dev_warn_ratelimited(&pdev->dev,
+						     "bad rx frame: len %u tailroom %d\n",
+						     length, skb_tailroom(skb));
+			dev_kfree_skb(skb);
+			continue;
+		}
+
 		skb_put(skb, length - ETH_FCS_LEN);
 		skb->protocol = eth_type_trans(skb, netdev);
 		atl1c_rx_checksum(adapter, skb, rrs);
-- 
2.54.0 (Apple Git-157)