[PATCH nf-next v2] netfilter: ip6t_NPT: ensure skb is writable before header modification

[PATCH nf-next v2] netfilter: ip6t_NPT: ensure skb is writable before header modification

[Joas Antonio]

ip6t_snpt_tg() and ip6t_dnpt_tg() modify the IPv6 source/destination
address in-place via ip6t_npt_map_pfx() without first calling
skb_ensure_writable().  When the skb is cloned (nflog, tee, multicast,
tc mirred), the write hits shared data visible to other users of the
same buffer:

  if (!ip6t_npt_map_pfx(npt, &ipv6_hdr(skb)->saddr))
          ...

ip6t_npt_map_pfx() does the actual in-place modification:

  addr->s6_addr32[idx] &= mask;
  addr->s6_addr32[idx] |= ~mask & npt->dst_pfx.in6.s6_addr32[idx];

Neither target checks writability first.  Every comparable target in
the tree does: ipt_ECN (line 32), ebt_dnat (line 24), ebt_snat
(line 25), xt_TCPOPTSTRIP (line 52).

Additionally, the ICMPv6 bounced header path uses
skb_header_pointer(), which returns a pointer to a stack-local copy
for nonlinear skbs.  ip6t_npt_map_pfx() then modifies the copy;
changes are silently lost and the inner IPv6 header in ICMPv6 error
messages is never translated.

Fix both by adding skb_ensure_writable() before each target function
operates, and replacing the skb_header_pointer() + helper pattern
with skb_ensure_writable() up to the inner header offset followed by
a direct pointer.  Remove the now-unused icmpv6_bounced_ipv6hdr().

Fixes: 8a91bb0c304b ("netfilter: ip6tables: add stateless IPv6-to-IPv6 Network Prefix Translation target")
Signed-off-by: Joas Antonio <joasantonio108@gmail.com>
---
Testing: verified with NFLOG + SNPT/DNPT rules on a namespace pair
that cloned skb data is no longer corrupted after translation.
Nonlinear-skb path confirmed by forcing GRO aggregation on the
ingress side; inner ICMPv6 header now carries the translated prefix.

 net/ipv6/netfilter/ip6t_NPT.c | 66 ++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/net/ipv6/netfilter/ip6t_NPT.c b/net/ipv6/netfilter/ip6t_NPT.c
index XXXXXXX..XXXXXXX 100644
--- a/net/ipv6/netfilter/ip6t_NPT.c
+++ b/net/ipv6/netfilter/ip6t_NPT.c
@@ -78,57 +78,65 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 	return true;
 }

-static struct ipv6hdr *icmpv6_bounced_ipv6hdr(struct sk_buff *skb,
-					      struct ipv6hdr *_bounced_hdr)
-{
-	if (ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6)
-		return NULL;
-
-	if (!icmpv6_is_err(icmp6_hdr(skb)->icmp6_type))
-		return NULL;
-
-	return skb_header_pointer(skb,
-				  skb_transport_offset(skb) + sizeof(struct icmp6hdr),
-				  sizeof(struct ipv6hdr),
-				  _bounced_hdr);
-}
-
 static unsigned int
 ip6t_snpt_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct ip6t_npt_tginfo *npt = par->targinfo;
-	struct ipv6hdr _bounced_hdr;
-	struct ipv6hdr *bounced_hdr;
 	struct in6_addr bounced_pfx;
+	struct ipv6hdr *bounced_hdr;
+	unsigned int bounced_off;
+
+	if (skb_ensure_writable(skb, sizeof(struct ipv6hdr)))
+		return NF_DROP;

 	if (!ip6t_npt_map_pfx(npt, &ipv6_hdr(skb)->saddr)) {
 		icmpv6_send(skb, ICMPV6_PARAMPROB, ICMPV6_HDR_FIELD,
 			    offsetof(struct ipv6hdr, saddr));
 		return NF_DROP;
 	}

 	/* rewrite dst addr of bounced packet which was sent to dst range */
-	bounced_hdr = icmpv6_bounced_ipv6hdr(skb, &_bounced_hdr);
-	if (bounced_hdr) {
-		ipv6_addr_prefix(&bounced_pfx, &bounced_hdr->daddr, npt->src_pfx_len);
-		if (ipv6_addr_cmp(&bounced_pfx, &npt->src_pfx.in6) == 0)
-			ip6t_npt_map_pfx(npt, &bounced_hdr->daddr);
-	}
+	if (ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6 ||
+	    !icmpv6_is_err(icmp6_hdr(skb)->icmp6_type))
+		return XT_CONTINUE;
+
+	bounced_off = skb_transport_offset(skb) + sizeof(struct icmp6hdr);
+	if (skb_ensure_writable(skb, bounced_off + sizeof(struct ipv6hdr)))
+		return XT_CONTINUE;
+
+	bounced_hdr = (struct ipv6hdr *)(skb_transport_header(skb) +
+					  sizeof(struct icmp6hdr));
+	ipv6_addr_prefix(&bounced_pfx, &bounced_hdr->daddr, npt->src_pfx_len);
+	if (ipv6_addr_cmp(&bounced_pfx, &npt->src_pfx.in6) == 0)
+		ip6t_npt_map_pfx(npt, &bounced_hdr->daddr);

 	return XT_CONTINUE;
 }

 static unsigned int
 ip6t_dnpt_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct ip6t_npt_tginfo *npt = par->targinfo;
-	struct ipv6hdr _bounced_hdr;
-	struct ipv6hdr *bounced_hdr;
 	struct in6_addr bounced_pfx;
+	struct ipv6hdr *bounced_hdr;
+	unsigned int bounced_off;
+
+	if (skb_ensure_writable(skb, sizeof(struct ipv6hdr)))
+		return NF_DROP;

 	if (!ip6t_npt_map_pfx(npt, &ipv6_hdr(skb)->daddr)) {
 		icmpv6_send(skb, ICMPV6_PARAMPROB, ICMPV6_HDR_FIELD,
 			    offsetof(struct ipv6hdr, daddr));
 		return NF_DROP;
 	}

 	/* rewrite src addr of bounced packet which was sent from dst range */
-	bounced_hdr = icmpv6_bounced_ipv6hdr(skb, &_bounced_hdr);
-	if (bounced_hdr) {
-		ipv6_addr_prefix(&bounced_pfx, &bounced_hdr->saddr, npt->src_pfx_len);
-		if (ipv6_addr_cmp(&bounced_pfx, &npt->src_pfx.in6) == 0)
-			ip6t_npt_map_pfx(npt, &bounced_hdr->saddr);
-	}
+	if (ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6 ||
+	    !icmpv6_is_err(icmp6_hdr(skb)->icmp6_type))
+		return XT_CONTINUE;
+
+	bounced_off = skb_transport_offset(skb) + sizeof(struct icmp6hdr);
+	if (skb_ensure_writable(skb, bounced_off + sizeof(struct ipv6hdr)))
+		return XT_CONTINUE;
+
+	bounced_hdr = (struct ipv6hdr *)(skb_transport_header(skb) +
+					  sizeof(struct icmp6hdr));
+	ipv6_addr_prefix(&bounced_pfx, &bounced_hdr->saddr, npt->src_pfx_len);
+	if (ipv6_addr_cmp(&bounced_pfx, &npt->src_pfx.in6) == 0)
+		ip6t_npt_map_pfx(npt, &bounced_hdr->saddr);

 	return XT_CONTINUE;
 }
--
2.43.0