[PATCH nf-next v2] netfilter: nf_reject: validate TCP header length before sending resets

[PATCH nf-next v2] netfilter: nf_reject: validate TCP header length before sending resets

[zhangmh25]

From: Minghao Zhang <zhangmh25@mails.tsinghua.edu.cn>

The TCP header validation in both nf_reject_ip_tcphdr_get() and
nf_reject_ip6_tcphdr_get() checks that the TCP segment is at least
sizeof(struct tcphdr) long, but does not verify that the header
length claimed via the data offset is actually present in the packet.
A peer can send a TCP packet claiming a 60-byte header although only
the fixed 20-byte header is present.

The reset generation then computes the ACK number using the claimed
header length, and the subtraction of the data offset from the TCP
segment length underflows, producing a wrapped ACK value in the
generated RST. For a 20-byte segment with doff = 15, ACK bit clear,
sequence number 0, SYN and FIN clear, the RST carries ack_seq
0xffffffd8 instead of a meaningful value.

Reject TCP headers whose data offset is smaller than the fixed header
or exceeds the TCP segment length, and derive that segment length
from the length declared in the IP header rather than from skb->len
(IPv6 packets with a zero payload_len keep the skb-length handling).
On the IPv4 and IPv6 receive paths the skb has already been trimmed
to the declared length by ip_rcv_core() / ip6_rcv_core(), but the
netdev ingress, inet ingress and bridge reject paths pass the
untrimmed frame, where skb->len still counts Ethernet padding and any
trailing bytes the sender appends. There, an skb->len based check
can still accept a data offset that runs past the IP-declared
segment, and the ACK
computation counts padding bytes that were never part of the segment.

For IPv4, use skb_ip_totlen() for both the data offset check and the
ACK computation: BIG TCP GSO packets carry tot_len == 0, and the
helper keeps the skb length for those. For IPv6, use payload_len
minus the extension header chain, and preserve the existing
skb-length handling when payload_len is zero (jumbograms and BIG TCP
aggregates). GRO aggregates keep working: inet_gro_complete() and
ipv6_gro_complete() update tot_len / payload_len to the aggregate
length, and aggregates past 64 KiB are covered by the zero-length
handling on both sides. The TCP checksum validation keeps its
existing skb-length basis and is unchanged; whether it rejects a
padded segment whose checksum was signed over the IP-declared length
depends on skb->ip_summed and the hook, exactly as before this
change.

With connection tracking enabled, segments with an inconsistent data
offset are already flagged by tcp_error() in nf_conntrack_proto_tcp.c
(th->doff*4 < sizeof(struct tcphdr) || tcplen < th->doff*4) on the
paths where conntrack runs before the reject rule, so rulesets with
an early "ct state invalid drop" rule never pass them to the reject
target. The netdev and inet ingress reject paths run before conntrack
and are not covered by that. The affected configurations are stateless
rulesets, netdev-family reject rules, and rulesets whose reject rule
is not preceded by an invalid-state drop.

Discovered through LLM-assisted static review and confirmed by manual
inspection and QEMU testing. Both reject paths were tested with
nftables "reject with tcp reset" rules: on both IPv4 and IPv6, a
probe with doff = 15 produced an RST with a wrapped ack_seq, and with
the fix no RST is generated for such packets while probes with
well-formed headers still get a correct RST. The untrimmed paths were
exercised through a netdev ingress reject rule: a probe whose doff =
15 only fits because of 40 appended trailing bytes still elicited an
RST with the skb->len based check and gets no RST with this change,
and a probe with otherwise well-formed TCP headers plus six bytes of
appended padding is now acknowledged exactly instead of by the padding
length on both families; on IPv4 this is the Ethernet minimum frame
size, while the IPv6 fixed headers alone already exceed that minimum,
so there the bytes are trailing data.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 764d8a9f2407 ("[NETFILTER]: Add IPv6 REJECT target")
Reported-by: Minghao Zhang <zhangmh25@mails.tsinghua.edu.cn>
Reported-by: Jianjun Chen <jianjun@tsinghua.edu.cn>
Link: https://lore.kernel.org/netdev/20261102155622.2658789-1-zhangmh25@mails.tsinghua.edu.cn/
Signed-off-by: Minghao Zhang <zhangmh25@mails.tsinghua.edu.cn>
---
Changes since v1:
- retarget to nf-next, as suggested by Pablo Neira Ayuso;
- rebase onto the current nf-next tree;
- derive the TCP segment length from the IP header instead of
  skb->len, for both the data offset check and the ACK computation,
  so Ethernet padding and trailing bytes on the untrimmed netdev
  ingress and bridge reject paths can no longer make an oversized
  data offset pass the check or skew the ACK (pointed out by the
  netdev AI review);
- use skb_ip_totlen() for the IPv4 length, so BIG TCP GSO packets
  with tot_len == 0 keep the skb-length behaviour;
- derive the IPv6 extension header length from the parser's own
  offset basis, so the calculation does not assume that the skb data
  starts at the network header;
- document the conntrack reachability limitation pointed out by
  Pablo Neira Ayuso;
