* [PATCH net] netfilter: ip6t_NPT: ensure skb is writable before header modification
@ 2026-09-30 18:09 Joas Antonio
2026-09-30 20:49 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Joas Antonio @ 2026-09-30 18:09 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo, fw, coreteam
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH net] netfilter: ip6t_NPT: ensure skb is writable before header modification
2026-09-30 18:09 [PATCH net] netfilter: ip6t_NPT: ensure skb is writable before header modification Joas Antonio
@ 2026-09-30 20:49 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-30 20:49 UTC (permalink / raw)
To: Joas Antonio; +Cc: netfilter-devel, fw, coreteam
Hi,
On Wed, Sep 30, 2026 at 11:09:39AM -0700, Joas Antonio wrote:
> 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.
This is nf-next material.
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-30 20:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-30 18:09 [PATCH net] netfilter: ip6t_NPT: ensure skb is writable before header modification Joas Antonio
2026-09-30 20:49 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox