netdev - Re: [net-next,10/11] netfilter: nft_set_rbtree: validate open interval overlap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260206020816.3174524-1-kuba@kernel.org>
Date: Thu,  5 Feb 2026 18:08:16 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: fw@...len.de
Cc: Jakub Kicinski <kuba@...nel.org>,
	edumazet@...gle.com,
	davem@...emloft.net,
	pablo@...filter.org,
	pabeni@...hat.com,
	netfilter-devel@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [net-next,10/11] netfilter: nft_set_rbtree: validate open interval overlap

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
netfilter: nft_set_rbtree: validate open interval overlap

This patch adds validation for open interval overlaps in the nft_set_rbtree
implementation. It introduces a flag field to struct nft_set_elem to track
when processing the last element in add/delete commands, using this in
combination with a start element cookie to detect partial overlaps.

Fixes: 7c84d41416d8 ("netfilter: nft_set_rbtree: Detect partial overlaps on insertion")

> diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> --- a/net/netfilter/nft_set_rbtree.c
> +++ b/net/netfilter/nft_set_rbtree.c

[ ... ]

> @@ -459,7 +477,26 @@
>  	if (rbe_ge && !nft_rbtree_cmp(set, new, rbe_ge) &&
>  	    nft_rbtree_interval_start(rbe_ge) == nft_rbtree_interval_start(new)) {
>  		*elem_priv = &rbe_ge->priv;
> -		nft_rbtree_set_start_cookie(priv, rbe_ge);
> +
> +		/* - Corner case: new start element of open interval (which
> +		 *   comes as last element in the batch) overlaps the start of
> +		 *   an existing interval with an end element: partial overlap.
> +		 */
> +		node = rb_first(&priv->root);
> +		rbe = __nft_rbtree_next_active(node, genmask);
> +		if (nft_rbtree_interval_end(rbe)) {
> +			rbe = nft_rbtree_next_active(rbe, genmask);
> +			if (nft_rbtree_interval_start(rbe) &&
                            ^^^

Can nft_rbtree_next_active() return NULL here? Looking at the code, if the
tree contains only a null interval end marker, nft_rbtree_next_active()
would return NULL when there are no more active elements. The subsequent
call to nft_rbtree_interval_start(rbe) would then dereference NULL.

Would it be safer to add a NULL check before accessing rbe?

> +			    !nft_rbtree_cmp(set, new, rbe)) {
> +				if (last)
> +					return -ENOTEMPTY;
> +
> +				/* Maybe open interval? */
> +				open_interval = NFT_RBTREE_OPEN_INTERVAL;
> +			}
> +		}
> +		nft_rbtree_set_start_cookie_open(priv, rbe_ge, open_interval);
> +
>  		return -EEXIST;
>  	}