Re: [PATCH] netfilter: nf_ct_helper: Fix possible panic when nf_conntrack_helper_unregister is used in an unloadable module
On Fri, Mar 01, 2019 at 01:56:06PM +0800, Su Yanjun wrote: > From: Su Yanjun > > Because nf_conntrack_helper_unregister maybe used in an unloadable module, > it uses 'synchronize_rcu' which may cause kernel panic. > > According to the artical: > RCU and Unloadable Modules > https://lwn.net/Articles/217484/ > > When we have a heavy rcu callback load, then some of the callbacks might be > deferred in order to allow other processing to proceed. sychnorize_rcu does > not wait rcu callback complete and module may be unloaded before callback > done. > > This patch uses rcu_barrier instead of synchronize_rcu will prevent this > situation. > > Signed-off-by: Su Yanjun > --- > net/netfilter/nf_conntrack_helper.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_helper.c > b/net/netfilter/nf_conntrack_helper.c > index 274baf1..0ee9378 100644 > --- a/net/netfilter/nf_conntrack_helper.c > +++ b/net/netfilter/nf_conntrack_helper.c > @@ -397,8 +397,15 @@ void nf_conntrack_helper_unregister(struct > nf_conntrack_helper *me) > > /* Make sure every nothing is still using the helper unless its a >* connection in the hash. > + * > + * 'synchronize_rcu' may have problem when rcu callback function > + * is used in unloadable modules. Use rcu_barrier instead, so that > + * it will wait until rcu callback completes before modules are > + * unloaded. > + * More detail about rcu_barrier please see: > + * https://lwn.net/Articles/217484/ >*/ > - synchronize_rcu(); > + rcu_barrier(); Are you sure this is correct? IIRC rcu_barrier() makes sure no pending callback is still waiting in the queue to run. We have don't use call_rcu() in this code, which is what rcu_barrier() is meant for. Please correct me if I'm mistaken. Thanks! > > nf_ct_expect_iterate_destroy(expect_iter_me, NULL); > nf_ct_iterate_destroy(unhelp, me); > @@ -406,7 +413,7 @@ void nf_conntrack_helper_unregister(struct > nf_conntrack_helper *me) > /* Maybe someone has gotten the helper already when unhelp above. >* So need to wait it. >*/ > - synchronize_rcu(); > + rcu_barrier(); > } > EXPORT_SYMBOL_GPL(nf_conntrack_helper_unregister); > > -- > 2.7.4 > >
Re: [PATCH] netfilter: nf_ct_helper: Fix possible panic when nf_conntrack_helper_unregister is used in an unloadable module
On 2019/3/1 16:43, Sergei Shtylyov wrote: Hello! On 01.03.2019 8:56, Su Yanjun wrote: From: Su Yanjun Because nf_conntrack_helper_unregister maybe used in an unloadable module, it uses 'synchronize_rcu' which may cause kernel panic. According to the artical: Article? I got it. RCU and Unloadable Modules https://lwn.net/Articles/217484/ When we have a heavy rcu callback load, then some of the callbacks might be deferred in order to allow other processing to proceed. sychnorize_rcu does not wait rcu callback complete and module may be unloaded before callback done. This patch uses rcu_barrier instead of synchronize_rcu will prevent this ^ that/which missed? Yes. situation. Signed-off-by: Su Yanjun [...] MBR, Sergei Thanks. Su
Re: [PATCH] netfilter: nf_ct_helper: Fix possible panic when nf_conntrack_helper_unregister is used in an unloadable module
Hello! On 01.03.2019 8:56, Su Yanjun wrote: From: Su Yanjun Because nf_conntrack_helper_unregister maybe used in an unloadable module, it uses 'synchronize_rcu' which may cause kernel panic. According to the artical: Article? RCU and Unloadable Modules https://lwn.net/Articles/217484/ When we have a heavy rcu callback load, then some of the callbacks might be deferred in order to allow other processing to proceed. sychnorize_rcu does not wait rcu callback complete and module may be unloaded before callback done. This patch uses rcu_barrier instead of synchronize_rcu will prevent this ^ that/which missed? situation. Signed-off-by: Su Yanjun [...] MBR, Sergei
[PATCH] netfilter: nf_ct_helper: Fix possible panic when nf_conntrack_helper_unregister is used in an unloadable module
From: Su Yanjun Because nf_conntrack_helper_unregister maybe used in an unloadable module, it uses 'synchronize_rcu' which may cause kernel panic. According to the artical: RCU and Unloadable Modules https://lwn.net/Articles/217484/ When we have a heavy rcu callback load, then some of the callbacks might be deferred in order to allow other processing to proceed. sychnorize_rcu does not wait rcu callback complete and module may be unloaded before callback done. This patch uses rcu_barrier instead of synchronize_rcu will prevent this situation. Signed-off-by: Su Yanjun --- net/netfilter/nf_conntrack_helper.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c index 274baf1..0ee9378 100644 --- a/net/netfilter/nf_conntrack_helper.c +++ b/net/netfilter/nf_conntrack_helper.c @@ -397,8 +397,15 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me) /* Make sure every nothing is still using the helper unless its a * connection in the hash. +* +* 'synchronize_rcu' may have problem when rcu callback function +* is used in unloadable modules. Use rcu_barrier instead, so that +* it will wait until rcu callback completes before modules are +* unloaded. +* More detail about rcu_barrier please see: +* https://lwn.net/Articles/217484/ */ - synchronize_rcu(); + rcu_barrier(); nf_ct_expect_iterate_destroy(expect_iter_me, NULL); nf_ct_iterate_destroy(unhelp, me); @@ -406,7 +413,7 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me) /* Maybe someone has gotten the helper already when unhelp above. * So need to wait it. */ - synchronize_rcu(); + rcu_barrier(); } EXPORT_SYMBOL_GPL(nf_conntrack_helper_unregister); -- 2.7.4
