Re: [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Jakub,

IP6T_RT_FST_MASK is just fine.

On Sat, Sep 19, 2026 at 01:34:49PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 0 · Low: 1
> 
> - [Low] Validator/matcher flag-semantics disagreement in
>   net/ipv6/netfilter/ip6t_rt.c.
> 
> --
> 
> netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read
> 
> This adds a loop-header bound (i < rtinfo->addrnr) to the non-strict
> address matching loop in rt_mt6(), removing the backwards break at the
> bottom of the loop.  It also adds a rt_mt6_check() rejection for rules
> that set IP6T_RT_FST_MASK with rtinfo->addrnr == 0.
> 
> > diff --git a/net/ipv6/netfilter/ip6t_rt.c b/net/ipv6/netfilter/ip6t_rt.c
> > index 8051425213dd..9880faf3cc7d 100644
> > --- a/net/ipv6/netfilter/ip6t_rt.c
> > +++ b/net/ipv6/netfilter/ip6t_rt.c
> > @@ -96,7 +96,8 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
> >  			unsigned int i = 0;
> >  
> >  			for (temp = 0;
> > -			     temp < (unsigned int)((hdrlen - 8) / 16);
> > +			     temp < (unsigned int)((hdrlen - 8) / 16) &&
> > +			     i < rtinfo->addrnr;
> >  			     temp++) {
> >  				ap = skb_header_pointer(skb,
> >  							ptr
> > @@ -112,8 +113,6 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
> >  
> >  				if (ipv6_addr_equal(ap, &rtinfo->addrs[i]))
> >  					i++;
> > -				if (i == rtinfo->addrnr)
> > -					break;
> >  			}
> >  			if (i == rtinfo->addrnr)
> >  				return ret;
> 
> [ ... ]
> 
> > @@ -162,6 +161,12 @@ static int rt_mt6_check(const struct xt_mtchk_param *par)
> >  		pr_info_ratelimited("too many addresses specified\n");
> >  		return -EINVAL;
> >  	}
> > +
> > +	if ((rtinfo->flags & IP6T_RT_FST_MASK) && !rtinfo->addrnr) {
> > +		pr_info_ratelimited("address list match requested but addrnr is 0\n");
> > +		return -EINVAL;
> > +	}
> > +
> 
> [Severity: Low]
> Is the mask used here wider than the flag the matcher actually keys on?
> IP6T_RT_FST_MASK is 0x30, covering both bits:
> 
> include/uapi/linux/netfilter_ipv6/ip6t_rt.h:
>     #define IP6T_RT_FST_MASK	0x30
>     #define IP6T_RT_FST 		0x10
>     #define IP6T_RT_FST_NSTRICT	0x20

This is from iptables:

        case O_RT0NSTRICT:
                if (!(cb->xflags & F_RT0ADDRS))
                        xtables_error(PARAMETER_PROBLEM,
                                   "`--rt-0-addr ...' required before `--rt-0-not-strict'");
                rtinfo->flags |= IP6T_RT_FST_NSTRICT;

The error message says that --rt-0-not-strict needs --rt-0-addr,
which is IP6T_RT_FST.

... that is IP6T_RT_FST_MASK.

> But rt_mt6() gates every access to rtinfo->addrs[] and rtinfo->addrnr on
> IP6T_RT_FST alone:
> 
> net/ipv6/netfilter/ip6t_rt.c:rt_mt6() {
> 	if (!(rtinfo->flags & IP6T_RT_FST)) {
> 		return ret;
> 	} else if (rtinfo->flags & IP6T_RT_FST_NSTRICT) {
> 	...
> }
> 
> So a rule with only IP6T_RT_FST_NSTRICT set and addrnr == 0 never touches
> addrs[] at match time, yet it is now rejected with -EINVAL at load time.

Yes, but IP6T_RT_FST_NSTRICT set and addrnr == 0 should not ever
happen.

This LLM is lacking userspace context.

> Would narrowing the new test to IP6T_RT_FST avoid that load-time behaviour
> change?  Such blobs can only come from direct IP6T_SO_SET_REPLACE or
> nft_compat callers, since libip6t_rt requires --rt-0-addrs before
> --rt-0-not-strict, so no ruleset produced by iptables appears to be
> affected.
> 
> Does the new check also make the strict branch's addrnr == 0 case
> unreachable?  That branch has a meaningful zero-address semantic, matching
> a routing header that carries no addresses at all (hdrlen == 8):
> 
> net/ipv6/netfilter/ip6t_rt.c:rt_mt6() {
> 	...
> 			if (temp == rtinfo->addrnr &&
> 			    temp == (unsigned int)((hdrlen - 8) / 16))
> 				return ret;
> 	...
> }
> 
> With addrnr == 0 now refused at load time, that comparison can no longer be
> satisfied with addrnr == 0.
> 
> One more thing worth noting: is the new checkentry test needed for memory
> safety at all?  The loop-header change bounds i to addrnr, and addrnr is
> already capped at IP6T_RT_HOPS by the preceding test, so &rtinfo->addrs[i]
> stays inside the 16-entry array on its own.
> 
> Separately, and not something this patch introduces: a rule with
> IP6T_RT_TYP | IP6T_RT_FST_NSTRICT, rt_type 0 and addrnr > 0 still passes
> rt_mt6_check() as an address list match, but rt_mt6() returns at the
> !(rtinfo->flags & IP6T_RT_FST) test and silently ignores every configured
> address.  Should the validator and the matcher agree on which flag means
> "match the address list"?
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918112844.194503-2-pablo%40netfilter.org