Hi Pablo, > -----Original Message----- > From: Pablo Neira Ayuso [mailto:pablo@xxxxxxxxxxxxx] > Sent: Wednesday, March 29, 2017 6:08 PM > To: gfree.wind@xxxxxxxxxxx > Cc: netfilter-devel@xxxxxxxxxxxxxxx; Gao Feng <fgao@xxxxxxxxxx> > Subject: Re: [PATCH nf-next v2 1/1] netfilter: helper: Remove useless rcu lock > when get expectfn > > On Wed, Mar 22, 2017 at 10:15:02AM +0800, gfree.wind@xxxxxxxxxxx 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> > > --- > > v2: Shorter subject, per Pablo > > v1: Initial version > > > > 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); > > nf_ct_helper_expectfn_find_by_name() is called from ctnetlink, via > ctnetlink_create_expect() and rcu read side lock is not held there. There are two reasons. 1. The rcu lock would be added in my patch " netfilter: helper: Add the rcu lock when call __nf_conntrack_helper_find" for nf https://patchwork.ozlabs.org/patch/741865/. So the ctnetlink_create_expect would hold the rcu lock after apply that patch. 2. Because these two functions return one pointer which needs RCU lock, so the caller must hold rcu lock. Or it still meets one error even though there is one rcu lock in these two functions. Because the memory which the returned pointer point to would be freed already after rcu_read_unlock. So the rcu lock is unnecessary in these functions. 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
