From: bluhm <bluhm@openbsd.org>
Date: Wed, 14 Feb 2024 22:41:48 +0000 (+0000)
Subject: Check IP length in ether_extract_headers().
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ac5f541a6072f0b9e7989f8663c277a2e904f79f;p=openbsd

Check IP length in ether_extract_headers().

For LRO with ix(4) it is necessary to detect ethernet padding.
Extract ip_len and ip6_plen from the mbuf and provide it to the
drivers.
Add extended sanitity checks, like IP packet is shorter than TCP
header.  This prevents offloading to network hardware with bougus
packets.
Also iphlen of extracted headers contains header length for IPv4
and IPv6, to make code in drivers simpler.

OK mglocker@
---

diff --git a/sys/dev/pci/if_bnxt.c b/sys/dev/pci/if_bnxt.c
index d6d79e9a3dc..f067857e3ec 100644
--- a/sys/dev/pci/if_bnxt.c
+++ b/sys/dev/pci/if_bnxt.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_bnxt.c,v 1.46 2024/02/13 13:58:19 bluhm Exp $	*/
+/*	$OpenBSD: if_bnxt.c,v 1.47 2024/02/14 22:41:48 bluhm Exp $	*/
 /*-
  * Broadcom NetXtreme-C/E network driver.
  *
@@ -1432,10 +1432,8 @@ bnxt_start(struct ifqueue *ifq)
 			if (ext.tcp) {
 				lflags |= TX_BD_LONG_LFLAGS_LSO;
 				hdrsize = sizeof(*ext.eh);
-				if (ext.ip4)
-					hdrsize += ext.ip4hlen;
-				else if (ext.ip6)
-					hdrsize += sizeof(*ext.ip6);
+				if (ext.ip4 || ext.ip6)
+					hdrsize += ext.iphlen;
 				else
 					tcpstat_inc(tcps_outbadtso);
 
diff --git a/sys/dev/pci/if_em.c b/sys/dev/pci/if_em.c
index 18a4ce3f2e9..d86f453bdbb 100644
--- a/sys/dev/pci/if_em.c
+++ b/sys/dev/pci/if_em.c
@@ -31,7 +31,7 @@ POSSIBILITY OF SUCH DAMAGE.
 
 ***************************************************************************/
 
-/* $OpenBSD: if_em.c,v 1.372 2024/02/13 13:58:19 bluhm Exp $ */
+/* $OpenBSD: if_em.c,v 1.373 2024/02/14 22:41:48 bluhm Exp $ */
 /* $FreeBSD: if_em.c,v 1.46 2004/09/29 18:28:28 mlaier Exp $ */
 
 #include <dev/pci/if_em.h>
