Re: [PATCH net 8/9] netfilter: ip6_tables: set F_PROTO when proto value is nonzero

[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] Stale/contradicted UAPI documentation for `struct
  ip6t_ip6::proto`.

--

netfilter: ip6_tables: set F_PROTO when proto value is nonzero

This makes the kernel set IP6T_F_PROTO for any rule that carries a nonzero
ipv6.proto, so ip6_packet_match() always performs the protocol comparison
and extension header walk instead of skipping it when userspace left the
flag clear. It mirrors the normalisation already done in nft_compat.c.

> diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
> index f42fb96ef64b6..313c4aac377aa 100644
> --- a/net/ipv6/netfilter/ip6_tables.c
> +++ b/net/ipv6/netfilter/ip6_tables.c
> @@ -647,6 +647,11 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
>  	/* Clear counters and comefrom */
>  	e->counters = ((struct xt_counters) { 0, 0 });
>  	e->comefrom = 0;
> +
> +	/* set F_PROTO, else ip6_packet_match won't do the right thing. */
> +	if (e->ipv6.proto)
> +		e->ipv6.flags |= IP6T_F_PROTO;
> +
>  	return 0;
>  }

[Severity: Low]
Should the uapi header comment be updated in the same patch?  The
description of struct ip6t_ip6::proto in
include/uapi/linux/netfilter_ipv6/ip6_tables.h still documents the
opposite contract:

	/* Upper protocol number
	 * ...
	 * - You also need to set IP6T_FLAGS_PROTO to "flags" to check protocol.
	 */
	__u16 proto;

After this change, a non-standard userspace that follows that comment and
sets proto = IPPROTO_TCP while leaving flags = 0 (expecting a
protocol-agnostic rule) gets a protocol-narrowed rule instead, and
ip6_packet_match() now takes the flag-gated branch, including the
ipv6_find_hdr() walk and its *hotdrop = true path when the lookup fails
with _frag_off == 0:

	if (ip6info->flags & IP6T_F_PROTO) {
		...
		protohdr = ipv6_find_hdr(skb, protoff, -1, &_frag_off, NULL);
		if (protohdr < 0) {
			if (_frag_off == 0)
				*hotdrop = true;

Standard ip6tables always sets IP6T_F_PROTO together with -p, so this only
affects hand-crafted rule blobs, and the resulting rule is stricter rather
than unsafe.  Still, the field's documented meaning changes here.

While in that comment, IP6T_FLAGS_PROTO does not exist anywhere in the
tree; the macro defined a few lines below is IP6T_F_PROTO.  That name is a
pre-existing typo and was not introduced by this patch, but it sits in the
same sentence this change invalidates.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907171732.1407739-2-pablo%40netfilter.org