On Fri, Apr 14, 2017 at 07:37:50AM +0800, Liping Zhang wrote:
> Hi Pablo,
>
> 2017-04-14 7:16 GMT+08:00 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>:
> [...]
> >> #ifndef _NF_CONNTRACK_HELPER_H
> >> #define _NF_CONNTRACK_HELPER_H
> >> +#include <linux/refcount.h>
> >> #include <net/netfilter/nf_conntrack.h>
> >> #include <net/netfilter/nf_conntrack_extend.h>
> >> #include <net/netfilter/nf_conntrack_expect.h>
> >> @@ -26,6 +27,7 @@ struct nf_conntrack_helper {
> >> struct hlist_node hnode; /* Internal use. */
> >>
> >> char name[NF_CT_HELPER_NAME_LEN]; /* name of the module */
> >> + refcount_t refcnt;
> >
> > Should this new refcnt; thing be in the new struct nfnl_cthelper?
> >
> > I think this refcount is only required by the userspace helper
> > infrastructure, not existing in-kernel helpers.
> >
> > I think like that we can skip patch 1/2 in this series.
>
> We must call nf_conntrack_helper_try_module_get to get the helper,
> either it is userspace or in-kernel helpers. Also the caller didn't care
> the helpers is userspace or in-kernel, so I think introducing the
> nf_conntrack_helper_put is necessary.
>
> Also, this path set is prepared for per-net helper. For in-kernel helpers,
> we will still need to kmemdup the original one. I mean:
> helper = kmemdup(ftp_helper);
> helper->expect_policy = kmemdup(ftp_exp_policy);
> nf_conntrack_helper_register(net, helper);
>
> And similar to nfnetlink_cttimeout, for net_exit, we should:
> list_for_each_entry_safe()
> unregister_helper();
> if (refcount_dec_and_test(&cur->refcnt))
> call_rcu(free_xxx);
>
> For nf_conntrack_helper_put, we should:
> if (refcount_dec_and_test(&cur->refcnt))
> call_rcu(free_xxx);
>
> So I think put this "refcount_t refcnt" to struct nf_conntrack_helper
> is better.
OK, please resubmit target to nf-next. It's too late for patches
already for nf, we're at -rc6 and this problem has been there for a
bit of time. So waiting a bit more to get this fixed should be OK.
Thanks.
--
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
