2017-03-21 18:33 GMT+08:00 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>:
> On Sun, Mar 19, 2017 at 10:36:02PM +0800, Liping Zhang wrote:
>> From: Liping Zhang <zlpnobody@xxxxxxxxx>
>>
>> The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while
>> nfct_helper operation is protected by nfnl_lock(NFNL_SUBSYS_CTHELPER).
>> So it's possible that one CPU is walking the nf_ct_helper_hash for
>> cthelper add/get/del, another cpu is doing nf_conntrack_helpers_unregister
>> at the same time. This is dangrous, and may cause use after free error.
>>
>> Note, delete operation will flush all cthelpers added via nfnetlink, so
>> using rcu to do protect is not easy.
>>
>> Now introduce a dummy list to record all the cthelpers added via
>> nfnetlink, then we can walk the dummy list instead of walking the
>> nf_ct_helper_hash. Also, keep nfnl_cthelper_dump_table unchanged, it
>> may be invoked without nfnl_lock(NFNL_SUBSYS_CTHELPER) held.
>>
>> Signed-off-by: Liping Zhang <zlpnobody@xxxxxxxxx>
>> ---
>> net/netfilter/nfnetlink_cthelper.c | 176 +++++++++++++++++++------------------
>> 1 file changed, 89 insertions(+), 87 deletions(-)
>>
>> diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
>> index fc4733f..47424ec 100644
>> --- a/net/netfilter/nfnetlink_cthelper.c
>> +++ b/net/netfilter/nfnetlink_cthelper.c
>> @@ -32,6 +32,13 @@ MODULE_LICENSE("GPL");
>> MODULE_AUTHOR("Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>");
>> MODULE_DESCRIPTION("nfnl_cthelper: User-space connection tracking helpers");
>>
>> +struct nfnl_cthelper {
>> + struct list_head list;
>> + struct nf_conntrack_helper *helper;
>> +};
>> +
>> +static LIST_HEAD(nfnl_cthelper_list);
>
> We need a field possible_net_t so we can store what netns this helper
> belongs to, thus in case of flush command, we just remove the helpers
> that this netns owns.
Yes, good point, I will send v2.
Thanks Pablo.
--
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
