* [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments
@ 2026-09-25 14:11 Axel Mierczuk
2026-09-25 14:11 ` [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6 Axel Mierczuk
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Axel Mierczuk @ 2026-09-25 14:11 UTC (permalink / raw)
To: Julian Anastasov, Simon Horman
Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David Ahern,
Ido Schimmel, Eric Dumazet, netfilter-devel, lvs-devel, coreteam,
netdev, Willy Tarreau, Keith Hoodlet, Axel Mierczuk
This series fixes a 16-byte OOB write past the verified skb area,
reachable through ICMPv6 errors that quote non-first fragments.
Following Julian's suggestion, it preserves ESP ICMPv6 handling.
Patch 1 includes the embedded IP header in ciph.len at both ICMPv6
call sites when parsing stops at a non-first fragment. The change
follows the ip_vs_fill_iph_skb_icmp() calls and does not depend on
patch 2.
Patch 2 makes ipv6_find_hdr() set *offset to the fragment payload
for non-first fragments when target < 0, so every successful return
updates it. It carries no Fixes tag because it defines an offset
that was previously unspecified.
Axel Mierczuk (2):
ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
ipv6: update *offset for non-first fragments in ipv6_find_hdr()
net/ipv6/exthdrs_core.c | 6 +++++-
net/netfilter/ipvs/ip_vs_core.c | 14 ++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
2026-09-25 14:11 [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Axel Mierczuk
@ 2026-09-25 14:11 ` Axel Mierczuk
2026-09-29 15:57 ` netdev-bot+sashiko
2026-09-25 14:11 ` [PATCH nf 2/2] ipv6: update *offset for non-first fragments in ipv6_find_hdr() Axel Mierczuk
2026-09-25 17:50 ` [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Julian Anastasov
2 siblings, 1 reply; 9+ messages in thread
From: Axel Mierczuk @ 2026-09-25 14:11 UTC (permalink / raw)
To: Julian Anastasov, Simon Horman
Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David Ahern,
Ido Schimmel, Eric Dumazet, netfilter-devel, lvs-devel, coreteam,
netdev, Willy Tarreau, Keith Hoodlet, Axel Mierczuk, stable
ipv6_find_hdr() does not set the header offset for non-first
fragments, so an ICMPv6 error embedding such an ESP fragment leaves
ip_vs_fill_iph_skb_icmp() with len == off. The NAT paths size
skb_ensure_writable() from len, and ip_vs_nat_icmp_v6() then writes
the embedded addresses after the validated area.
Include the embedded IP header in ciph.len at both ICMPv6 call
sites when parsing stops at a non-first fragment. ESP lookup does
not require a transport header. Non-first TCP, UDP and SCTP fragments
bypass IPVS ICMP handling before connection lookup.
This fix does not depend on patch 2. Stable kernels without commit
342e24a339b9 ("ipvs: do not mangle ICMP replies for non-first fragments")
also need that commit to handle incoming ICMPv6 errors quoting
non-first ESP fragments.
Fixes: 63dca2c0b0e7 ("ipvs: Fix faulty IPv6 extension header handling in IPVS")
Cc: stable@vger.kernel.org
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Axel Mierczuk <axel.mierczuk@1password.com>
---
net/netfilter/ipvs/ip_vs_core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index fd503f0efb57..cfd193196a5b 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1206,6 +1206,13 @@ static int ip_vs_out_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
true, &ciph))
return NF_ACCEPT; /* The packet looks wrong, ignore */
+ /* ipv6_find_hdr() does not include the embedded header for
+ * non-first fragments, add it so that ESP can pass and the
+ * NAT writable checks cover the rewritten addresses
+ */
+ if (ciph.len == ciph.off)
+ ciph.len += sizeof(struct ipv6hdr);
+
pp = ip_vs_proto_get(ciph.protocol);
if (!pp)
return NF_ACCEPT;
@@ -2036,6 +2043,13 @@ static int ip_vs_in_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
if (!ip_vs_fill_iph_skb_icmp(AF_INET6, skb, offset, true, &ciph))
return NF_ACCEPT;
+ /* ipv6_find_hdr() does not include the embedded header for
+ * non-first fragments, add it so that ESP can pass and the
+ * NAT writable checks cover the rewritten addresses
+ */
+ if (ciph.len == ciph.off)
+ ciph.len += sizeof(struct ipv6hdr);
+
pd = ip_vs_proto_data_get(ipvs, ciph.protocol);
if (!pd)
return NF_ACCEPT;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf 2/2] ipv6: update *offset for non-first fragments in ipv6_find_hdr()
2026-09-25 14:11 [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Axel Mierczuk
2026-09-25 14:11 ` [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6 Axel Mierczuk
@ 2026-09-25 14:11 ` Axel Mierczuk
2026-09-29 15:57 ` netdev-bot+sashiko
2026-09-25 17:50 ` [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Julian Anastasov
2 siblings, 1 reply; 9+ messages in thread
From: Axel Mierczuk @ 2026-09-25 14:11 UTC (permalink / raw)
To: Julian Anastasov, Simon Horman
Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David Ahern,
Ido Schimmel, Eric Dumazet, netfilter-devel, lvs-devel, coreteam,
netdev, Willy Tarreau, Keith Hoodlet, Axel Mierczuk
The non-first fragment early return is the only successful exit of
ipv6_find_hdr() that leaves *offset untouched. IPVS used the initial
offset as if it had been updated.
For non-first fragments with target < 0, set *offset immediately
after the Fragment header, at the start of the fragment payload.
Callers must still account for the nonzero fragment offset. A
NEXTHDR_FRAGMENT search continues to return the Fragment header's
offset. Other target < 0 callers check the returned fragment offset
or ignore *offset here.
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Axel Mierczuk <axel.mierczuk@1password.com>
---
net/ipv6/exthdrs_core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
index 4a9748338cf4..e27f5b8cc154 100644
--- a/net/ipv6/exthdrs_core.c
+++ b/net/ipv6/exthdrs_core.c
@@ -179,7 +179,10 @@ EXPORT_SYMBOL_GPL(ipv6_find_tlv);
*
* Note that non-1st fragment is special case that "the protocol number
* of last header" is "next header" field in Fragment header. In this case,
- * *offset is meaningless and fragment offset is stored in *fragoff if fragoff
+ * for target < 0, *offset points immediately after the Fragment header,
+ * at the start of the fragment payload. Callers must still account for
+ * the nonzero fragment offset before interpreting the payload. The
+ * fragment offset is stored in *fragoff if fragoff
* isn't NULL.
*
* if flags is not NULL and it's a fragment, then the frag flag
@@ -261,6 +264,7 @@ int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
hp->nexthdr == NEXTHDR_NONE)) {
if (fragoff)
*fragoff = _frag_off;
+ *offset = start + sizeof(struct frag_hdr);
return hp->nexthdr;
}
if (!found)
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments
2026-09-25 14:11 [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Axel Mierczuk
2026-09-25 14:11 ` [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6 Axel Mierczuk
2026-09-25 14:11 ` [PATCH nf 2/2] ipv6: update *offset for non-first fragments in ipv6_find_hdr() Axel Mierczuk
@ 2026-09-25 17:50 ` Julian Anastasov
2026-09-28 11:15 ` Ido Schimmel
2 siblings, 1 reply; 9+ messages in thread
From: Julian Anastasov @ 2026-09-25 17:50 UTC (permalink / raw)
To: Axel Mierczuk
Cc: Simon Horman, Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
David Ahern, Ido Schimmel, Eric Dumazet, netfilter-devel,
lvs-devel, coreteam, netdev, Willy Tarreau, Keith Hoodlet
Hello,
On Fri, 25 Sep 2026, Axel Mierczuk wrote:
> This series fixes a 16-byte OOB write past the verified skb area,
> reachable through ICMPv6 errors that quote non-first fragments.
> Following Julian's suggestion, it preserves ESP ICMPv6 handling.
>
> Patch 1 includes the embedded IP header in ciph.len at both ICMPv6
> call sites when parsing stops at a non-first fragment. The change
> follows the ip_vs_fill_iph_skb_icmp() calls and does not depend on
> patch 2.
>
> Patch 2 makes ipv6_find_hdr() set *offset to the fragment payload
> for non-first fragments when target < 0, so every successful return
> updates it. It carries no Fixes tag because it defines an offset
> that was previously unspecified.
>
> Axel Mierczuk (2):
> ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
> ipv6: update *offset for non-first fragments in ipv6_find_hdr()
>
> net/ipv6/exthdrs_core.c | 6 +++++-
> net/netfilter/ipvs/ip_vs_core.c | 14 ++++++++++++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
Both patches look good to me, thanks!
Acked-by: Julian Anastasov <ja@ssi.bg>
The ipv6 patch needs review from the net team and
may need some Fixes line:
Fixes: b777e0ce7437 ("[NETFILTER]: make ipv6_find_hdr() find transport protocol header")
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments
2026-09-25 17:50 ` [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Julian Anastasov
@ 2026-09-28 11:15 ` Ido Schimmel
2026-09-30 17:34 ` Julian Anastasov
0 siblings, 1 reply; 9+ messages in thread
From: Ido Schimmel @ 2026-09-28 11:15 UTC (permalink / raw)
To: Julian Anastasov
Cc: Axel Mierczuk, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, David Ahern, Eric Dumazet, netfilter-devel,
lvs-devel, coreteam, netdev, Willy Tarreau, Keith Hoodlet
On Fri, Sep 25, 2026 at 08:50:58PM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Fri, 25 Sep 2026, Axel Mierczuk wrote:
>
> > This series fixes a 16-byte OOB write past the verified skb area,
> > reachable through ICMPv6 errors that quote non-first fragments.
> > Following Julian's suggestion, it preserves ESP ICMPv6 handling.
> >
> > Patch 1 includes the embedded IP header in ciph.len at both ICMPv6
> > call sites when parsing stops at a non-first fragment. The change
> > follows the ip_vs_fill_iph_skb_icmp() calls and does not depend on
> > patch 2.
> >
> > Patch 2 makes ipv6_find_hdr() set *offset to the fragment payload
> > for non-first fragments when target < 0, so every successful return
> > updates it. It carries no Fixes tag because it defines an offset
> > that was previously unspecified.
> >
> > Axel Mierczuk (2):
> > ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
> > ipv6: update *offset for non-first fragments in ipv6_find_hdr()
> >
> > net/ipv6/exthdrs_core.c | 6 +++++-
> > net/netfilter/ipvs/ip_vs_core.c | 14 ++++++++++++++
> > 2 files changed, 19 insertions(+), 1 deletion(-)
>
> Both patches look good to me, thanks!
>
> Acked-by: Julian Anastasov <ja@ssi.bg>
>
> The ipv6 patch needs review from the net team and
> may need some Fixes line:
>
> Fixes: b777e0ce7437 ("[NETFILTER]: make ipv6_find_hdr() find transport protocol header")
Doesn't patch 2 make patch 1 redundant? It seems that the comments added
by patch 1 become stale with patch 2, no?
If so, why do we need both? Why not send only patch 2 and blame
63dca2c0b0e7a?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
2026-09-25 14:11 ` [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6 Axel Mierczuk
@ 2026-09-29 15:57 ` netdev-bot+sashiko
0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 15:57 UTC (permalink / raw)
To: axel.mierczuk
Cc: ja, horms, pablo, fw, phil, dsahern, idosch, edumazet,
netfilter-devel, lvs-devel, coreteam, netdev, w, keith.hoodlet,
kuba
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] Three problems with the comment and commit message; the code
change itself is correct.
--
ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
When ipv6_find_hdr() stops at a non-first fragment and leaves len equal
to off, this patch adds sizeof(struct ipv6hdr) to ciph.len in both
ip_vs_out_icmp_v6() and ip_vs_in_icmp_v6(). With that, the
skb_ensure_writable() calls in the ICMPv6 NAT paths cover the embedded
addresses that ip_vs_nat_icmp_v6() rewrites.
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index fd503f0efb578..cfd193196a5bc 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1206,6 +1206,13 @@ static int ip_vs_out_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
> true, &ciph))
> return NF_ACCEPT; /* The packet looks wrong, ignore */
>
> + /* ipv6_find_hdr() does not include the embedded header for
> + * non-first fragments, add it so that ESP can pass and the
> + * NAT writable checks cover the rewritten addresses
> + */
[Severity: Low]
The code change looks correct. Is the "so that ESP can pass" part of
this comment accurate, though?
I couldn't find anything that blocks an embedded IPv6 ESP fragment based
on ciph.len:
- Neither ip_vs_out_icmp_v6() nor ip_vs_in_icmp_v6() compares ciph.len
with ciph.off.
- The only such check is the "ciph->len - ciph->off" ihl check, and it
exists only on the IPv4 path.
- The ESP lookup reads only the addresses:
net/netfilter/ipvs/ip_vs_proto_ah_esp.c:ah_esp_conn_fill_param_proto() {
if (likely(!ip_vs_iph_inverse(iph)))
ip_vs_conn_fill_param(ipvs, af, IPPROTO_UDP,
&iph->saddr, htons(PORT_ISAKMP),
&iph->daddr, htons(PORT_ISAKMP), p);
...
}
So non-first ESP fragments already reached connection lookup and NAT
before this patch. That is how the out-of-bounds write happened.
The new value seems to be used only as ctoff in handle_response_icmp()
and as wlen in ip_vs_icmp_xmit_v6():
net/netfilter/ipvs/ip_vs_xmit.c:ip_vs_icmp_xmit_v6() {
...
wlen = ciph->len;
...
}
Would the comment be clearer if it said that len must cover the embedded
IPv6 header that ip_vs_nat_icmp_v6() rewrites? The same comment appears
again in ip_vs_in_icmp_v6().
The commit message also says:
This fix does not depend on patch 2.
Once this is applied, "patch 2" has no meaning in the git history. Could
the message name the companion commit instead: "ipv6: update *offset for
non-first fragments in ipv6_find_hdr()"?
That companion commit makes ipv6_find_hdr() set
*offset = start + sizeof(struct frag_hdr) on the non-first-fragment
early return. After it lands, can ciph.len == ciph.off still be true in
either function?
If not, the new branch can never be reached in mainline. The comment's
premise, "ipv6_find_hdr() does not include the embedded header for
non-first fragments", would then be false. Should the comment say it
applies only to kernels where ipv6_find_hdr() leaves *offset unchanged?
> + if (ciph.len == ciph.off)
> + ciph.len += sizeof(struct ipv6hdr);
> +
> pp = ip_vs_proto_get(ciph.protocol);
> if (!pp)
> return NF_ACCEPT;
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141155.17603-1-axel.mierczuk%401password.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nf 2/2] ipv6: update *offset for non-first fragments in ipv6_find_hdr()
2026-09-25 14:11 ` [PATCH nf 2/2] ipv6: update *offset for non-first fragments in ipv6_find_hdr() Axel Mierczuk
@ 2026-09-29 15:57 ` netdev-bot+sashiko
0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 15:57 UTC (permalink / raw)
To: axel.mierczuk
Cc: ja, horms, pablo, fw, phil, dsahern, idosch, edumazet,
netfilter-devel, lvs-devel, coreteam, netdev, w, keith.hoodlet,
kuba
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 1 · High: 0 · Medium: 0 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Critical] The new early-return assignment `*offset = start +
sizeof(struct frag_hdr)` in ipv6_find_hdr()…
- [Low] This patch leaves dead code and stale comments in IPVS.
- [Low] The commit message says "Other target < 0 callers check the
returned fragment offset or ignore *offset here." That is not accurate…
- [Low] The commit message justifies the change only with "IPVS used the
initial offset as if it had been updated".
--
ipv6: update *offset for non-first fragments in ipv6_find_hdr()
This commit makes ipv6_find_hdr() set *offset to the byte after the
Fragment header when it returns early for a non-first fragment with
target < 0. The kernel-doc is updated to match. The motivation given is
that IPVS used the initial offset as if it had been updated.
> The non-first fragment early return is the only successful exit of
> ipv6_find_hdr() that leaves *offset untouched. IPVS used the initial
> offset as if it had been updated.
[Severity: Low]
What user-visible problem does this fix? The message doesn't name one, and
there is no Fixes: tag. So it is hard to tell whether this is meant for
net/stable or is a semantics cleanup of an exported helper with many
in-tree callers.
The IPVS ICMPv6 paths already work around the old behaviour (previous patch
in the series). The offset still seems to matter elsewhere, though.
ip_vs_fill_iph_skb_off() does:
iphdr->len = offset;
Take an untracked non-first ESP fragment matched through
ah_esp_conn_out_get(). Before this patch iph->len stayed at 0 for it, and
handle_response() then does:
if (skb_ensure_writable(skb, iph->len))
goto drop;
...
ipv6_hdr(skb)->saddr = cp->vaddr.in6;
This patch changes the writable length used there.
If this is a fix, could the commit message describe this effect and add a
Fixes: tag?
> For non-first fragments with target < 0, set *offset immediately
> after the Fragment header, at the start of the fragment payload.
> Callers must still account for the nonzero fragment offset. A
> NEXTHDR_FRAGMENT search continues to return the Fragment header's
> offset. Other target < 0 callers check the returned fragment offset
> or ignore *offset here.
[Severity: Low]
Is the last sentence accurate for nf_tables? nft_set_pktinfo_ipv6(),
__nft_set_pktinfo_ipv6_validate() and nft_set_pktinfo_ipv6_ingress() in
include/net/netfilter/nf_tables_ipv6.h store thoff whatever frag_off is:
protohdr = ipv6_find_hdr(pkt->skb, &thoff, -1, &frag_off, &flags);
...
pkt->thoff = thoff;
pkt->fragoff = frag_off;
nf_trace_fill_pkt_info() in net/netfilter/nf_tables_trace.c then splits
the NFTA_TRACE_NETWORK_HEADER and NFTA_TRACE_TRANSPORT_HEADER dumps at
nft_thoff(pkt). It never checks pkt->fragoff:
nh_end = pkt->flags & NFT_PKTINFO_L4PROTO ? nft_thoff(pkt) : skb->len;
...
len = min_t(unsigned int, skb->len - nft_thoff(pkt), NFT_TRACETYPE_TRANSPORT_HSIZE);
Before this patch, a non-first fragment had thoff = 0. The network header
dump was empty and the transport dump held the IPv6 header.
After it, the network dump covers the IPv6, extension and Fragment
headers, and the transport dump holds fragment payload. So nft monitor
trace output changes for non-first IPv6 fragments.
Could the commit message mention this change?
The SYNPROXY callers also use this offset without checking the fragment
offset. That case is covered in the comment on the code below.
> diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
> index 4a9748338cf40..e27f5b8cc1542 100644
> --- a/net/ipv6/exthdrs_core.c
> +++ b/net/ipv6/exthdrs_core.c
[ ... ]
> @@ -261,6 +264,7 @@ int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
> hp->nexthdr == NEXTHDR_NONE)) {
> if (fragoff)
> *fragoff = _frag_off;
> + *offset = start + sizeof(struct frag_hdr);
[Severity: Critical]
Can this offset end up past skb->len? In the NEXTHDR_FRAGMENT branch,
skb_header_pointer() has only read hp (2 bytes at start) and frag_off
(2 bytes at start + 2). All that is known is skb->len >= start + 4.
This early return also skips the check that guards every other successful
exit:
if (!found) {
if (skb->len - start < hdrlen)
return -EBADMSG;
Before this patch, every successful return left *offset <= skb->len. Now
it can be as large as skb->len + 4.
Here is an example. A 44-byte IPv6 packet has payload_len = 4 and
nexthdr = 44. Its 4 bytes of Fragment header carry nexthdr = TCP and a
nonzero fragment offset. ip6_rcv_core() trims the skb to 44 bytes, and
ipv6_find_hdr() returns IPPROTO_TCP with *offset = 48.
nft_set_pktinfo_ipv6() only checks thoff > U16_MAX, so pkt->thoff = 48.
nft_synproxy_do_eval() checks pkt->tprot but not pkt->fragoff:
if (pkt->tprot != IPPROTO_TCP) {
...
}
if (nf_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP,
nft_pf(pkt))) {
That leads to:
nft_synproxy_do_eval()
nf_checksum()
nf_ip6_checksum()
skb_checksum(skb, 0, dataoff, 0)
BUG_ON(len)
nf_ip6_checksum() also computes skb->len - dataoff, which underflows.
synproxy_tg6() in ip6t_SYNPROXY is exposed the same way. It passes
par->thoff to nf_ip6_checksum() without checking par->fragoff, and
ip6_packet_match() sets par->thoff from ipv6_find_hdr().
nf_ct_frag6_gather() would drop such a packet. Untracked packets skip it,
though, because ipv6_defrag() returns NF_ACCEPT for IP_CT_UNTRACKED.
Examples are notrack in an nft chain with priority below -400, ip6table_raw
with raw_before_defrag, and a "ct state untracked synproxy" rule. An
unprivileged user can also set this up inside a user and network
namespace.
Other callers see an out-of-range offset too:
- In nf_trace_fill_pkt_info(), skb->len - nft_thoff(pkt) wraps and the
trace event is lost.
- In IPVS, a ciph.len beyond skb->len makes skb_ensure_writable() fail.
ICMPv6 errors that used to be NATed are then dropped.
The updated kernel-doc says *offset points at the start of the fragment
payload. That isn't guaranteed when the Fragment header is truncated.
Should this return -EBADMSG when skb->len - start is less than
sizeof(struct frag_hdr)? Another option is to read the full struct
frag_hdr with skb_header_pointer() before setting the offset.
[Severity: Low]
Does this assignment make the workaround from the previous patch in the
series ("ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6") dead code?
ip_vs_out_icmp_v6() and ip_vs_in_icmp_v6() in
net/netfilter/ipvs/ip_vs_core.c both contain:
/* ipv6_find_hdr() does not include the embedded header for
* non-first fragments, add it so that ESP can pass and the
* NAT writable checks cover the rewritten addresses
*/
if (ciph.len == ciph.off)
ciph.len += sizeof(struct ipv6hdr);
Both call ip_vs_fill_iph_skb_icmp() with a nonzero offset. So
ipv6_find_hdr() starts at *offset + sizeof(struct ipv6hdr).
Every successful return now writes *offset, either start or
start + sizeof(struct frag_hdr). So ciph.len can no longer equal
ciph.off. The comment at both sites now also contradicts the helper's
documented behaviour.
The previous patch needs to stand alone for stable. Could this patch, or a
follow-up, drop the check and update those comments?
> return hp->nexthdr;
> }
> if (!found)
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141155.17603-1-axel.mierczuk%401password.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments
2026-09-28 11:15 ` Ido Schimmel
@ 2026-09-30 17:34 ` Julian Anastasov
2026-10-01 16:10 ` Ido Schimmel
0 siblings, 1 reply; 9+ messages in thread
From: Julian Anastasov @ 2026-09-30 17:34 UTC (permalink / raw)
To: Ido Schimmel
Cc: Axel Mierczuk, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, David Ahern, Eric Dumazet, netfilter-devel,
lvs-devel, coreteam, netdev, Willy Tarreau, Keith Hoodlet
Hello,
On Mon, 28 Sep 2026, Ido Schimmel wrote:
> On Fri, Sep 25, 2026 at 08:50:58PM +0300, Julian Anastasov wrote:
> >
> > > Axel Mierczuk (2):
> > > ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
> > > ipv6: update *offset for non-first fragments in ipv6_find_hdr()
> > >
> > > net/ipv6/exthdrs_core.c | 6 +++++-
> > > net/netfilter/ipvs/ip_vs_core.c | 14 ++++++++++++++
> > > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > Both patches look good to me, thanks!
> >
> > Acked-by: Julian Anastasov <ja@ssi.bg>
> >
> > The ipv6 patch needs review from the net team and
> > may need some Fixes line:
> >
> > Fixes: b777e0ce7437 ("[NETFILTER]: make ipv6_find_hdr() find transport protocol header")
>
> Doesn't patch 2 make patch 1 redundant? It seems that the comments added
> by patch 1 become stale with patch 2, no?
>
> If so, why do we need both? Why not send only patch 2 and blame
> 63dca2c0b0e7a?
The idea was to have IPVS fix that can be backported
alone, if needed.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments
2026-09-30 17:34 ` Julian Anastasov
@ 2026-10-01 16:10 ` Ido Schimmel
0 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2026-10-01 16:10 UTC (permalink / raw)
To: Julian Anastasov
Cc: Axel Mierczuk, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, David Ahern, Eric Dumazet, netfilter-devel,
lvs-devel, coreteam, netdev, Willy Tarreau, Keith Hoodlet
On Wed, Sep 30, 2026 at 08:34:57PM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Mon, 28 Sep 2026, Ido Schimmel wrote:
>
> > On Fri, Sep 25, 2026 at 08:50:58PM +0300, Julian Anastasov wrote:
> > >
> > > > Axel Mierczuk (2):
> > > > ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6
> > > > ipv6: update *offset for non-first fragments in ipv6_find_hdr()
> > > >
> > > > net/ipv6/exthdrs_core.c | 6 +++++-
> > > > net/netfilter/ipvs/ip_vs_core.c | 14 ++++++++++++++
> > > > 2 files changed, 19 insertions(+), 1 deletion(-)
> > >
> > > Both patches look good to me, thanks!
> > >
> > > Acked-by: Julian Anastasov <ja@ssi.bg>
> > >
> > > The ipv6 patch needs review from the net team and
> > > may need some Fixes line:
> > >
> > > Fixes: b777e0ce7437 ("[NETFILTER]: make ipv6_find_hdr() find transport protocol header")
> >
> > Doesn't patch 2 make patch 1 redundant? It seems that the comments added
> > by patch 1 become stale with patch 2, no?
> >
> > If so, why do we need both? Why not send only patch 2 and blame
> > 63dca2c0b0e7a?
>
> The idea was to have IPVS fix that can be backported
> alone, if needed.
I assumed this was the case and it's your call, but then the appropriate
thing to do would be to wait with patch 2 until patch 1 is in net-next
and change it so that it removes the now dead code. At least that's how
I handle similar cases with my submissions.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-01 16:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-25 14:11 [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Axel Mierczuk
2026-09-25 14:11 ` [PATCH nf 1/2] ipvs: avoid out-of-bounds write in ip_vs_nat_icmp_v6 Axel Mierczuk
2026-09-29 15:57 ` netdev-bot+sashiko
2026-09-25 14:11 ` [PATCH nf 2/2] ipv6: update *offset for non-first fragments in ipv6_find_hdr() Axel Mierczuk
2026-09-29 15:57 ` netdev-bot+sashiko
2026-09-25 17:50 ` [PATCH nf 0/2] ipvs: fix OOB write when NATing ICMPv6 errors quoting non-first fragments Julian Anastasov
2026-09-28 11:15 ` Ido Schimmel
2026-09-30 17:34 ` Julian Anastasov
2026-10-01 16:10 ` Ido Schimmel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox