From: bluhm Date: Tue, 13 Feb 2024 13:58:19 +0000 (+0000) Subject: Analyse header layout in ether_extract_headers(). X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e78a66e5775d3e9cf59b16f193be327ae6d56426;p=openbsd Analyse header layout in ether_extract_headers(). Several drivers need IPv4 header length and TCP offset for checksum offload, TSO and LRO. Accessing these fields directly caused crashes on sparc64 due to misaligned access. It cannot be guaranteed that IP and TCP header is 4 byte aligned in driver level. Also gcc 4.2.1 assumes that bit fields can be accessed with 32 bit load instructions. Use memcpy() in ether_extract_headers() to get the bits from IPv4 and TCP header and store the header length in struct ether_extracted. From there network drivers can esily use it without caring about alignment and bit shift. Do some sanity checks with the length values to prevent that invalid values from evil packets get stored into hardware registers. If check fails, clear the pointer to the header to hide it from the driver. Add debug prints that help to figure out the reason for bad packets and provide information when debugging drivers. OK mglocker@ --- diff --git a/sys/dev/pci/if_bnxt.c b/sys/dev/pci/if_bnxt.c index 4771121e096..d6d79e9a3dc 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.45 2024/01/19 03:25:13 jmatthew Exp $ */ +/* $OpenBSD: if_bnxt.c,v 1.46 2024/02/13 13:58:19 bluhm Exp $ */ /*- * Broadcom NetXtreme-C/E network driver. * @@ -1433,13 +1433,13 @@ bnxt_start(struct ifqueue *ifq) lflags |= TX_BD_LONG_LFLAGS_LSO; hdrsize = sizeof(*ext.eh); if (ext.ip4) - hdrsize += ext.ip4->ip_hl << 2; + hdrsize += ext.ip4hlen; else if (ext.ip6) hdrsize += sizeof(*ext.ip6); else tcpstat_inc(tcps_outbadtso); - hdrsize += ext.tcp->th_off << 2; + hdrsize += ext.tcphlen; txhi->hdr_size = htole16(hdrsize / 2); outlen = m->m_pkthdr.ph_mss; diff --git a/sys/dev/pci/if_em.c b/sys/dev/pci/if_em.c index 2c668af926c..18a4ce3f2e9 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.371 2024/01/28 18:42:58 mglocker Exp $ */ +/* $OpenBSD: if_em.c,v 1.372 2024/02/13 13:58:19 bluhm Exp $ */ /* $FreeBSD: if_em.c,v 1.46 2004/09/29 18:28:28 mlaier Exp $ */ #include @@ -2433,7 +2433,7 @@ 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.ip4->ip_hl << 2; + iphlen = ext.ip4hlen; type_tucmd_mlhl |= E1000_ADVTXD_TUCMD_IPV4; if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) { diff --git a/sys/dev/pci/if_igc.c b/sys/dev/pci/if_igc.c index 5deb53df970..952a9209efe 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.15 2024/01/23 08:48:12 kevlo Exp $ */ +/* $OpenBSD: if_igc.c,v 1.16 2024/02/13 13:58:19 bluhm Exp $ */ /*- * SPDX-License-Identifier: BSD-2-Clause * @@ -2028,7 +2028,7 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod, ether_extract_headers(mp, &ext); if (ext.ip4) { - iphlen = ext.ip4->ip_hl << 2; + iphlen = ext.ip4hlen; type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV4; if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) { diff --git a/sys/dev/pci/if_ix.c b/sys/dev/pci/if_ix.c index 9b838f49e34..87a6bfef88d 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.206 2023/11/10 15:51:20 bluhm Exp $ */ +/* $OpenBSD: if_ix.c,v 1.207 2024/02/13 13:58:19 bluhm Exp $ */ /****************************************************************************** @@ -2502,7 +2502,7 @@ ixgbe_tx_offload(struct mbuf *mp, uint32_t *vlan_macip_lens, *vlan_macip_lens |= (ethlen << IXGBE_ADVTXD_MACLEN_SHIFT); if (ext.ip4) { - iphlen = ext.ip4->ip_hl << 2; + iphlen = ext.ip4hlen; if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) { *olinfo_status |= IXGBE_TXD_POPTS_IXSM << 8; @@ -2542,7 +2542,7 @@ ixgbe_tx_offload(struct mbuf *mp, uint32_t *vlan_macip_lens, if (ext.tcp) { uint32_t hdrlen, thlen, paylen, outlen; - thlen = ext.tcp->th_off << 2; + thlen = ext.tcphlen; outlen = mp->m_pkthdr.ph_mss; *mss_l4len_idx |= outlen << IXGBE_ADVTXD_MSS_SHIFT; @@ -3277,11 +3277,11 @@ ixgbe_rxeof(struct rx_ring *rxr) hdrlen += ETHER_VLAN_ENCAP_LEN; #endif if (ext.ip4) - hdrlen += ext.ip4->ip_hl << 2; + hdrlen += ext.ip4hlen; if (ext.ip6) hdrlen += sizeof(*ext.ip6); if (ext.tcp) { - hdrlen += ext.tcp->th_off << 2; + hdrlen += ext.tcphlen; tcpstat_inc(tcps_inhwlro); tcpstat_add(tcps_inpktlro, pkts); } else { diff --git a/sys/dev/pci/if_ixl.c b/sys/dev/pci/if_ixl.c index 1f89dd73e87..153f3677d39 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.95 2024/01/07 21:01:45 bluhm Exp $ */ +/* $OpenBSD: if_ixl.c,v 1.96 2024/02/13 13:58:19 bluhm Exp $ */ /* * Copyright (c) 2013-2015, Intel Corporation @@ -2827,7 +2827,7 @@ ixl_tx_setup_offload(struct mbuf *m0, struct ixl_tx_ring *txr, IXL_TX_DESC_CMD_IIPT_IPV4_CSUM : IXL_TX_DESC_CMD_IIPT_IPV4; - hlen = ext.ip4->ip_hl << 2; + hlen = ext.ip4hlen; #ifdef INET6 } else if (ext.ip6) { offload |= IXL_TX_DESC_CMD_IIPT_IPV6; @@ -2844,10 +2844,12 @@ ixl_tx_setup_offload(struct mbuf *m0, struct ixl_tx_ring *txr, if (ext.tcp && ISSET(m0->m_pkthdr.csum_flags, M_TCP_CSUM_OUT)) { offload |= IXL_TX_DESC_CMD_L4T_EOFT_TCP; - offload |= (uint64_t)ext.tcp->th_off << IXL_TX_DESC_L4LEN_SHIFT; + offload |= (uint64_t)(ext.tcphlen >> 2) + << IXL_TX_DESC_L4LEN_SHIFT; } else if (ext.udp && ISSET(m0->m_pkthdr.csum_flags, M_UDP_CSUM_OUT)) { offload |= IXL_TX_DESC_CMD_L4T_EOFT_UDP; - offload |= (sizeof(*ext.udp) >> 2) << IXL_TX_DESC_L4LEN_SHIFT; + offload |= (uint64_t)(sizeof(*ext.udp) >> 2) + << IXL_TX_DESC_L4LEN_SHIFT; } if (ISSET(m0->m_pkthdr.csum_flags, M_TCP_TSO)) { @@ -2855,7 +2857,7 @@ ixl_tx_setup_offload(struct mbuf *m0, struct ixl_tx_ring *txr, struct ixl_tx_desc *ring, *txd; uint64_t cmd = 0, paylen, outlen; - hlen += ext.tcp->th_off << 2; + hlen += ext.tcphlen; outlen = m0->m_pkthdr.ph_mss; paylen = m0->m_pkthdr.len - ETHER_HDR_LEN - hlen; diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c index 550c4deb9f3..3648e4ac3f0 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.29 2023/12/20 09:51:06 jan Exp $ */ +/* $OpenBSD: if_vio.c,v 1.30 2024/02/13 13:58:19 bluhm Exp $ */ /* * Copyright (c) 2012 Stefan Fritsch, Alexander Fiveg. @@ -765,7 +765,7 @@ again: hdr->csum_offset = offsetof(struct udphdr, uh_sum); if (ext.ip4) - hdr->csum_start += ext.ip4->ip_hl << 2; + hdr->csum_start += ext.ip4hlen; #ifdef INET6 else if (ext.ip6) hdr->csum_start += sizeof(*ext.ip6); diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c index a77a0d6543c..2d732535ca0 100644 --- a/sys/net/if_ethersubr.c +++ b/sys/net/if_ethersubr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_ethersubr.c,v 1.291 2023/07/27 20:21:25 jan Exp $ */ +/* $OpenBSD: if_ethersubr.c,v 1.292 2024/02/13 13:58:19 bluhm Exp $ */ /* $NetBSD: if_ethersubr.c,v 1.19 1996/05/07 02:40:30 thorpej Exp $ */ /* @@ -140,6 +140,20 @@ didn't get a copy, you may request one from . #include #endif /* MPLS */ +/* #define ETHERDEBUG 1 */ +#ifdef ETHERDEBUG +int etherdebug = ETHERDEBUG; +#define DNPRINTF(level, fmt, args...) \ + do { \ + if (etherdebug >= level) \ + printf("%s: " fmt "\n", __func__, ## args); \ + } while (0) +#else +#define DNPRINTF(level, fmt, args...) \ + do { } while (0) +#endif +#define DPRINTF(fmt, args...) DNPRINTF(1, fmt, args) + u_int8_t etherbroadcastaddr[ETHER_ADDR_LEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; u_int8_t etheranyaddr[ETHER_ADDR_LEN] = @@ -1034,56 +1048,126 @@ ether_e64_to_addr(struct ether_addr *ea, uint64_t e64) /* Parse different TCP/IP protocol headers for a quick view inside an mbuf. */ void -ether_extract_headers(struct mbuf *mp, struct ether_extracted *ext) +ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext) { struct mbuf *m; - uint64_t hlen; + size_t hlen; int hoff; uint8_t ipproto; uint16_t ether_type; + /* gcc 4.2.1 on sparc64 may create 32 bit loads on unaligned mbuf */ + union { + u_char hc_data; +#if _BYTE_ORDER == _LITTLE_ENDIAN + struct { + u_int hl:4, /* header length */ + v:4; /* version */ + } hc_ip; + struct { + u_int x2:4, /* (unused) */ + off:4; /* data offset */ + } hc_th; +#endif +#if _BYTE_ORDER == _BIG_ENDIAN + struct { + u_int v:4, /* version */ + hl:4; /* header length */ + } hc_ip; + struct { + u_int off:4, /* data offset */ + x2:4; /* (unused) */ + } hc_th; +#endif + } hdrcpy; /* Return NULL if header was not recognized. */ memset(ext, 0, sizeof(*ext)); - if (mp->m_len < sizeof(*ext->eh)) - return; + KASSERT(ISSET(m0->m_flags, M_PKTHDR)); + ext->paylen = m0->m_pkthdr.len; - ext->eh = mtod(mp, struct ether_header *); + if (m0->m_len < sizeof(*ext->eh)) { + DPRINTF("m_len %d, eh %zu", m0->m_len, sizeof(*ext->eh)); + return; + } + ext->eh = mtod(m0, struct ether_header *); ether_type = ntohs(ext->eh->ether_type); hlen = sizeof(*ext->eh); + if (ext->paylen < hlen) { + DPRINTF("paylen %u, ehlen %zu", ext->paylen, hlen); + ext->eh = NULL; + return; + } + ext->paylen -= hlen; #if NVLAN > 0 if (ether_type == ETHERTYPE_VLAN) { - ext->evh = mtod(mp, struct ether_vlan_header *); + if (m0->m_len < sizeof(*ext->evh)) { + DPRINTF("m_len %d, evh %zu", + m0->m_len, sizeof(*ext->evh)); + return; + } + ext->evh = mtod(m0, struct ether_vlan_header *); ether_type = ntohs(ext->evh->evl_proto); hlen = sizeof(*ext->evh); + if (sizeof(*ext->eh) + ext->paylen < hlen) { + DPRINTF("paylen %zu, evhlen %zu", + sizeof(*ext->eh) + ext->paylen, hlen); + ext->evh = NULL; + return; + } + ext->paylen = sizeof(*ext->eh) + ext->paylen - hlen; } #endif switch (ether_type) { case ETHERTYPE_IP: - m = m_getptr(mp, hlen, &hoff); - if (m == NULL || m->m_len - hoff < sizeof(*ext->ip4)) + m = m_getptr(m0, hlen, &hoff); + if (m == NULL || m->m_len - hoff < sizeof(*ext->ip4)) { + DPRINTF("m_len %d, hoff %d, ip4 %zu", + m ? m->m_len : -1, hoff, sizeof(*ext->ip4)); return; + } ext->ip4 = (struct ip *)(mtod(m, caddr_t) + hoff); - if (ISSET(ntohs(ext->ip4->ip_off), IP_MF|IP_OFFMASK)) + memcpy(&hdrcpy.hc_data, ext->ip4, 1); + hlen = hdrcpy.hc_ip.hl << 2; + if (m->m_len - hoff < hlen) { + DPRINTF("m_len %d, hoff %d, iphl %zu", + m ? m->m_len : -1, hoff, hlen); + ext->ip4 = NULL; return; - - hlen = ext->ip4->ip_hl << 2; + } + if (ext->paylen < hlen) { + DPRINTF("paylen %u, ip4hlen %zu", ext->paylen, hlen); + ext->ip4 = NULL; + return; + } + ext->ip4hlen = hlen; + ext->paylen -= hlen; ipproto = ext->ip4->ip_p; + if (ISSET(ntohs(ext->ip4->ip_off), IP_MF|IP_OFFMASK)) + return; break; #ifdef INET6 case ETHERTYPE_IPV6: - m = m_getptr(mp, hlen, &hoff); - if (m == NULL || m->m_len - hoff < sizeof(*ext->ip6)) + m = m_getptr(m0, hlen, &hoff); + if (m == NULL || m->m_len - hoff < sizeof(*ext->ip6)) { + DPRINTF("m_len %d, hoff %d, ip6 %zu", + m ? m->m_len : -1, hoff, sizeof(*ext->ip6)); return; + } ext->ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff); hlen = sizeof(*ext->ip6); + if (ext->paylen < hlen) { + DPRINTF("paylen %u, ip6hlen %zu", ext->paylen, hlen); + ext->ip6 = NULL; + return; + } + ext->paylen -= hlen; ipproto = ext->ip6->ip6_nxt; - break; #endif default: @@ -1093,16 +1177,51 @@ ether_extract_headers(struct mbuf *mp, struct ether_extracted *ext) switch (ipproto) { case IPPROTO_TCP: m = m_getptr(m, hoff + hlen, &hoff); - if (m == NULL || m->m_len - hoff < sizeof(*ext->tcp)) + if (m == NULL || m->m_len - hoff < sizeof(*ext->tcp)) { + DPRINTF("m_len %d, hoff %d, tcp %zu", + m ? m->m_len : -1, hoff, sizeof(*ext->tcp)); return; + } ext->tcp = (struct tcphdr *)(mtod(m, caddr_t) + hoff); + + memcpy(&hdrcpy.hc_data, &ext->tcp->th_flags - 1, 1); + hlen = hdrcpy.hc_th.off << 2; + if (m->m_len - hoff < hlen) { + DPRINTF("m_len %d, hoff %d, thoff %zu", + m ? m->m_len : -1, hoff, hlen); + ext->tcp = NULL; + return; + } + if (ext->paylen < hlen) { + DPRINTF("paylen %u, tcphlen %zu", ext->paylen, hlen); + ext->tcp = NULL; + return; + } + ext->tcphlen = hlen; + ext->paylen -= hlen; break; case IPPROTO_UDP: m = m_getptr(m, hoff + hlen, &hoff); - if (m == NULL || m->m_len - hoff < sizeof(*ext->udp)) + if (m == NULL || m->m_len - hoff < sizeof(*ext->udp)) { + DPRINTF("m_len %d, hoff %d, tcp %zu", + m ? m->m_len : -1, hoff, sizeof(*ext->tcp)); return; + } 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); + ext->udp = NULL; + return; + } break; } + + DNPRINTF(2, "%s%s%s%s%s%s ip4h %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); } diff --git a/sys/netinet/if_ether.h b/sys/netinet/if_ether.h index ed28944e721..4f5edd31b14 100644 --- a/sys/netinet/if_ether.h +++ b/sys/netinet/if_ether.h @@ -1,4 +1,4 @@ -/* $OpenBSD: if_ether.h,v 1.90 2023/07/27 20:21:25 jan Exp $ */ +/* $OpenBSD: if_ether.h,v 1.91 2024/02/13 13:58:19 bluhm Exp $ */ /* $NetBSD: if_ether.h,v 1.22 1996/05/11 13:00:00 mycroft Exp $ */ /* @@ -307,6 +307,9 @@ struct ether_extracted { struct ip6_hdr *ip6; struct tcphdr *tcp; struct udphdr *udp; + u_int ip4hlen; + u_int tcphlen; + u_int paylen; }; void ether_extract_headers(struct mbuf *, struct ether_extracted *);