[PATCH] netfilter: nft_set_hash: fix potential NULL deref in nft_rhash_deactivate

[PATCH] netfilter: nft_set_hash: fix potential NULL deref in nft_rhash_deactivate

[Melbin K Mathew]

In nft_rhash_deactivate(), rhashtable_lookup() may return NULL when the
set element is not found, but the function unconditionally returns
&he->priv.

Dereferencing a member of a NULL pointer is undefined behavior in C.
Although the current struct layout places 'priv' at offset 0 (making
this behave like returning NULL), this is fragile and relies on
implementation details.

Make the NULL case explicit and return NULL when the lookup fails.

Fixes: c07b3b683133 ("netfilter: nf_tables: add rhashtable set backend")

Signed-off-by: Melbin K Mathew <mlbnkm1@gmail.com>
---
 net/netfilter/nft_set_hash.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
index ba01ce75d6de..9ff25ebf93cf 100644
--- a/net/netfilter/nft_set_hash.c
+++ b/net/netfilter/nft_set_hash.c
@@ -231,6 +231,9 @@ nft_rhash_deactivate(const struct net *net, const struct nft_set *set,
 
 	rcu_read_unlock();
 
+	if (!he)
+		return NULL;
+
 	return &he->priv;
 }
 
-- 
2.45.2

Re: [PATCH] netfilter: nft_set_hash: fix potential NULL deref in nft_rhash_deactivate

[Florian Westphal]

Melbin K Mathew <mlbnkm1@gmail.com> wrote:
> In nft_rhash_deactivate(), rhashtable_lookup() may return NULL when the
> set element is not found, but the function unconditionally returns
> &he->priv.

Which is equal to 'return NULL' in that case.

> Dereferencing a member of a NULL pointer is undefined behavior in C.

&he->priv doesn't dereference he, it returns the address of the member.

> Although the current struct layout places 'priv' at offset 0 (making
> this behave like returning NULL), this is fragile and relies on
> implementation details.

Its not fragile, this file has:

        BUILD_BUG_ON(offsetof(struct nft_rhash_elem, priv) != 0);

to ensure 'priv is first member' requirement.