On Fri, Mar 01, 2019 at 01:56:06PM +0800, Su Yanjun wrote: > From: Su Yanjun <suyj.f...@cn.fujitsu.com> > > 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 <suyj.f...@cn.fujitsu.com> > --- > 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 > >
