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.
--
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