- drop the stable Cc and target nf-next following maintainer
  feedback.
 net/ipv4/netfilter/nf_reject_ipv4.c | 20 +++++++++++++++++++-
 net/ipv6/netfilter/nf_reject_ipv6.c | 28 ++++++++++++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index 7cb33af671e2..31c72b30bcf2 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -176,6 +176,7 @@ nf_reject_ip_tcphdr_get(struct sk_buff *oldskb,
 			struct tcphdr *_oth, int hook)
 {
 	const struct tcphdr *oth;
+	unsigned int tcplen;
 
 	/* IP header checks: fragment. */
 	if (ip_hdr(oldskb)->frag_off & htons(IP_OFFSET))
@@ -189,6 +190,22 @@ nf_reject_ip_tcphdr_get(struct sk_buff *oldskb,
 	if (oth == NULL)
 		return NULL;
 
+	/* Reject invalid or truncated TCP headers.  Use the length
+	 * declared in the IP header: the netdev ingress and bridge
+	 * reject paths see untrimmed skbs, where skb->len may include
+	 * Ethernet padding and trailing bytes past tot_len.
+	 * skb_ip_totlen() keeps the skb length for BIG TCP GSO
+	 * aggregates, which carry tot_len == 0.
+	 */
+	tcplen = skb_ip_totlen(oldskb);
+	if (tcplen < ip_hdrlen(oldskb) + sizeof(struct tcphdr))
+		return NULL;
+	tcplen -= ip_hdrlen(oldskb);
+
+	if (oth->doff < sizeof(struct tcphdr) / 4 ||
+	    tcplen < (oth->doff << 2))
+		return NULL;
+
 	/* No RST for RST. */
 	if (oth->rst)
 		return NULL;
@@ -240,7 +257,8 @@ static void nf_reject_ip_tcphdr_put(struct sk_buff *nskb, const struct sk_buff *
 		tcph->seq = oth->ack_seq;
 	} else {
 		tcph->ack_seq = htonl(ntohl(oth->seq) + oth->syn + oth->fin +
-				      oldskb->len - ip_hdrlen(oldskb) -
+				      skb_ip_totlen(oldskb) -
+				      ip_hdrlen(oldskb) -
 				      (oth->doff << 2));
 		tcph->ack = 1;
 	}
diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
index 5e08d8529cc8..634304d2ad4e 100644
--- a/net/ipv6/netfilter/nf_reject_ipv6.c
+++ b/net/ipv6/netfilter/nf_reject_ipv6.c
@@ -198,18 +198,33 @@ nf_reject_ip6_tcphdr_get(struct sk_buff *oldskb,
 	const struct ipv6hdr *oip6h = ipv6_hdr(oldskb);
 	u8 proto;
 	__be16 frag_off;
-	int tcphoff;
+	int tcphoff, start;
 
 	proto = oip6h->nexthdr;
-	tcphoff = ipv6_skip_exthdr(oldskb, ((u8 *)(oip6h + 1) - oldskb->data),
-				   &proto, &frag_off);
+	start = (u8 *)(oip6h + 1) - oldskb->data;
+	tcphoff = ipv6_skip_exthdr(oldskb, start, &proto, &frag_off);
 
 	if ((tcphoff < 0) || (tcphoff > oldskb->len)) {
 		pr_debug("Cannot get TCP header.\n");
 		return NULL;
 	}
 
-	*otcplen = oldskb->len - tcphoff;
+	/* Use the length declared in the IPv6 header: the netdev ingress
+	 * and bridge reject paths see untrimmed skbs, where skb->len may
+	 * include Ethernet padding and trailing bytes past payload_len.
+	 * Preserve the existing skb-length handling when payload_len is
+	 * zero.
+	 */
+	if (oip6h->payload_len) {
+		unsigned int ext_len = tcphoff - start;
+
+		if (ntohs(oip6h->payload_len) <
+		    ext_len + sizeof(struct tcphdr))
+			return NULL;
+		*otcplen = ntohs(oip6h->payload_len) - ext_len;
+	} else {
+		*otcplen = oldskb->len - tcphoff;
+	}
 
 	/* IP header checks: fragment, too short. */
 	if (proto != IPPROTO_TCP || *otcplen < sizeof(struct tcphdr)) {
@@ -223,6 +238,11 @@ nf_reject_ip6_tcphdr_get(struct sk_buff *oldskb,
 	if (otcph == NULL)
 		return NULL;
 
+	/* Reject invalid or truncated TCP headers. */
+	if (otcph->doff < sizeof(struct tcphdr) / 4 ||
+	    *otcplen < (otcph->doff << 2))
+		return NULL;
+
 	/* No RST for RST. */
 	if (otcph->rst) {
 		pr_debug("RST is set\n");
-- 
2.34.1

Re: [PATCH nf-next v2] netfilter: nf_reject: validate TCP header length before sending resets

[Pablo Neira Ayuso]

On Tue, Nov 10, 2026 at 12:47:26AM +0800, zhangmh25@mails.tsinghua.edu.cn wrote:
> From: Minghao Zhang <zhangmh25@mails.tsinghua.edu.cn>
> 
[...]
> With connection tracking enabled, segments with an inconsistent data
> offset are already flagged by tcp_error() in nf_conntrack_proto_tcp.c
> (th->doff*4 < sizeof(struct tcphdr) || tcplen < th->doff*4) on the
> paths where conntrack runs before the reject rule, so rulesets with
> an early "ct state invalid drop" rule never pass them to the reject
> target. The netdev and inet ingress reject paths run before conntrack
> and are not covered by that. The affected configurations are stateless
> rulesets, netdev-family reject rules, and rulesets whose reject rule
> is not preceded by an invalid-state drop.

I am not sure I want to duplicate all these checks that already exist
in nf_conntrack only for the stateless case.