[PATCH nf v2 1/1] netfilter: validate L4 headers after userspace packet writes

[PATCH nf v2 1/1] netfilter: validate L4 headers after userspace packet writes

[Zhiling Zou]

NFQUEUE and nft_payload can hand packet data modified by userspace back
to the stack. Recent restrictions keep link and network headers stable,
but the transport header can still be changed.

A packet can therefore keep the same network header and conntrack entry
while changing the transport header layout. For TCP, increasing doff can
make later helper or NAT code use a different transport-header base than
the parser used, and can make offsets point past skb->tail.

Extend NFQUEUE payload validation to check the final L4 protocol and base
header after IPv4 options or IPv6 extension headers. Reject packets whose
L4 protocol no longer matches an attached non-template conntrack entry,
reject unknown L4 protocols for writable payload replacement, and
validate TCP data offset before reinjection.

For nft payload writes, reject transport-header stores that overlap TCP
doff. nft_nh_write_ok() already rejects network-header protocol changes,
so keeping doff stable prevents nft payload writes from changing the TCP
header length underneath conntrack and helper users.

Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
---
changes in v2:
- Replace the H.323-specific NAT/helper-only fix with generic validation
  for userspace packet writes.
- Validate NFQUEUE payload replacements after IPv4 option and IPv6
  extension-header validation, including TCP data offset and attached
  non-template conntrack L4 protocol consistency.
- Reject nft_payload transport-header writes that overlap TCP doff.
- v1 Link: https://lore.kernel.org/all/68faab5bf406aad612233e76994c649fbc547b8d.1785133455.git.zhilinz@nebusec.ai

 net/netfilter/nfnetlink_queue.c | 58 +++++++++++++++++++++++++++++++--
 net/netfilter/nft_payload.c     | 13 ++++++++
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index b8aaf39cb4d8e..562b7bf6c8777 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -28,10 +28,16 @@
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nfnetlink_queue.h>
 #include <linux/netfilter/nf_conntrack_common.h>
+#include <linux/icmp.h>
+#include <linux/icmpv6.h>
 #include <linux/list.h>
+#include <linux/sctp.h>
 #include <linux/cgroup-defs.h>
 #include <linux/rhashtable.h>
 #include <linux/jhash.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
+#include <net/gre.h>
 #include <net/gso.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
@@ -1192,6 +1198,49 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
 	return err;
 }
 
