| Message ID | 20260901213519.945074-1-pablo@netfilter.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [nf] netfilter: nf_nat: unregister and release hooks on error | expand |
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 > >
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; }
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(-)