[PATCH nf] netfilter: nf_nat: unregister and release hooks on error

[PATCH nf] netfilter: nf_nat: unregister and release hooks on error

[Pablo Neira Ayuso]

After the base nat hooks are registered, the first nested nat hook is
added through nf_hook_entries_insert_raw() and ->users is bumped.

However, sashiko reports that nf_hook_entries_insert_raw() fails, then
->users count remains at zero and nf_nat_unregister_fn() could hit
WARN_ON() since base nat hooks with no users is unexpected in the
unregistration path.

Fixes: 1cd472bf036c ("netfilter: nf_nat: add nat hook register functions to nf_nat")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_nat_core.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 8ac326e1eb5b..71027754b5ab 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -1240,15 +1240,25 @@ int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
 	nat_ops = nat_proto_net->nat_hook_ops;
 	priv = nat_ops[hooknum].priv;
 	if (WARN_ON_ONCE(!priv)) {
-		mutex_unlock(&nf_nat_proto_mutex);
-		return -EOPNOTSUPP;
+		ret = -EOPNOTSUPP;
+		goto err_out;
 	}
 
 	ret = nf_hook_entries_insert_raw(&priv->entries, ops);
-	if (ret == 0)
-		nat_proto_net->users++;
+	if (ret)
+		goto err_out;
 
+	nat_proto_net->users++;
 	mutex_unlock(&nf_nat_proto_mutex);
+
+	return 0;
+err_out:
+	if (nat_proto_net->users == 0) {
+		nf_unregister_net_hooks(net, nat_ops, ops_count);
+		mutex_unlock(&nf_nat_proto_mutex);
+		kfree_rcu(nat_ops, rcu);
+	}
+
 	return ret;
 }
 
-- 
2.47.3

Re: [PATCH nf] netfilter: nf_nat: unregister and release hooks on error

[Pablo Neira Ayuso]

On Tue, Sep 01, 2026 at 11:35:19PM +0200, Pablo Neira Ayuso wrote:
> After the base nat hooks are registered, the first nested nat hook is
> added through nf_hook_entries_insert_raw() and ->users is bumped.
> 
> However, sashiko reports that nf_hook_entries_insert_raw() fails, then
> ->users count remains at zero and nf_nat_unregister_fn() could hit
> WARN_ON() since base nat hooks with no users is unexpected in the
> unregistration path.

Too fast, this is botched. I'll provide v2.

> Fixes: 1cd472bf036c ("netfilter: nf_nat: add nat hook register functions to nf_nat")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_nat_core.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 8ac326e1eb5b..71027754b5ab 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -1240,15 +1240,25 @@ int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
>  	nat_ops = nat_proto_net->nat_hook_ops;
>  	priv = nat_ops[hooknum].priv;
>  	if (WARN_ON_ONCE(!priv)) {
> -		mutex_unlock(&nf_nat_proto_mutex);
> -		return -EOPNOTSUPP;
> +		ret = -EOPNOTSUPP;
> +		goto err_out;
>  	}
>  
>  	ret = nf_hook_entries_insert_raw(&priv->entries, ops);
> -	if (ret == 0)
> -		nat_proto_net->users++;
> +	if (ret)
> +		goto err_out;
>  
> +	nat_proto_net->users++;
>  	mutex_unlock(&nf_nat_proto_mutex);
> +
> +	return 0;
> +err_out:
> +	if (nat_proto_net->users == 0) {
> +		nf_unregister_net_hooks(net, nat_ops, ops_count);
> +		mutex_unlock(&nf_nat_proto_mutex);
> +		kfree_rcu(nat_ops, rcu);
> +	}
> +
>  	return ret;
>  }
>  
> -- 
> 2.47.3
> 
>