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

Pablo Neira Ayuso <[email protected]>
Newsgroups gmane.comp.security.firewalls.netfilter.devel
Message-ID <apdJ27G-YYAkr7fa@chamomile>
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 <[email protected]>
> ---
>  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
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.