+static bool nfqnl_validate_l4(const u8 *data, unsigned int data_len,
+			      const struct nf_queue_entry *e, u8 proto)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+	enum ip_conntrack_info ctinfo;
+	const struct nf_conn *ct;
+
+	ct = nf_ct_get(e->skb, &ctinfo);
+	if (ct && !nf_ct_is_template(ct) && nf_ct_protonum(ct) != proto)
+		return false;
+#endif
+
+	switch (proto) {
+	case IPPROTO_TCP: {
+		const struct tcphdr *th = (const struct tcphdr *)data;
+		unsigned int thlen;
+
+		if (data_len < sizeof(*th))
+			return false;
+
+		thlen = __tcp_hdrlen(th);
+		if (thlen < sizeof(*th) || data_len < thlen)
+			return false;
+
+		return true;
+	}
+	case IPPROTO_UDP:
+		return data_len >= sizeof(struct udphdr);
+	case IPPROTO_ICMP:
+		return data_len >= sizeof(struct icmphdr);
+	case IPPROTO_ICMPV6:
+		return data_len >= sizeof(struct icmp6hdr);
+	case IPPROTO_SCTP:
+		return data_len >= sizeof(struct sctphdr);
+	case IPPROTO_GRE:
+		return data_len >= sizeof(struct gre_base_hdr);
+	case IPPROTO_NONE:
+		return true;
+	}
+
+	return false;
+}
+
 static bool nfqnl_validate_ipopts(const struct iphdr *iph_new,
 				  const struct nf_queue_entry *e)
 {
@@ -1229,7 +1278,9 @@ static bool nfqnl_validate_ip4(const struct iphdr *iph, unsigned int data_len,
 	/* support for ipopts mangling would require
 	 * recompile + skb transport header update.
 	 */
-	return nfqnl_validate_ipopts(iph, e);
+	return nfqnl_validate_ipopts(iph, e) &&
+	       nfqnl_validate_l4((const u8 *)iph + ihl, data_len - ihl, e,
+				 iph->protocol);
 }
 
 static bool nfqnl_validate_one_exthdr(const u8 *data,
@@ -1286,7 +1337,8 @@ static bool nfqnl_validate_exthdr(const struct ipv6hdr *ip6_new,
 		int hdrlen;
 
 		if (orig_nexthdr == NEXTHDR_NONE)
-			return true;
+			return nfqnl_validate_l4(data, data_len, e,
+						 new_nexthdr);
 
 		if (unlikely(exthdr_cnt++ >= IP6_MAX_EXT_HDRS_CNT))
 			return false;
@@ -1323,7 +1375,7 @@ static bool nfqnl_validate_exthdr(const struct ipv6hdr *ip6_new,
 		data += hdrlen;
 	}
 
-	return true;
+	return nfqnl_validate_l4(data, data_len, e, new_nexthdr);
 }
 
 static bool nfqnl_validate_ip6(const struct ipv6hdr *ip6, unsigned int data_len,
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index 391539a1ceaa7..63cf3b4f7420d 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -1077,6 +1077,17 @@ static bool nft_payload_csum_write_ok(const struct nft_pktinfo *pkt,
 	return false;
 }
 
+static bool nft_th_write_ok(const struct nft_pktinfo *pkt,
+			    const struct nft_payload_set *priv)
+{
+	unsigned int doff = offsetof(struct tcphdr, ack_seq) + sizeof(__be32);
+
+	if (pkt->tprot != IPPROTO_TCP)
+		return true;
+
+	return priv->offset > doff || priv->offset + priv->len <= doff;
+}
+
 static void nft_payload_set_eval(const struct nft_expr *expr,
 				 struct nft_regs *regs,
 				 const struct nft_pktinfo *pkt)
@@ -1115,6 +1126,8 @@ static void nft_payload_set_eval(const struct nft_expr *expr,
 	case NFT_PAYLOAD_TRANSPORT_HEADER:
 		if (!(pkt->flags & NFT_PKTINFO_L4PROTO) || pkt->fragoff)
 			goto err;
+		if (!nft_th_write_ok(pkt, priv))
+			goto err;
 		offset = nft_thoff(pkt);
 		break;
 	case NFT_PAYLOAD_INNER_HEADER:
-- 
2.43.0

Re: [PATCH nf v2 1/1] netfilter: validate L4 headers after userspace packet writes

[Pablo Neira Ayuso]

On Thu, Jul 30, 2026 at 03:30:26PM +0800, Zhiling Zou wrote:
[...]
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index b8aaf39cb4d8e..562b7bf6c8777 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -28,10 +28,16 @@
>  #include <linux/netfilter/nfnetlink.h>
>  #include <linux/netfilter/nfnetlink_queue.h>
>  #include <linux/netfilter/nf_conntrack_common.h>
> +#include <linux/icmp.h>
> +#include <linux/icmpv6.h>
>  #include <linux/list.h>
> +#include <linux/sctp.h>
>  #include <linux/cgroup-defs.h>
>  #include <linux/rhashtable.h>
>  #include <linux/jhash.h>
> +#include <linux/tcp.h>
> +#include <linux/udp.h>
> +#include <net/gre.h>
>  #include <net/gso.h>
>  #include <net/sock.h>
>  #include <net/tcp_states.h>
> @@ -1192,6 +1198,49 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
>  	return err;
>  }
>  
> +static bool nfqnl_validate_l4(const u8 *data, unsigned int data_len,
> +			      const struct nf_queue_entry *e, u8 proto)
> +{
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +	enum ip_conntrack_info ctinfo;
> +	const struct nf_conn *ct;
> +
> +	ct = nf_ct_get(e->skb, &ctinfo);
> +	if (ct && !nf_ct_is_template(ct) && nf_ct_protonum(ct) != proto)

I think it should be easier to disallow protocol number mangling in
the IP header (layer 3 restrictions), if not done already.

> +		return false;
> +#endif
> +
> +	switch (proto) {
> +	case IPPROTO_TCP: {
> +		const struct tcphdr *th = (const struct tcphdr *)data;

This needs to use skb_header_pointer() here, you cannot assume the tcp
header is in a linear area.

> +		unsigned int thlen;
> +
> +		if (data_len < sizeof(*th))
> +			return false;
> +
> +		thlen = __tcp_hdrlen(th);
> +		if (thlen < sizeof(*th) || data_len < thlen)
> +			return false;
> +
> +		return true;
> +	}
> +	case IPPROTO_UDP:
> +		return data_len >= sizeof(struct udphdr);
> +	case IPPROTO_ICMP:
> +		return data_len >= sizeof(struct icmphdr);
> +	case IPPROTO_ICMPV6:
> +		return data_len >= sizeof(struct icmp6hdr);
> +	case IPPROTO_SCTP:
> +		return data_len >= sizeof(struct sctphdr);
> +	case IPPROTO_GRE:
> +		return data_len >= sizeof(struct gre_base_hdr);
> +	case IPPROTO_NONE:

Remove this and make it part of default and return true if protocol is
unknown.

> +		return true;
> +	}
> +
> +	return false;

Default is false for an unknown protocol, should be true.

> +}
> +
>  static bool nfqnl_validate_ipopts(const struct iphdr *iph_new,
>  				  const struct nf_queue_entry *e)
>  {
> @@ -1229,7 +1278,9 @@ static bool nfqnl_validate_ip4(const struct iphdr *iph, unsigned int data_len,
>  	/* support for ipopts mangling would require
>  	 * recompile + skb transport header update.
>  	 */
> -	return nfqnl_validate_ipopts(iph, e);
> +	return nfqnl_validate_ipopts(iph, e) &&
> +	       nfqnl_validate_l4((const u8 *)iph + ihl, data_len - ihl, e,
> +				 iph->protocol);
>  }
>  
>  static bool nfqnl_validate_one_exthdr(const u8 *data,
> @@ -1286,7 +1337,8 @@ static bool nfqnl_validate_exthdr(const struct ipv6hdr *ip6_new,
>  		int hdrlen;
>  
>  		if (orig_nexthdr == NEXTHDR_NONE)
> -			return true;
> +			return nfqnl_validate_l4(data, data_len, e,
> +						 new_nexthdr);
>  
>  		if (unlikely(exthdr_cnt++ >= IP6_MAX_EXT_HDRS_CNT))
>  			return false;
> @@ -1323,7 +1375,7 @@ static bool nfqnl_validate_exthdr(const struct ipv6hdr *ip6_new,
>  		data += hdrlen;
>  	}
>  
> -	return true;
> +	return nfqnl_validate_l4(data, data_len, e, new_nexthdr);
>  }
>  
>  static bool nfqnl_validate_ip6(const struct ipv6hdr *ip6, unsigned int data_len,
> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
> index 391539a1ceaa7..63cf3b4f7420d 100644
> --- a/net/netfilter/nft_payload.c
> +++ b/net/netfilter/nft_payload.c
> @@ -1077,6 +1077,17 @@ static bool nft_payload_csum_write_ok(const struct nft_pktinfo *pkt,
>  	return false;
>  }
>  
> +static bool nft_th_write_ok(const struct nft_pktinfo *pkt,
> +			    const struct nft_payload_set *priv)
> +{
> +	unsigned int doff = offsetof(struct tcphdr, ack_seq) + sizeof(__be32);
> +
> +	if (pkt->tprot != IPPROTO_TCP)
> +		return true;
> +
> +	return priv->offset > doff || priv->offset + priv->len <= doff;
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

maybe simply check priv->offset >= doff here?

> +}
> +
>  static void nft_payload_set_eval(const struct nft_expr *expr,
>  				 struct nft_regs *regs,
>  				 const struct nft_pktinfo *pkt)
> @@ -1115,6 +1126,8 @@ static void nft_payload_set_eval(const struct nft_expr *expr,
>  	case NFT_PAYLOAD_TRANSPORT_HEADER:
>  		if (!(pkt->flags & NFT_PKTINFO_L4PROTO) || pkt->fragoff)
>  			goto err;
> +		if (!nft_th_write_ok(pkt, priv))
> +			goto err;
>  		offset = nft_thoff(pkt);
>  		break;
>  	case NFT_PAYLOAD_INNER_HEADER:
> -- 
> 2.43.0

Re: [PATCH nf v2 1/1] netfilter: validate L4 headers after userspace packet writes

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > +	const struct nf_conn *ct;
> > +
> > +	ct = nf_ct_get(e->skb, &ctinfo);
> > +	if (ct && !nf_ct_is_template(ct) && nf_ct_protonum(ct) != proto)
> 
> I think it should be easier to disallow protocol number mangling in
> the IP header (layer 3 restrictions), if not done already.

How?  nfqueue is whole-replace, not a delta.
Or do you mean checking ip_hdr(skb) vs. the protocol field in userspace
provided buffer?

I would prefer this solution (i.e. check ct protocol), it still allows theoretical nfqueue based
tunneling header insertion, if done in prerouting before conntrack.

> > +	switch (proto) {
> > +	case IPPROTO_TCP: {
> > +		const struct tcphdr *th = (const struct tcphdr *)data;
> 
> This needs to use skb_header_pointer() here, you cannot assume the tcp
> header is in a linear area.

data is a linear buffer coming from userspace
(nla_data(nfqa[NFQA_PAYLOAD]).

> > +	case IPPROTO_SCTP:
> > +		return data_len >= sizeof(struct sctphdr);
> > +	case IPPROTO_GRE:
> > +		return data_len >= sizeof(struct gre_base_hdr);
> > +	case IPPROTO_NONE:
> 
> Remove this and make it part of default and return true if protocol is
> unknown.

Hmm.  Its likely safe to accept unknown headers, here.

Perhaps next iteration should indeed do what you suggest but also check
check ESP and AH.

> Default is false for an unknown protocol, should be true.

I suggested it this way, i.e. don't permit unknown l4 protocols, but
maybe its too restrictive.

> > +	if (pkt->tprot != IPPROTO_TCP)
> > +		return true;
> > +
> > +	return priv->offset > doff || priv->offset + priv->len <= doff;
>                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> maybe simply check priv->offset >= doff here?

Could you elaborate?  The above LGTM.  priv->offset > doff is already
tested?  I mean, write is ok either if offset exceeds doff (lhs)
or if offset + length doesn't touch doff area (rhs).

Did you mean "just reject everything exceeding doff"?

Patch LGTM, except perhaps switching to "allow unknowns" in nfqueue.

Re: [PATCH nf v2 1/1] netfilter: validate L4 headers after userspace packet writes

[Pablo Neira Ayuso]

On Thu, Jul 30, 2026 at 01:06:27PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > +	const struct nf_conn *ct;
> > > +
> > > +	ct = nf_ct_get(e->skb, &ctinfo);
> > > +	if (ct && !nf_ct_is_template(ct) && nf_ct_protonum(ct) != proto)
> > 
> > I think it should be easier to disallow protocol number mangling in
> > the IP header (layer 3 restrictions), if not done already.
> 
> How?  nfqueue is whole-replace, not a delta.
> Or do you mean checking ip_hdr(skb) vs. the protocol field in userspace
> provided buffer?
> 
> I would prefer this solution (i.e. check ct protocol), it still allows theoretical nfqueue based
> tunneling header insertion, if done in prerouting before conntrack.

OK

> > > +	switch (proto) {
> > > +	case IPPROTO_TCP: {
> > > +		const struct tcphdr *th = (const struct tcphdr *)data;
> > 
> > This needs to use skb_header_pointer() here, you cannot assume the tcp
> > header is in a linear area.
> 
> data is a linear buffer coming from userspace
> (nla_data(nfqa[NFQA_PAYLOAD]).

Indeed.

> > > +	case IPPROTO_SCTP:
> > > +		return data_len >= sizeof(struct sctphdr);
> > > +	case IPPROTO_GRE:
> > > +		return data_len >= sizeof(struct gre_base_hdr);
> > > +	case IPPROTO_NONE:
> > 
> > Remove this and make it part of default and return true if protocol is
> > unknown.
> 
> Hmm.  Its likely safe to accept unknown headers, here.
> 
> Perhaps next iteration should indeed do what you suggest but also check
> check ESP and AH.
> 
> > Default is false for an unknown protocol, should be true.
> 
> I suggested it this way, i.e. don't permit unknown l4 protocols, but
> maybe its too restrictive.
> 
> > > +	if (pkt->tprot != IPPROTO_TCP)
> > > +		return true;
> > > +
> > > +	return priv->offset > doff || priv->offset + priv->len <= doff;
> >                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> > maybe simply check priv->offset >= doff here?
> 
> Could you elaborate?  The above LGTM.  priv->offset > doff is already
> tested?  I mean, write is ok either if offset exceeds doff (lhs)
> or if offset + length doesn't touch doff area (rhs).
> 
> Did you mean "just reject everything exceeding doff"?
> 
> Patch LGTM, except perhaps switching to "allow unknowns" in nfqueue.

OK, thanks for explaining.