[PATCH nf] netfilter: nft_rbtree: ignore inactive matching element with no descendants

[PATCH nf] netfilter: nft_rbtree: ignore inactive matching element with no descendants

[Pablo Neira Ayuso]

If we find a matching element that is inactive with no descendants, we
jump to the found label, then crash because of nul-dereference on the
left branch.

Fix this by checking that the element is active and not an interval end
and skipping the logic that only applies to the tree iteration.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
Slightly larger than Florian's fix but we get rid of the goto here that
gcc consider branches with gotos as unlikely.

 net/netfilter/nft_rbtree.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
index 6473936..ffe9ae0 100644
--- a/net/netfilter/nft_rbtree.c
+++ b/net/netfilter/nft_rbtree.c
@@ -70,7 +70,6 @@ static bool nft_rbtree_lookup(const struct net *net, const struct nft_set *set,
 		} else if (d > 0)
 			parent = parent->rb_right;
 		else {
-found:
 			if (!nft_set_elem_active(&rbe->ext, genmask)) {
 				parent = parent->rb_left;
 				continue;
@@ -84,9 +83,12 @@ found:
 		}
 	}
 
-	if (set->flags & NFT_SET_INTERVAL && interval != NULL) {
-		rbe = interval;
-		goto found;
+	if (set->flags & NFT_SET_INTERVAL && interval != NULL &&
+	    nft_set_elem_active(&interval->ext, genmask) &&
+	    !nft_rbtree_interval_end(interval)) {
+		spin_unlock_bh(&nft_rbtree_lock);
+		*ext = &interval->ext;
+		return true;
 	}
 out:
 	spin_unlock_bh(&nft_rbtree_lock);
-- 
2.1.4

Re: [PATCH nf] netfilter: nft_rbtree: ignore inactive matching element with no descendants

[Anders K. Pedersen]

On man, 2016-08-01 at 13:38 +0200, Pablo Neira Ayuso wrote:
> If we find a matching element that is inactive with no descendants,
> we
> jump to the found label, then crash because of nul-dereference on the
> left branch.
> 
> Fix this by checking that the element is active and not an interval
> end
> and skipping the logic that only applies to the tree iteration.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

I applied this on top of Linux 4.7 and verified that it solves the
kernel BUGs, I previously ran into with nftables sets.

Tested-by: Anders K. Pedersen <akp@akp.dk>

Thanks,
Anders K. Pedersen