* [PATCH nf] netfilter: ip6tables: set hotdrop for malformed extension header matches
@ 2026-07-09 6:30 Zhixing Chen
2026-07-09 11:02 ` Phil Sutter
0 siblings, 1 reply; 3+ messages in thread
From: Zhixing Chen @ 2026-07-09 6:30 UTC (permalink / raw)
To: Florian Westphal, Pablo Neira Ayuso
Cc: Phil Sutter, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netfilter-devel, coreteam, netdev,
Zhixing Chen
The hbh, srh and ipv6header matches have paths that return false for
malformed IPv6 extension header packets without setting hotdrop.
For hbh, strict option parsing stops when the option type or length field
cannot be read, or when advancing to the next requested option would
exceed the available header data. Mark these packets for hotdrop instead
of treating them as a rule mismatch.
For srh, keep a missing SRH as a normal mismatch, but set hotdrop when
header lookup fails for other reasons, when the SRH fixed header is not
present, when the advertised SRH length exceeds the available skb data, or
when SID selector reads fail.
For ipv6header, set hotdrop when the next extension header is not fully
present or when the advertised extension header length exceeds the
available skb data.
Returning false treats the packet as a rule mismatch. Set hotdrop for
these malformed packets so they cannot bypass rules intended to drop
packets with these IPv6 extension headers.
Signed-off-by: Zhixing Chen <running910@gmail.com>
---
This is a follow-up to the previous IPv6 extension header hotdrop fix:
https://lore.kernel.org/netfilter-devel/20260703125709.16493-6-fw@strlen.de/
---
net/ipv6/netfilter/ip6t_hbh.c | 21 +++++++++-----
| 10 +++++--
net/ipv6/netfilter/ip6t_srh.c | 42 ++++++++++++++++++++++------
3 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/net/ipv6/netfilter/ip6t_hbh.c b/net/ipv6/netfilter/ip6t_hbh.c
index 6d1a5d2026a6..a67f7d0fe93f 100644
--- a/net/ipv6/netfilter/ip6t_hbh.c
+++ b/net/ipv6/netfilter/ip6t_hbh.c
@@ -104,8 +104,10 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
break;
tp = skb_header_pointer(skb, ptr, sizeof(_opttype),
&_opttype);
- if (tp == NULL)
- break;
+ if (!tp) {
+ par->hotdrop = true;
+ return false;
+ }
/* Type check */
if (*tp != (optinfo->opts[temp] & 0xFF00) >> 8) {
@@ -120,13 +122,17 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
u16 spec_len;
/* length field exists ? */
- if (hdrlen < 2)
- break;
+ if (hdrlen < 2) {
+ par->hotdrop = true;
+ return false;
+ }
lp = skb_header_pointer(skb, ptr + 1,
sizeof(_optlen),
&_optlen);
- if (lp == NULL)
- break;
+ if (!lp) {
+ par->hotdrop = true;
+ return false;
+ }
spec_len = optinfo->opts[temp] & 0x00FF;
if (spec_len != 0x00FF && spec_len != *lp) {
@@ -147,7 +153,8 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
if ((ptr > skb->len - optlen || hdrlen < optlen) &&
temp < optinfo->optsnr - 1) {
pr_debug("new pointer is too large!\n");
- break;
+ par->hotdrop = true;
+ return false;
}
ptr += optlen;
hdrlen -= optlen;
--git a/net/ipv6/netfilter/ip6t_ipv6header.c b/net/ipv6/netfilter/ip6t_ipv6header.c
index c52ff929c93b..0568eb99eb1c 100644
--- a/net/ipv6/netfilter/ip6t_ipv6header.c
+++ b/net/ipv6/netfilter/ip6t_ipv6header.c
@@ -53,8 +53,10 @@ ipv6header_mt6(const struct sk_buff *skb, struct xt_action_param *par)
break;
}
/* Is there enough space for the next ext header? */
- if (len < (int)sizeof(struct ipv6_opt_hdr))
+ if (len < (int)sizeof(struct ipv6_opt_hdr)) {
+ par->hotdrop = true;
return false;
+ }
/* ESP -> evaluate */
if (nexthdr == NEXTHDR_ESP) {
temp |= MASK_ESP;
@@ -99,8 +101,10 @@ ipv6header_mt6(const struct sk_buff *skb, struct xt_action_param *par)
nexthdr = hp->nexthdr;
len -= hdrlen;
ptr += hdrlen;
- if (ptr > skb->len)
- break;
+ if (ptr > skb->len) {
+ par->hotdrop = true;
+ return false;
+ }
}
if (nexthdr != NEXTHDR_NONE && nexthdr != NEXTHDR_ESP)
diff --git a/net/ipv6/netfilter/ip6t_srh.c b/net/ipv6/netfilter/ip6t_srh.c
index db0fd64d8986..33a659b9b0e3 100644
--- a/net/ipv6/netfilter/ip6t_srh.c
+++ b/net/ipv6/netfilter/ip6t_srh.c
@@ -27,16 +27,25 @@ static bool srh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
struct ipv6_sr_hdr *srh;
struct ipv6_sr_hdr _srh;
int hdrlen, srhoff = 0;
+ int err;
- if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
+ err = ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL);
+ if (err < 0) {
+ if (err != -ENOENT)
+ par->hotdrop = true;
return false;
+ }
srh = skb_header_pointer(skb, srhoff, sizeof(_srh), &_srh);
- if (!srh)
+ if (!srh) {
+ par->hotdrop = true;
return false;
+ }
hdrlen = ipv6_optlen(srh);
- if (skb->len - srhoff < hdrlen)
+ if (skb->len - srhoff < hdrlen) {
+ par->hotdrop = true;
return false;
+ }
if (srh->type != IPV6_SRCRT_TYPE_4)
return false;
@@ -121,16 +130,25 @@ static bool srh1_mt6(const struct sk_buff *skb, struct xt_action_param *par)
struct in6_addr _psid, _nsid, _lsid;
struct ipv6_sr_hdr *srh;
struct ipv6_sr_hdr _srh;
+ int err;
- if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
+ err = ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL);
+ if (err < 0) {
+ if (err != -ENOENT)
+ par->hotdrop = true;
return false;
+ }
srh = skb_header_pointer(skb, srhoff, sizeof(_srh), &_srh);
- if (!srh)
+ if (!srh) {
+ par->hotdrop = true;
return false;
+ }
hdrlen = ipv6_optlen(srh);
- if (skb->len - srhoff < hdrlen)
+ if (skb->len - srhoff < hdrlen) {
+ par->hotdrop = true;
return false;
+ }
if (srh->type != IPV6_SRCRT_TYPE_4)
return false;
@@ -206,8 +224,10 @@ static bool srh1_mt6(const struct sk_buff *skb, struct xt_action_param *par)
psidoff = srhoff + sizeof(struct ipv6_sr_hdr) +
((srh->segments_left + 1) * sizeof(struct in6_addr));
psid = skb_header_pointer(skb, psidoff, sizeof(_psid), &_psid);
- if (!psid)
+ if (!psid) {
+ par->hotdrop = true;
return false;
+ }
if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_PSID,
ipv6_masked_addr_cmp(psid, &srhinfo->psid_msk,
&srhinfo->psid_addr)))
@@ -221,8 +241,10 @@ static bool srh1_mt6(const struct sk_buff *skb, struct xt_action_param *par)
nsidoff = srhoff + sizeof(struct ipv6_sr_hdr) +
((srh->segments_left - 1) * sizeof(struct in6_addr));
nsid = skb_header_pointer(skb, nsidoff, sizeof(_nsid), &_nsid);
- if (!nsid)
+ if (!nsid) {
+ par->hotdrop = true;
return false;
+ }
if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_NSID,
ipv6_masked_addr_cmp(nsid, &srhinfo->nsid_msk,
&srhinfo->nsid_addr)))
@@ -233,8 +255,10 @@ static bool srh1_mt6(const struct sk_buff *skb, struct xt_action_param *par)
if (srhinfo->mt_flags & IP6T_SRH_LSID) {
lsidoff = srhoff + sizeof(struct ipv6_sr_hdr);
lsid = skb_header_pointer(skb, lsidoff, sizeof(_lsid), &_lsid);
- if (!lsid)
+ if (!lsid) {
+ par->hotdrop = true;
return false;
+ }
if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LSID,
ipv6_masked_addr_cmp(lsid, &srhinfo->lsid_msk,
&srhinfo->lsid_addr)))
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH nf] netfilter: ip6tables: set hotdrop for malformed extension header matches
2026-07-09 6:30 [PATCH nf] netfilter: ip6tables: set hotdrop for malformed extension header matches Zhixing Chen
@ 2026-07-09 11:02 ` Phil Sutter
2026-07-13 3:10 ` Zhixing Chen
0 siblings, 1 reply; 3+ messages in thread
From: Phil Sutter @ 2026-07-09 11:02 UTC (permalink / raw)
To: Zhixing Chen
Cc: Florian Westphal, Pablo Neira Ayuso, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
netfilter-devel, coreteam, netdev
Hi,
On Thu, Jul 09, 2026 at 02:30:12PM +0800, Zhixing Chen wrote:
> The hbh, srh and ipv6header matches have paths that return false for
> malformed IPv6 extension header packets without setting hotdrop.
>
> For hbh, strict option parsing stops when the option type or length field
> cannot be read, or when advancing to the next requested option would
> exceed the available header data. Mark these packets for hotdrop instead
> of treating them as a rule mismatch.
There is another candidate for hotdrop in there, e.g. the "Packet
smaller than it's length field" check in line 76. Or is this a
legitimate non-match?
Given the many common blocks, maybe introduce a 'hotdrop' goto label to
jump to instead of break/return?
>
> For srh, keep a missing SRH as a normal mismatch, but set hotdrop when
> header lookup fails for other reasons, when the SRH fixed header is not
> present, when the advertised SRH length exceeds the available skb data, or
> when SID selector reads fail.
I think the 'srh->segments_left > srh->first_segment' case is also a
candidate:
According to RFC8200, segments_left contains the "Number of route
segments remaining, i.e., number of explicitly listed intermediate nodes
still to be visited before reaching the final destination."
RFC8754 reads: "Last Entry: contains the index (zero based), in the
Segment List, of the last element of the Segment List." ('first_segment'
is called Last Entry in there.)
AIUI, segments_left should never exceed first_segment in a packet.
Though RFC8754 mentions a case where "Segments Left is greater than Last
Entry", but it's about HMAC verification and it doesn't explain why it
should happen.
[...]
> diff --git a/net/ipv6/netfilter/ip6t_ipv6header.c b/net/ipv6/netfilter/ip6t_ipv6header.c
> index c52ff929c93b..0568eb99eb1c 100644
> --- a/net/ipv6/netfilter/ip6t_ipv6header.c
> +++ b/net/ipv6/netfilter/ip6t_ipv6header.c
> @@ -53,8 +53,10 @@ ipv6header_mt6(const struct sk_buff *skb, struct xt_action_param *par)
> break;
> }
> /* Is there enough space for the next ext header? */
> - if (len < (int)sizeof(struct ipv6_opt_hdr))
> + if (len < (int)sizeof(struct ipv6_opt_hdr)) {
> + par->hotdrop = true;
> return false;
> + }
This check is actually redundant, no? The following call to
skb_header_pointer() should discover the skb->len underrun?
Cheers, Phil
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH nf] netfilter: ip6tables: set hotdrop for malformed extension header matches
2026-07-09 11:02 ` Phil Sutter
@ 2026-07-13 3:10 ` Zhixing Chen
0 siblings, 0 replies; 3+ messages in thread
From: Zhixing Chen @ 2026-07-13 3:10 UTC (permalink / raw)
To: Phil Sutter
Cc: Florian Westphal, Pablo Neira Ayuso, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
netfilter-devel, coreteam, netdev
Hi Phil,
Thanks a lot for the review.
> There is another candidate for hotdrop in there, e.g. the "Packet
> smaller than it's length field" check in line 76. Or is this a
> legitimate non-match?
This should already be covered in the current nf/net tree by the previous
IPv6 extension header hotdrop fix, but I will double-check the base before
sending v2.
> Given the many common blocks, maybe introduce a 'hotdrop' goto label to
> jump to instead of break/return?
That makes sense to me for the repeated hbh error paths. I will look at
the flow again and use a hotdrop label where it keeps the code clearer.
> I think the 'srh->segments_left > srh->first_segment' case is also a
> candidate:
I will revisit this one and check it against RFC8200/RFC8754. My current
understanding is that this looks more like malformed input than a normal
rule mismatch, so I will account for it in v2 if the check fits cleanly.
Thanks,
Zhixing
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-13 3:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 6:30 [PATCH nf] netfilter: ip6tables: set hotdrop for malformed extension header matches Zhixing Chen
2026-07-09 11:02 ` Phil Sutter
2026-07-13 3:10 ` Zhixing Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.