[PATCH net] net/sched: act_nat: Fix missing headroom COW and integer underflow in header rewriting

Muhammad Bilal <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
tcf_nat_act() accesses and rewrites IPv4, TCP, UDP, and ICMP headers
based on noff = skb_network_offset(skb).

When the network header resides in the headroom (noff < 0), two issues
occur:
1. sizeof(*iph) + noff can evaluate to a negative value or underflow
   when passed to functions expecting unsigned lengths, such as
   pskb_may_pull() and skb_try_make_writable().
2. skb_try_make_writable() only evaluates writability from skb->data
   forwards and does not invoke skb_cow() on the headroom. When modifying
   cloned SKBs (e.g. from packet sockets, tc mirred, or BPF redirects),
   in-place header modification via iph->saddr / iph->daddr mutates
   shared headroom data directly, leading to packet corruption and page
   cache corruption.

Fix this by introducing a helper nat_ensure_writable() that validates
headroom using skb_cow(skb, -offset) when offset is negative before
ensuring writability across the modified header length.

Fixes: b4219952356b ("[PKT_SCHED]: Add stateless NAT")
Signed-off-by: Muhammad Bilal <[email protected]>
---
 net/sched/act_nat.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 28cb48419616..1bf5d55b3dc4 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -112,6 +112,18 @@ static struct tc_action_ops act_nat_ops;
 
 static const struct rhashtable_params tcf_nat_ht_params;
 
+static int nat_ensure_writable(struct sk_buff *skb, int offset, size_t len)
+{
+	if (offset < 0) {
+		if (skb_cow(skb, -offset))
+			return -ENOMEM;
+		if (offset + (int)len > 0)
+			return skb_ensure_writable(skb, offset + len);
+		return 0;
+	}
+	return skb_ensure_writable(skb, offset + len);
+}
+
 TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 				  const struct tc_action *a,
 				  struct tcf_result *res)
@@ -142,7 +154,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	egress = parms->flags & TCA_NAT_FLAG_EGRESS;
 
 	noff = skb_network_offset(skb);
-	if (!pskb_may_pull(skb, sizeof(*iph) + noff))
+	if (nat_ensure_writable(skb, noff, sizeof(*iph)))
 		goto drop;
 
 	iph = ip_hdr(skb);
@@ -153,9 +165,6 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 		addr = iph->daddr;
 
 	if (!((old_addr ^ addr) & mask)) {
-		if (skb_try_make_writable(skb, sizeof(*iph) + noff))
-			goto drop;
-
 		new_addr &= mask;
 		new_addr |= addr & ~mask;
 
@@ -180,8 +189,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	{
 		struct tcphdr *tcph;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*tcph) + noff) ||
-		    skb_try_make_writable(skb, ihl + sizeof(*tcph) + noff))
+		if (nat_ensure_writable(skb, noff, ihl + sizeof(*tcph)))
 			goto drop;
 
 		tcph = (void *)(skb_network_header(skb) + ihl);
@@ -193,8 +201,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	{
 		struct udphdr *udph;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*udph) + noff) ||
-		    skb_try_make_writable(skb, ihl + sizeof(*udph) + noff))
+		if (nat_ensure_writable(skb, noff, ihl + sizeof(*udph)))
 			goto drop;
 
 		udph = (void *)(skb_network_header(skb) + ihl);
@@ -209,19 +216,19 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	{
 		struct icmphdr *icmph;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + noff))
+		if (nat_ensure_writable(skb, noff, ihl + sizeof(*icmph)))
 			goto drop;
 
 		icmph = (void *)(skb_network_header(skb) + ihl);
 
 		if (!icmp_is_err(icmph->type))
 			break;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + sizeof(*iph) +
-					noff))
+		if (nat_ensure_writable(skb, noff,
+					ihl + sizeof(*icmph) + sizeof(*iph)))
 			goto drop;
 
 		icmph = (void *)(skb_network_header(skb) + ihl);
 		iph = (void *)(icmph + 1);
 		if (egress)
 			addr = iph->daddr;
 		else
 			addr = iph->saddr;
 
 		if ((old_addr ^ addr) & mask)
 			break;
-
-		if (skb_try_make_writable(skb, ihl + sizeof(*icmph) +
-					  sizeof(*iph) + noff))
-			goto drop;
 
 		icmph = (void *)(skb_network_header(skb) + ihl);
 		iph = (void *)(icmph + 1);
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.