* [PATCH NETFILTER v2] netfilter: gre: nf_ct_gre_keymap_flush() removal
@ 2021-07-01 5:02 Vasily Averin
2021-07-02 0:56 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Vasily Averin @ 2021-07-01 5:02 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: netfilter-devel, coreteam, linux-kernel
nf_ct_gre_keymap_flush() is useless.
It is called from nf_conntrack_cleanup_net_list() only and tries to remove
nf_ct_gre_keymap entries from pernet gre keymap list. Though:
a) at this point the list should already be empty, all its entries were
deleted during the conntracks cleanup, because
nf_conntrack_cleanup_net_list() executes nf_ct_iterate_cleanup(kill_all)
before nf_conntrack_proto_pernet_fini():
nf_conntrack_cleanup_net_list
+- nf_ct_iterate_cleanup
| nf_ct_put
| nf_conntrack_put
| nf_conntrack_destroy
| destroy_conntrack
| destroy_gre_conntrack
| nf_ct_gre_keymap_destroy
`- nf_conntrack_proto_pernet_fini
nf_ct_gre_keymap_flush
b) Let's say we find that the keymap list is not empty. This means netns
still has a conntrack associated with gre, in which case we should not free
its memory, because this will lead to a double free and related crashes.
However I doubt it could have gone unnoticed for years, obviously
this does not happen in real life. So I think we can remove
both nf_ct_gre_keymap_flush() and nf_conntrack_proto_pernet_fini().
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Acked-by: Florian Westphal <fw@strlen.de>
---
v2: resent, original letter was graylisted
include/net/netfilter/nf_conntrack_core.h | 1 -
net/netfilter/nf_conntrack_core.c | 1 -
net/netfilter/nf_conntrack_proto.c | 7 -------
net/netfilter/nf_conntrack_proto_gre.c | 13 -------------
4 files changed, 22 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 09f2efea0b97..13807ea94cd2 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -30,7 +30,6 @@ void nf_conntrack_cleanup_net(struct net *net);
void nf_conntrack_cleanup_net_list(struct list_head *net_exit_list);
void nf_conntrack_proto_pernet_init(struct net *net);
-void nf_conntrack_proto_pernet_fini(struct net *net);
int nf_conntrack_proto_init(void);
void nf_conntrack_proto_fini(void);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index e0befcf8113a..ad53666120e9 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2461,7 +2461,6 @@ void nf_conntrack_cleanup_net_list(struct list_head *net_exit_list)
}
list_for_each_entry(net, net_exit_list, exit_list) {
- nf_conntrack_proto_pernet_fini(net);
nf_conntrack_ecache_pernet_fini(net);
nf_conntrack_expect_pernet_fini(net);
free_percpu(net->ct.stat);
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 89e5bac384d7..2ca25aaad032 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -697,13 +697,6 @@ void nf_conntrack_proto_pernet_init(struct net *net)
#endif
}
-void nf_conntrack_proto_pernet_fini(struct net *net)
-{
-#ifdef CONFIG_NF_CT_PROTO_GRE
- nf_ct_gre_keymap_flush(net);
-#endif
-}
-
module_param_call(hashsize, nf_conntrack_set_hashsize, param_get_uint,
&nf_conntrack_htable_size, 0600);
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index db11e403d818..728eeb0aea87 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -55,19 +55,6 @@ static inline struct nf_gre_net *gre_pernet(struct net *net)
return &net->ct.nf_ct_proto.gre;
}
-void nf_ct_gre_keymap_flush(struct net *net)
-{
- struct nf_gre_net *net_gre = gre_pernet(net);
- struct nf_ct_gre_keymap *km, *tmp;
-
- spin_lock_bh(&keymap_lock);
- list_for_each_entry_safe(km, tmp, &net_gre->keymap_list, list) {
- list_del_rcu(&km->list);
- kfree_rcu(km, rcu);
- }
- spin_unlock_bh(&keymap_lock);
-}
-
static inline int gre_key_cmpfn(const struct nf_ct_gre_keymap *km,
const struct nf_conntrack_tuple *t)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH NETFILTER v2] netfilter: gre: nf_ct_gre_keymap_flush() removal
2021-07-01 5:02 [PATCH NETFILTER v2] netfilter: gre: nf_ct_gre_keymap_flush() removal Vasily Averin
@ 2021-07-02 0:56 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2021-07-02 0:56 UTC (permalink / raw)
To: Vasily Averin
Cc: Jozsef Kadlecsik, Florian Westphal, netfilter-devel, coreteam,
linux-kernel
On Thu, Jul 01, 2021 at 08:02:24AM +0300, Vasily Averin wrote:
> nf_ct_gre_keymap_flush() is useless.
> It is called from nf_conntrack_cleanup_net_list() only and tries to remove
> nf_ct_gre_keymap entries from pernet gre keymap list. Though:
> a) at this point the list should already be empty, all its entries were
> deleted during the conntracks cleanup, because
> nf_conntrack_cleanup_net_list() executes nf_ct_iterate_cleanup(kill_all)
> before nf_conntrack_proto_pernet_fini():
> nf_conntrack_cleanup_net_list
> +- nf_ct_iterate_cleanup
> | nf_ct_put
> | nf_conntrack_put
> | nf_conntrack_destroy
> | destroy_conntrack
> | destroy_gre_conntrack
> | nf_ct_gre_keymap_destroy
> `- nf_conntrack_proto_pernet_fini
> nf_ct_gre_keymap_flush
>
> b) Let's say we find that the keymap list is not empty. This means netns
> still has a conntrack associated with gre, in which case we should not free
> its memory, because this will lead to a double free and related crashes.
> However I doubt it could have gone unnoticed for years, obviously
> this does not happen in real life. So I think we can remove
> both nf_ct_gre_keymap_flush() and nf_conntrack_proto_pernet_fini().
Also applied.
I think nf_ct_gre_keymap_flush() became useless when the GRE was
de-modularized (built-in into nf_conntrack).
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-07-02 0:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 5:02 [PATCH NETFILTER v2] netfilter: gre: nf_ct_gre_keymap_flush() removal Vasily Averin
2021-07-02 0:56 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).