* [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
* 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
* [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 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-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 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