Re: [PATCH nf] netfilter: ip6tables: set hotdrop for malformed extension header matches

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