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

[netdev-bot+sashiko@kernel.org]

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

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.

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