| Message ID | 20240904144418.1162839-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 031ae72825cef43e4650140b800ad58bf7a6a466 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ila: call nf_unregister_net_hooks() sooner | expand |
Eric Dumazet <edumazet@google.com> wrote: > syzbot found an use-after-free Read in ila_nf_input [1] > > Issue here is that ila_xlat_exit_net() frees the rhashtable, > then call nf_unregister_net_hooks(). > > It should be done in the reverse way, with a synchronize_rcu(). > > This is a good match for a pre_exit() method. Indeed, thanks Eric. Reviewed-by: Florian Westphal <fw@strlen.de>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 4 Sep 2024 14:44:18 +0000 you wrote: > syzbot found an use-after-free Read in ila_nf_input [1] > > Issue here is that ila_xlat_exit_net() frees the rhashtable, > then call nf_unregister_net_hooks(). > > It should be done in the reverse way, with a synchronize_rcu(). > > [...] Here is the summary with links: - [net] ila: call nf_unregister_net_hooks() sooner https://git.kernel.org/netdev/net/c/031ae72825ce You are awesome, thank you!
diff --git a/net/ipv6/ila/ila.h b/net/ipv6/ila/ila.h index ad5f6f6ba333027497b0c593521524ac6738b3ca..85b92917849bff2735c49007f24e017742cc53de 100644 --- a/net/ipv6/ila/ila.h +++ b/net/ipv6/ila/ila.h @@ -108,6 +108,7 @@ int ila_lwt_init(void); void ila_lwt_fini(void); int ila_xlat_init_net(struct net *net); +void ila_xlat_pre_exit_net(struct net *net); void ila_xlat_exit_net(struct net *net); int ila_xlat_nl_cmd_add_mapping(struct sk_buff *skb, struct genl_info *info); diff --git a/net/ipv6/ila/ila_main.c b/net/ipv6/ila/ila_main.c index 69caed07315f0c8bdb105de57b2baf758028f1aa..976c78efbae17021b709e138b42cd1a76db6828b 100644 --- a/net/ipv6/ila/ila_main.c +++ b/net/ipv6/ila/ila_main.c @@ -71,6 +71,11 @@ static __net_init int ila_init_net(struct net *net) return err; } +static __net_exit void ila_pre_exit_net(struct net *net) +{ + ila_xlat_pre_exit_net(net); +} + static __net_exit void ila_exit_net(struct net *net) { ila_xlat_exit_net(net); @@ -78,6 +83,7 @@ static __net_exit void ila_exit_net(struct net *net) static struct pernet_operations ila_net_ops = { .init = ila_init_net, + .pre_exit = ila_pre_exit_net, .exit = ila_exit_net, .id = &ila_net_id, .size = sizeof(struct ila_net), diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c index 67e8c9440977a40206f4e0544d28fa787922757d..534a4498e280d7603d95dd500f04158d5ad889a4 100644 --- a/net/ipv6/ila/ila_xlat.c +++ b/net/ipv6/ila/ila_xlat.c @@ -619,6 +619,15 @@ int ila_xlat_init_net(struct net *net) return 0; } +void ila_xlat_pre_exit_net(struct net *net) +{ + struct ila_net *ilan = net_generic(net, ila_net_id); + + if (ilan->xlat.hooks_registered) + nf_unregister_net_hooks(net, ila_nf_hook_ops, + ARRAY_SIZE(ila_nf_hook_ops)); +} + void ila_xlat_exit_net(struct net *net) { struct ila_net *ilan = net_generic(net, ila_net_id); @@ -626,10 +635,6 @@ void ila_xlat_exit_net(struct net *net) rhashtable_free_and_destroy(&ilan->xlat.rhash_table, ila_free_cb, NULL); free_bucket_spinlocks(ilan->xlat.locks); - - if (ilan->xlat.hooks_registered) - nf_unregister_net_hooks(net, ila_nf_hook_ops, - ARRAY_SIZE(ila_nf_hook_ops)); } static int ila_xlat_addr(struct sk_buff *skb, bool sir2ila)