Re: [PATCH nf] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Mar 29, 2017 at 07:05:10PM +0800, Liping Zhang wrote:
> Hi Pablo,
> 
> 2017-03-29 18:41 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> [...]
> > Wait.
> >
> > Just a comestic change, would this look better if we just do:
> >
> >         hlist_for_each_entry(cur, &nf_ct_helper_hash[h], hnode) {
> >                  if (!strcmp(h->name, name) &&
> >                      (h->tuple.src.l3num != NFPROTO_UNSPEC &&
> >                       h->tuple.src.l3num != l3num)) &&
> 
> I think this check condition here should be:
> (h->tuple.src.l3num == NFPROTO_UNSPEC || h->tuple.src.l3num == l3num)

Yes, I noticed after sending indeed.

> >                      h->tuple.dst.protonum == protonum) {
> >                         ret = -EEXIST;
> >                         goto out;
> >                  }
> >
> >                  /* avoid unpredictable behaviour for auto_assign_helper */
> >                  if (!(me->flags & NF_CT_HELPER_F_USERSPACE)) &&
> >                      nf_ct_tuple_src_mask_cmp(&cur->tuple, &me->tuple, &mask)) {
> >                         ret = -EEXIST;
> >                         goto out;
> >         }
> >
> > __nf_conntrack_helper_find() uses the _rcu variant to iterate over the
> > helper hash table, and we don't need this given we hold the mutex.
> > This is likely going to spot false positives with the RCU debugging
> > instrumentation I think.
> 
> Right, I did not notice this point. I will send V2 later.

Thanks!