@@ -2413,7 +2413,6 @@ em_tx_ctx_setup(struct em_queue *que, struct mbuf *mp, u_int head,
 	struct e1000_adv_tx_context_desc *TD;
 	uint32_t vlan_macip_lens = 0, type_tucmd_mlhl = 0, mss_l4len_idx = 0;
 	int off = 0;
-	uint8_t iphlen;
 
 	*olinfo_status = 0;
 	*cmd_type_len = 0;
@@ -2433,8 +2432,6 @@ em_tx_ctx_setup(struct em_queue *que, struct mbuf *mp, u_int head,
 	vlan_macip_lens |= (sizeof(*ext.eh) << E1000_ADVTXD_MACLEN_SHIFT);
 
 	if (ext.ip4) {
-		iphlen = ext.ip4hlen;
-
 		type_tucmd_mlhl |= E1000_ADVTXD_TUCMD_IPV4;
 		if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
 			*olinfo_status |= E1000_TXD_POPTS_IXSM << 8;
@@ -2442,18 +2439,14 @@ em_tx_ctx_setup(struct em_queue *que, struct mbuf *mp, u_int head,
 		}
 #ifdef INET6
 	} else if (ext.ip6) {
-		iphlen = sizeof(*ext.ip6);
-
 		type_tucmd_mlhl |= E1000_ADVTXD_TUCMD_IPV6;
 #endif
-	} else {
-		iphlen = 0;
 	}
 
 	*cmd_type_len |= E1000_ADVTXD_DTYP_DATA | E1000_ADVTXD_DCMD_IFCS;
 	*cmd_type_len |= E1000_ADVTXD_DCMD_DEXT;
 	*olinfo_status |= mp->m_pkthdr.len << E1000_ADVTXD_PAYLEN_SHIFT;
-	vlan_macip_lens |= iphlen;
+	vlan_macip_lens |= ext.iphlen;
 	type_tucmd_mlhl |= E1000_ADVTXD_DCMD_DEXT | E1000_ADVTXD_DTYP_CTXT;
 
 	if (ext.tcp) {
diff --git a/sys/dev/pci/if_igc.c b/sys/dev/pci/if_igc.c
index 952a9209efe..b7bc205b666 100644
--- a/sys/dev/pci/if_igc.c
+++ b/sys/dev/pci/if_igc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_igc.c,v 1.16 2024/02/13 13:58:19 bluhm Exp $	*/
+/*	$OpenBSD: if_igc.c,v 1.17 2024/02/14 22:41:48 bluhm Exp $	*/
 /*-
  * SPDX-License-Identifier: BSD-2-Clause
  *
@@ -2005,7 +2005,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
 	struct igc_adv_tx_context_desc *txdesc;
 	uint32_t type_tucmd_mlhl = 0;
 	uint32_t vlan_macip_lens = 0;
-	uint32_t iphlen;
 	int off = 0;
 
 	vlan_macip_lens |= (sizeof(*ext.eh) << IGC_ADVTXD_MACLEN_SHIFT);
@@ -2028,8 +2027,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
 	ether_extract_headers(mp, &ext);
 
 	if (ext.ip4) {
-		iphlen = ext.ip4hlen;
-
 		type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV4;
 		if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
 			*olinfo_status |= IGC_TXD_POPTS_IXSM << 8;
@@ -2037,15 +2034,13 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
 		}
 #ifdef INET6
 	} else if (ext.ip6) {
-		iphlen = sizeof(*ext.ip6);
-
 		type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV6;
 #endif
 	} else {
 		return 0;
 	}
 
-	vlan_macip_lens |= iphlen;
+	vlan_macip_lens |= ext.iphlen;
 	type_tucmd_mlhl |= IGC_ADVTXD_DCMD_DEXT | IGC_ADVTXD_DTYP_CTXT;
 
 	if (ext.tcp) {
diff --git a/sys/dev/pci/if_ix.c b/sys/dev/pci/if_ix.c
index 87a6bfef88d..ddbfbe3a7da 100644
--- a/sys/dev/pci/if_ix.c
+++ b/sys/dev/pci/if_ix.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ix.c,v 1.207 2024/02/13 13:58:19 bluhm Exp $	*/
+/*	$OpenBSD: if_ix.c,v 1.208 2024/02/14 22:41:48 bluhm Exp $	*/
 
 /******************************************************************************
 
@@ -2494,16 +2494,12 @@ ixgbe_tx_offload(struct mbuf *mp, uint32_t *vlan_macip_lens,
 {
 	struct ether_extracted ext;
 	int offload = 0;
-	uint32_t ethlen, iphlen;
 
 	ether_extract_headers(mp, &ext);
-	ethlen = sizeof(*ext.eh);
 
-	*vlan_macip_lens |= (ethlen << IXGBE_ADVTXD_MACLEN_SHIFT);
+	*vlan_macip_lens |= (sizeof(*ext.eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
 
 	if (ext.ip4) {
-		iphlen = ext.ip4hlen;
-
 		if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
 			*olinfo_status |= IXGBE_TXD_POPTS_IXSM << 8;
 			offload = 1;
@@ -2512,8 +2508,6 @@ ixgbe_tx_offload(struct mbuf *mp, uint32_t *vlan_macip_lens,
 		*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV4;
 #ifdef INET6
 	} else if (ext.ip6) {
-		iphlen = sizeof(*ext.ip6);
-
 		*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV6;
 #endif
 	} else {
@@ -2522,7 +2516,7 @@ ixgbe_tx_offload(struct mbuf *mp, uint32_t *vlan_macip_lens,
 		return offload;
 	}
 
-	*vlan_macip_lens |= iphlen;
+	*vlan_macip_lens |= ext.iphlen;
 
 	if (ext.tcp) {
 		*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_TCP;
@@ -2548,7 +2542,7 @@ ixgbe_tx_offload(struct mbuf *mp, uint32_t *vlan_macip_lens,
 			*mss_l4len_idx |= outlen << IXGBE_ADVTXD_MSS_SHIFT;
 			*mss_l4len_idx |= thlen << IXGBE_ADVTXD_L4LEN_SHIFT;
 
-			hdrlen = ethlen + iphlen + thlen;
+			hdrlen = sizeof(*ext.eh) + ext.iphlen + thlen;
 			paylen = mp->m_pkthdr.len - hdrlen;
 			CLR(*olinfo_status, IXGBE_ADVTXD_PAYLEN_MASK
 			    << IXGBE_ADVTXD_PAYLEN_SHIFT);
@@ -3276,10 +3270,8 @@ ixgbe_rxeof(struct rx_ring *rxr)
 				    ext.evh)
 					hdrlen += ETHER_VLAN_ENCAP_LEN;
 #endif
-				if (ext.ip4)
-					hdrlen += ext.ip4hlen;
-				if (ext.ip6)
-					hdrlen += sizeof(*ext.ip6);
+				if (ext.ip4 || ext.ip6)
+					hdrlen += ext.iphlen;
 				if (ext.tcp) {
 					hdrlen += ext.tcphlen;
 					tcpstat_inc(tcps_inhwlro);
diff --git a/sys/dev/pci/if_ixl.c b/sys/dev/pci/if_ixl.c
index 153f3677d39..3667c6bff9b 100644
--- a/sys/dev/pci/if_ixl.c
+++ b/sys/dev/pci/if_ixl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ixl.c,v 1.96 2024/02/13 13:58:19 bluhm Exp $ */
+/*	$OpenBSD: if_ixl.c,v 1.97 2024/02/14 22:41:48 bluhm Exp $ */
 
 /*
  * Copyright (c) 2013-2015, Intel Corporation
@@ -2826,18 +2826,15 @@ ixl_tx_setup_offload(struct mbuf *m0, struct ixl_tx_ring *txr,
 		offload |= ISSET(m0->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) ?
 		    IXL_TX_DESC_CMD_IIPT_IPV4_CSUM :
 		    IXL_TX_DESC_CMD_IIPT_IPV4;
- 
-		hlen = ext.ip4hlen;
 #ifdef INET6
 	} else if (ext.ip6) {
 		offload |= IXL_TX_DESC_CMD_IIPT_IPV6;
-
-		hlen = sizeof(*ext.ip6);
 #endif
 	} else {
 		panic("CSUM_OUT set for non-IP packet");
 		/* NOTREACHED */
 	}
+	hlen = ext.iphlen;
 
 	offload |= (ETHER_HDR_LEN >> 1) << IXL_TX_DESC_MACLEN_SHIFT;
 	offload |= (hlen >> 2) << IXL_TX_DESC_IPLEN_SHIFT;
diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c
index 3648e4ac3f0..db399e7190d 100644
--- a/sys/dev/pv/if_vio.c
+++ b/sys/dev/pv/if_vio.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_vio.c,v 1.30 2024/02/13 13:58:19 bluhm Exp $	*/
+/*	$OpenBSD: if_vio.c,v 1.31 2024/02/14 22:41:48 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2012 Stefan Fritsch, Alexander Fiveg.
@@ -764,12 +764,8 @@ again:
 			else
 				hdr->csum_offset = offsetof(struct udphdr, uh_sum);
 
-			if (ext.ip4)
-				hdr->csum_start += ext.ip4hlen;
-#ifdef INET6
-			else if (ext.ip6)
-				hdr->csum_start += sizeof(*ext.ip6);
-#endif
+			if (ext.ip4 || ext.ip6)
+				hdr->csum_start += ext.iphlen;
 			hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
 		}
 
diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
index 2d732535ca0..a7c6d5b1efe 100644
--- a/sys/net/if_ethersubr.c
+++ b/sys/net/if_ethersubr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ethersubr.c,v 1.292 2024/02/13 13:58:19 bluhm Exp $	*/
+/*	$OpenBSD: if_ethersubr.c,v 1.293 2024/02/14 22:41:48 bluhm Exp $	*/
 /*	$NetBSD: if_ethersubr.c,v 1.19 1996/05/07 02:40:30 thorpej Exp $	*/
 
 /*
@@ -1051,7 +1051,7 @@ void
 ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext)
 {
 	struct mbuf	*m;
-	size_t		 hlen;
+	size_t		 hlen, iplen;
 	int		 hoff;
 	uint8_t		 ipproto;
 	uint16_t	 ether_type;
@@ -1143,7 +1143,19 @@ ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext)
 			ext->ip4 = NULL;
 			return;
 		}
-		ext->ip4hlen = hlen;
+		iplen = ntohs(ext->ip4->ip_len);
+		if (ext->paylen < iplen) {
+			DPRINTF("paylen %u, ip4len %zu", ext->paylen, iplen);
+			ext->ip4 = NULL;
+			return;
+		}
+		if (iplen < hlen) {
+			DPRINTF("ip4len %zu, ip4hlen %zu", iplen, hlen);
+			ext->ip4 = NULL;
+			return;
+		}
+		ext->iplen = iplen;
+		ext->iphlen = hlen;
 		ext->paylen -= hlen;
 		ipproto = ext->ip4->ip_p;
 
@@ -1166,6 +1178,14 @@ ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext)
 			ext->ip6 = NULL;
 			return;
 		}
+		iplen = hlen + ntohs(ext->ip6->ip6_plen);
+		if (ext->paylen < iplen) {
+			DPRINTF("paylen %u, ip6len %zu", ext->paylen, iplen);
+			ext->ip6 = NULL;
+			return;
+		}
+		ext->iplen = iplen;
+		ext->iphlen = hlen;
 		ext->paylen -= hlen;
 		ipproto = ext->ip6->ip6_nxt;
 		break;
@@ -1192,8 +1212,9 @@ ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext)
 			ext->tcp = NULL;
 			return;
 		}
-		if (ext->paylen < hlen) {
-			DPRINTF("paylen %u, tcphlen %zu", ext->paylen, hlen);
+		if (ext->iplen - ext->iphlen < hlen) {
+			DPRINTF("iplen %u, iphlen %u, tcphlen %zu",
+			    ext->iplen, ext->iphlen, hlen);
 			ext->tcp = NULL;
 			return;
 		}
@@ -1211,17 +1232,18 @@ ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext)
 		ext->udp = (struct udphdr *)(mtod(m, caddr_t) + hoff);
 
 		hlen = sizeof(*ext->udp);
-		if (ext->paylen < hlen) {
-			DPRINTF("paylen %u, udphlen %zu", ext->paylen, hlen);
+		if (ext->iplen - ext->iphlen < hlen) {
+			DPRINTF("iplen %u, iphlen %u, udphlen %zu",
+			    ext->iplen, ext->iphlen, hlen);
 			ext->udp = NULL;
 			return;
 		}
 		break;
 	}
 
-	DNPRINTF(2, "%s%s%s%s%s%s ip4h %u, tcph %u, payl %u",
+	DNPRINTF(2, "%s%s%s%s%s%s ip %u, iph %u, tcph %u, payl %u",
 	    ext->eh ? "eh," : "", ext->evh ? "evh," : "",
 	    ext->ip4 ? "ip4," : "", ext->ip6 ? "ip6," : "",
 	    ext->tcp ? "tcp," : "", ext->udp ? "udp," : "",
-	    ext->ip4hlen, ext->tcphlen, ext->paylen);
+	    ext->iplen, ext->iphlen, ext->tcphlen, ext->paylen);
 }
diff --git a/sys/netinet/if_ether.h b/sys/netinet/if_ether.h
index 4f5edd31b14..f4240e8c4ec 100644
--- a/sys/netinet/if_ether.h
+++ b/sys/netinet/if_ether.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ether.h,v 1.91 2024/02/13 13:58:19 bluhm Exp $	*/
+/*	$OpenBSD: if_ether.h,v 1.92 2024/02/14 22:41:48 bluhm Exp $	*/
 /*	$NetBSD: if_ether.h,v 1.22 1996/05/11 13:00:00 mycroft Exp $	*/
 
 /*
@@ -307,7 +307,8 @@ struct ether_extracted {
 	struct ip6_hdr			*ip6;
 	struct tcphdr			*tcp;
 	struct udphdr			*udp;
-	u_int				 ip4hlen;
+	u_int				 iplen;
+	u_int				 iphlen;
 	u_int				 tcphlen;
 	u_int				 paylen;
 };