Hi Pablo, > -----Original Message----- > From: Pablo Neira Ayuso [mailto:pablo@xxxxxxxxxxxxx] > Sent: Tuesday, March 21, 2017 10:30 PM > To: fgao@xxxxxxxxxx > Cc: netfilter-devel@xxxxxxxxxxxxxxx; gfree.wind@xxxxxxxxx > Subject: Re: [PATCH net-next 1/1] netfilter: helper: Remove the rcu lock in > nf_ct_helper_expectfn_find_by_name and > nf_ct_helper_expectfn_find_by_symbol. > > On Tue, Mar 14, 2017 at 04:29:05PM +0800, fgao@xxxxxxxxxx wrote: > > From: Gao Feng <fgao@xxxxxxxxxx> > > > > Because these two functions return the nf_ct_helper_expectfn pointer > > which should be protected by rcu lock. So it should makes sure the > > caller should hold the rcu lock, not inside these functions. > > > > Signed-off-by: Gao Feng <fgao@xxxxxxxxxx> > > --- > > net/netfilter/nf_conntrack_helper.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/net/netfilter/nf_conntrack_helper.c > > b/net/netfilter/nf_conntrack_helper.c > > index 6dc44d9..bce3d1f 100644 > > --- a/net/netfilter/nf_conntrack_helper.c > > +++ b/net/netfilter/nf_conntrack_helper.c > > @@ -311,38 +311,36 @@ void nf_ct_helper_expectfn_unregister(struct > > nf_ct_helper_expectfn *n) } > > EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_unregister); > > > > +/* Caller should hold the rcu lock */ > > struct nf_ct_helper_expectfn * > > nf_ct_helper_expectfn_find_by_name(const char *name) { > > struct nf_ct_helper_expectfn *cur; > > bool found = false; > > > > - rcu_read_lock(); > > list_for_each_entry_rcu(cur, &nf_ct_helper_expectfn_list, head) { > > if (!strcmp(cur->name, name)) { > > found = true; > > break; > > } > > } > > - rcu_read_unlock(); > > return found ? cur : NULL; > > } > > EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_find_by_name); > > You have to collapse this patch to: > > http://patchwork.ozlabs.org/patch/740576/ > > Please... use shorter patch subject names, around 80 chars long. There is no > strict limit that I know, but this subject looks too long. Ok. I would make it shorter, and send another update. > > I think rcu read side is missing in every invocations to: > > __nf_conntrack_helper_find() > > in ctnetlink. So this patch would be larger, have a closer look and fix this in one > go, please. No problem, I would check all callers. Best Regards Feng -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
