From nobody Fri Sep 18 22:50:28 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4hmns63tcrz6t5DN for ; Fri, 18 Sep 2026 22:50:34 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4hmns60YLXz56kF for ; Fri, 18 Sep 2026 22:50:34 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1789771834; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=NlO9qiMHovr/QmWUZ7RgniolFf22OyTqjcS2/F3n0dU=; b=lqEyoBfNWfHHEDpkrePVcqnDmoxcnVtKmqwxaeHea8TfB0uVCtHFDVFxbIAiiu2R+7CKdg nQgZPccXvMB0loXACWARyA0h/zJx64xSicuoZHmxK1QTdPQrd0t5pWrbcFuDnjKvklWRvg viSgxeytF46JTqYUmAmO6iMf5Ub+ZEXwzRRDkZ4QqtiqX6SR/4yZXLZ6kN7ahf45ul7O64 49HQBYbPnLda9h/eVaa8ryRJNf70AJ1jqoV9jSWX0G0ROag0sYWA3s7/ksIdQ+6kTVfVBg 0ZcNu5s3NvfuCGxGcsuhqGGePXTyvfWB2eJvpZ9MbaazJX1QGJhn47U/GNs9ug== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1789771834; b=hmDvlhtFU7UzeVks1u8rIF79lDaBSiNX7pY9EU2NlmRotJuEDTZ3AXSRs2MVG0TAE7E4XF 6f6pgVMbdazgTNOi/P8wCBhPBFAZiOgW6GrxoU6RZ16JSmAfuXUOZci3M/2s+ho5mtQeYo rCm9N6R8mYzuyYL/OQ7wwIRBBk+ZeHBEBFQ4QoPOaSMB8htAVJGn6m99h1hlI/WQ6gK3qD wAYsJ//R9+gAdOZXlHcihg7aBPX1GofVQwb3o9fKfyAQ6FlhFtCPoMFtMGY43CvZCyKWrU t7fzinqfUdG8sxhUzM43F7FQi23oyYtHQYS0xX4IHxpxcYA+4F1FVRt1dL3ChA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1789771834; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=NlO9qiMHovr/QmWUZ7RgniolFf22OyTqjcS2/F3n0dU=; b=UCbcS0S3k4qfAaaIE0cO5pBM1NNwMCvP84VxNMuS0esFOQAGX0u3rDEbGyIQ2zPADD/UPw p1+VizLrvU6/AafPMgR0rO6VZRQbnzu0y7/KLVBKVY6tJIwy3+DDgHik+olwFJgwanv+Wp kDBeuJd36biS3cIxNjmjvTfndI4UjRIJmx/F5lJFytsfXx84Jaa+hZP5N2yS0zYW4Vv+cH +ejJTwFdTMb0IQpjeRIciYfdZ459h4fK7zSb67EeRd4iKZxVIDH7gkZP5J9AS0rJLG4y9X aaCH/z4kNXbCr37oxPZMQaciY4dZrsrWjfTlxe2iaigpZaI3s5EQgZ2oLftytA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4hmns56jFnzfKn for ; Fri, 18 Sep 2026 22:50:33 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 247da by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 18 Sep 2026 22:50:28 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Gleb Smirnoff Subject: git: 5f81426831a6 - main - ipfw: refactor macros around m_pullup() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: glebius X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 5f81426831a6cc682b87ae85a7e3a39879285260 Auto-Submitted: auto-generated Date: Fri, 18 Sep 2026 22:50:28 +0000 Message-Id: <6aadc034.247da.1404c9a6@gitrepo.freebsd.org> The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=5f81426831a6cc682b87ae85a7e3a39879285260 commit 5f81426831a6cc682b87ae85a7e3a39879285260 Author: Gleb Smirnoff AuthorDate: 2026-09-18 22:45:01 +0000 Commit: Gleb Smirnoff CommitDate: 2026-09-18 22:50:23 +0000 ipfw: refactor macros around m_pullup() In the prologue, where we check if the argument is a memory or an mbuf chain, do not set 'struct ip *ip' pointer. However, set the 'struct ether_header *eh' pointer there and set Etherner header length in 'ehlen'. Side effect of this refactor is that now Layer 2 hooks may send mbufs with ETHERTYPE_VLAN frames. However, current network stack doesn't do that. Write a new PULLUP() macro that would take type of the argument to determine how much to pull. Unlike PULLUP_TO() this macro can take typed pointer. This will allow to get rid of 'void *ulp' and bunch of casting macros in the next change. Use local bool variable to see if we need to unlock upon jump to pullup_failed. Embed pointer update into the branch of the macro, where pointers indeed need an update. Use new PULLUP() macro to pullup initial 'struct ip *ip' and 'struct ip6_hdr *ip6'. This removes max_protohdr sized pullup, that previously tried to pull more than an unmapped mbuf could yield, fixing a bug covered by the testcase sys/netpfil/ipfw/unmapped. Reviewed by: gallatin Differential Revision: https://reviews.freebsd.org/D59426 --- sys/netpfil/ipfw/ip_fw2.c | 180 ++++++++++++++++++++-------------------------- 1 file changed, 77 insertions(+), 103 deletions(-) diff --git a/sys/netpfil/ipfw/ip_fw2.c b/sys/netpfil/ipfw/ip_fw2.c index 133aeeb969a6..b56e4e8ac2d1 100644 --- a/sys/netpfil/ipfw/ip_fw2.c +++ b/sys/netpfil/ipfw/ip_fw2.c @@ -1463,6 +1463,7 @@ ipfw_chk(struct ip_fw_args *args) */ u_short offset = 0; u_short ip6f_mf = 0; + u_short ehlen; /* * Local copies of addresses. They are only valid if we have @@ -1508,104 +1509,79 @@ ipfw_chk(struct ip_fw_args *args) int done = 0; /* flag to exit the outer loop */ IPFW_RLOCK_TRACKER; + bool locked = false; bool mem; bool need_send_reject = false; int reject_code; uint16_t reject_mtu; if ((mem = (args->flags & IPFW_ARGS_LENMASK))) { - if (args->flags & IPFW_ARGS_ETHER) { - eh = (struct ether_header *)args->mem; - if (eh->ether_type == htons(ETHERTYPE_VLAN)) - ip = (struct ip *) - ((struct ether_vlan_header *)eh + 1); - else - ip = (struct ip *)(eh + 1); - } else { - eh = NULL; - ip = (struct ip *)args->mem; - } + eh = args->flags & IPFW_ARGS_ETHER ? args->mem : NULL; pktlen = IPFW_ARGS_LENGTH(args->flags); args->f_id.fib = args->ifp->if_fib; /* best guess */ } else { m = args->m; if (m->m_flags & M_SKIP_FIREWALL || (! V_ipfw_vnet_ready)) return (IP_FW_PASS); /* accept */ - if (args->flags & IPFW_ARGS_ETHER) { - /* We need some amount of data to be contiguous. */ - if (m->m_len < min(m->m_pkthdr.len, max_protohdr) && - (args->m = m = m_pullup(m, min(m->m_pkthdr.len, - max_protohdr))) == NULL) - goto pullup_failed; - eh = mtod(m, struct ether_header *); - ip = (struct ip *)(eh + 1); - } else { - eh = NULL; - ip = mtod(m, struct ip *); - } + eh = args->flags & IPFW_ARGS_ETHER ? + mtod(m, struct ether_header *) : NULL; pktlen = m->m_pkthdr.len; args->f_id.fib = M_GETFIB(m); /* mbuf not altered */ } + if (eh != NULL) { + if (eh->ether_type == htons(ETHERTYPE_VLAN)) + ehlen = sizeof(struct ether_vlan_header); + else + ehlen = sizeof(struct ether_header); + } else + ehlen = 0; + dst_ip.s_addr = 0; /* make sure it is initialized */ src_ip.s_addr = 0; /* make sure it is initialized */ src_port = dst_port = 0; DYN_INFO_INIT(&dyn_info); /* - * PULLUP_TO(len, p, T) makes sure that len + sizeof(T) is contiguous, - * then it sets p to point at the offset "len" in the mbuf. WARNING: the - * pointer might become stale after other pullups (but we never use it - * this way). + * PULLUP(p) makes typed pointer 'p' point at contiguous memory sized to + * its type, that is offset 'hlen' bytes from the beginning of the packet + * The macro reads variables 'hlen', 'ehlen' and 'm' or 'args->mem' in the + * current scope. If the macro does m_pullup(), the following current scope + * variables will be updated: 'm', 'args->m', 'eh', 'ip'. */ -#define PULLUP_TO(_len, p, T) PULLUP_LEN(_len, p, sizeof(T)) -#define EHLEN (eh != NULL ? ((char *)ip - (char *)eh) : 0) -#define _PULLUP_LOCKED(_len, p, T, unlock) \ -do { \ - int x = (_len) + T + EHLEN; \ - if (mem) { \ - if (__predict_false(pktlen < x)) { \ - unlock; \ - goto pullup_failed; \ - } \ - p = (char *)args->mem + (_len) + EHLEN; \ - } else { \ - if (__predict_false((m)->m_len < x)) { \ - args->m = m = m_pullup(m, x); \ - if (m == NULL) { \ - unlock; \ - goto pullup_failed; \ - } \ - } \ - p = mtod(m, char *) + (_len) + EHLEN; \ - } \ +#define PULLUP(_p) PULLUP_LEN((_p), sizeof(*(_p))) +#define PULLUP_TO(_p, _T) PULLUP_LEN((_p), sizeof(_T)) +#define PULLUP_LEN(_p, _size) \ +do { \ + int _max = hlen + ehlen + (_size); \ + if (mem) { \ + if (__predict_false(pktlen < _max)) \ + goto pullup_failed; \ + (_p) = (__typeof(_p))((char *)args->mem + hlen + ehlen);\ + } else { \ + if (__predict_false(m->m_len < _max)) { \ + args->m = m = m_pullup(m, _max); \ + if (m == NULL) \ + goto pullup_failed; \ + if (eh != NULL) { \ + eh = mtod(m, struct ether_header *); \ + ip = (struct ip *)(eh + 1); \ + } else \ + ip = mtod(m, struct ip *); \ + } \ + (_p) = (__typeof(_p))(mtod(m, char *) + hlen + ehlen); \ + } \ } while (0) -#define PULLUP_LEN(_len, p, T) _PULLUP_LOCKED(_len, p, T, ) -#define PULLUP_LEN_LOCKED(_len, p, T) \ - _PULLUP_LOCKED(_len, p, T, IPFW_PF_RUNLOCK(chain)); \ - UPDATE_POINTERS() -/* - * In case pointers got stale after pullups, update them. - */ -#define UPDATE_POINTERS() \ -do { \ - if (!mem) { \ - if (eh != NULL) { \ - eh = mtod(m, struct ether_header *); \ - ip = (struct ip *)(eh + 1); \ - } else \ - ip = mtod(m, struct ip *); \ - args->m = m; \ - } \ -} while (0) + PULLUP(ip); /* Identify IP packets and fill up variables. */ if (pktlen >= sizeof(struct ip6_hdr) && (eh == NULL || eh->ether_type == htons(ETHERTYPE_IPV6)) && ip->ip_v == 6) { - struct ip6_hdr *ip6 = (struct ip6_hdr *)ip; + struct ip6_hdr *ip6; + PULLUP(ip6); is_ipv6 = 1; args->flags |= IPFW_ARGS_IP6; hlen = sizeof(struct ip6_hdr); @@ -1614,14 +1590,14 @@ do { \ while (ulp == NULL && offset == 0) { switch (proto) { case IPPROTO_ICMPV6: - PULLUP_TO(hlen, ulp, struct icmp6_hdr); + PULLUP_TO(ulp, struct icmp6_hdr); #ifdef INET6 icmp6_type = ICMP6(ulp)->icmp6_type; #endif break; case IPPROTO_TCP: - PULLUP_TO(hlen, ulp, struct tcphdr); + PULLUP_TO(ulp, struct tcphdr); dst_port = TCP(ulp)->th_dport; src_port = TCP(ulp)->th_sport; /* save flags for dynamic rules */ @@ -1632,28 +1608,27 @@ do { \ if (pktlen >= hlen + sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr) + offsetof(struct sctp_init, a_rwnd)) - PULLUP_LEN(hlen, ulp, + PULLUP_LEN(ulp, sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr) + offsetof(struct sctp_init, a_rwnd)); else if (pktlen >= hlen + sizeof(struct sctphdr)) - PULLUP_LEN(hlen, ulp, pktlen - hlen); + PULLUP_LEN(ulp, pktlen - hlen); else - PULLUP_LEN(hlen, ulp, - sizeof(struct sctphdr)); + PULLUP_LEN(ulp, sizeof(struct sctphdr)); src_port = SCTP(ulp)->src_port; dst_port = SCTP(ulp)->dest_port; break; case IPPROTO_UDP: case IPPROTO_UDPLITE: - PULLUP_TO(hlen, ulp, struct udphdr); + PULLUP_TO(ulp, struct udphdr); dst_port = UDP(ulp)->uh_dport; src_port = UDP(ulp)->uh_sport; break; case IPPROTO_HOPOPTS: /* RFC 2460 */ - PULLUP_TO(hlen, ulp, struct ip6_hbh); + PULLUP_TO(ulp, struct ip6_hbh); ext_hd |= EXT_HOPOPTS; hlen += (((struct ip6_hbh *)ulp)->ip6h_len + 1) << 3; proto = ((struct ip6_hbh *)ulp)->ip6h_nxt; @@ -1661,7 +1636,7 @@ do { \ break; case IPPROTO_ROUTING: /* RFC 2460 */ - PULLUP_TO(hlen, ulp, struct ip6_rthdr); + PULLUP_TO(ulp, struct ip6_rthdr); switch (((struct ip6_rthdr *)ulp)->ip6r_type) { case 0: ext_hd |= EXT_RTHDR0; @@ -1686,7 +1661,7 @@ do { \ break; case IPPROTO_FRAGMENT: /* RFC 2460 */ - PULLUP_TO(hlen, ulp, struct ip6_frag); + PULLUP_TO(ulp, struct ip6_frag); ext_hd |= EXT_FRAGMENT; hlen += sizeof (struct ip6_frag); proto = ((struct ip6_frag *)ulp)->ip6f_nxt; @@ -1709,7 +1684,7 @@ do { \ break; case IPPROTO_DSTOPTS: /* RFC 2460 */ - PULLUP_TO(hlen, ulp, struct ip6_hbh); + PULLUP_TO(ulp, struct ip6_hbh); ext_hd |= EXT_DSTOPTS; hlen += (((struct ip6_hbh *)ulp)->ip6h_len + 1) << 3; proto = ((struct ip6_hbh *)ulp)->ip6h_nxt; @@ -1717,7 +1692,7 @@ do { \ break; case IPPROTO_AH: /* RFC 2402 */ - PULLUP_TO(hlen, ulp, struct ip6_ext); + PULLUP_TO(ulp, struct ip6_ext); ext_hd |= EXT_AH; hlen += (((struct ip6_ext *)ulp)->ip6e_len + 2) << 2; proto = ((struct ip6_ext *)ulp)->ip6e_nxt; @@ -1725,7 +1700,7 @@ do { \ break; case IPPROTO_ESP: /* RFC 2406 */ - PULLUP_TO(hlen, ulp, uint32_t); /* SPI, Seq# */ + PULLUP_TO(ulp, uint32_t); /* SPI, Seq# */ /* Anything past Seq# is variable length and * data past this ext. header is encrypted. */ ext_hd |= EXT_ESP; @@ -1742,21 +1717,21 @@ do { \ case IPPROTO_OSPFIGP: /* XXX OSPF header check? */ - PULLUP_TO(hlen, ulp, struct ip6_ext); + PULLUP_TO(ulp, struct ip6_ext); break; case IPPROTO_PIM: /* XXX PIM header check? */ - PULLUP_TO(hlen, ulp, struct pim); + PULLUP_TO(ulp, struct pim); break; case IPPROTO_GRE: /* RFC 1701 */ /* XXX GRE header check? */ - PULLUP_TO(hlen, ulp, struct grehdr); + PULLUP_TO(ulp, struct grehdr); break; case IPPROTO_CARP: - PULLUP_TO(hlen, ulp, offsetof( + PULLUP_TO(ulp, offsetof( struct carp_header, carp_counter)); if (CARP_ADVERTISEMENT != ((struct carp_header *)ulp)->carp_type) @@ -1764,21 +1739,21 @@ do { \ break; case IPPROTO_IPV6: /* RFC 2893 */ - PULLUP_TO(hlen, ulp, struct ip6_hdr); + PULLUP_TO(ulp, struct ip6_hdr); break; case IPPROTO_IPV4: /* RFC 2893 */ - PULLUP_TO(hlen, ulp, struct ip); + PULLUP_TO(ulp, struct ip); break; case IPPROTO_ETHERIP: /* RFC 3378 */ - PULLUP_LEN(hlen, ulp, + PULLUP_LEN(ulp, sizeof(struct etherip_header) + sizeof(struct ether_header)); break; case IPPROTO_PFSYNC: - PULLUP_TO(hlen, ulp, struct pfsync_header); + PULLUP_TO(ulp, struct pfsync_header); break; default: @@ -1788,11 +1763,10 @@ do { \ proto, ext_hd); if (V_fw_deny_unknown_exthdrs) return (IP_FW_DENY); - PULLUP_TO(hlen, ulp, struct ip6_ext); + PULLUP_TO(ulp, struct ip6_ext); break; } /*switch */ } - UPDATE_POINTERS(); ip6 = (struct ip6_hdr *)ip; args->f_id.addr_type = 6; args->f_id.src_ip6 = ip6->ip6_src; @@ -1818,7 +1792,7 @@ do { \ if (offset == 0) { switch (proto) { case IPPROTO_TCP: - PULLUP_TO(hlen, ulp, struct tcphdr); + PULLUP_TO(ulp, struct tcphdr); dst_port = TCP(ulp)->th_dport; src_port = TCP(ulp)->th_sport; /* save flags for dynamic rules */ @@ -1829,28 +1803,27 @@ do { \ if (pktlen >= hlen + sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr) + offsetof(struct sctp_init, a_rwnd)) - PULLUP_LEN(hlen, ulp, + PULLUP_LEN(ulp, sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr) + offsetof(struct sctp_init, a_rwnd)); else if (pktlen >= hlen + sizeof(struct sctphdr)) - PULLUP_LEN(hlen, ulp, pktlen - hlen); + PULLUP_LEN(ulp, pktlen - hlen); else - PULLUP_LEN(hlen, ulp, - sizeof(struct sctphdr)); + PULLUP_LEN(ulp, sizeof(struct sctphdr)); src_port = SCTP(ulp)->src_port; dst_port = SCTP(ulp)->dest_port; break; case IPPROTO_UDP: case IPPROTO_UDPLITE: - PULLUP_TO(hlen, ulp, struct udphdr); + PULLUP_TO(ulp, struct udphdr); dst_port = UDP(ulp)->uh_dport; src_port = UDP(ulp)->uh_sport; break; case IPPROTO_ICMP: - PULLUP_TO(hlen, ulp, struct icmphdr); + PULLUP_TO(ulp, struct icmphdr); //args->f_id.flags = ICMP(ulp)->icmp_type; break; @@ -1864,7 +1837,6 @@ do { \ } } - UPDATE_POINTERS(); args->f_id.addr_type = 4; args->f_id.src_ip = ntohl(src_ip.s_addr); args->f_id.dst_ip = ntohl(dst_ip.s_addr); @@ -1874,7 +1846,6 @@ do { \ args->f_id.addr_type = 1; /* XXX */ } -#undef PULLUP_TO pktlen = iplen < pktlen ? iplen: pktlen; /* Properly initialize the rest of f_id */ @@ -1887,6 +1858,7 @@ do { \ IPFW_PF_RUNLOCK(chain); return (IP_FW_PASS); /* accept */ } + locked = true; if (args->flags & IPFW_ARGS_REF) { /* * Packet has already been tagged as a result of a previous @@ -2580,7 +2552,7 @@ do { \ case O_TCPOPTS: if (proto == IPPROTO_TCP && offset == 0 && ulp){ - PULLUP_LEN_LOCKED(hlen, ulp, + PULLUP_LEN(ulp, (TCP(ulp)->th_off << 2)); match = tcpopts_match(TCP(ulp), cmd); } @@ -2605,8 +2577,7 @@ do { \ uint16_t mss, *p; int i; - PULLUP_LEN_LOCKED(hlen, ulp, - (TCP(ulp)->th_off << 2)); + PULLUP_LEN(ulp, TCP(ulp)->th_off << 2); if ((tcpopts_parse(TCP(ulp), &mss) & IP_FW_TCPOPT_MSS) == 0) break; @@ -3533,7 +3504,8 @@ do { \ } /* end of inner loop, scan opcodes */ #undef PULLUP_LEN -#undef PULLUP_LEN_LOCKED +#undef PULLUP_TO +#undef PULLUP if (done) break; @@ -3575,6 +3547,8 @@ do { \ return (retval); pullup_failed: + if (locked) + IPFW_PF_RUNLOCK(chain); if (V_fw_verbose) printf("ipfw: pullup failed\n"); return (IP_FW_DENY);