netdev - Re: [PATCH] rhashtable: Fix missing elements when inserting.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 25 May 2017 01:17:16 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH] rhashtable: Fix missing elements when inserting.

2017-05-25 1:13 GMT+09:00 Taehee Yoo <ap420073@...il.com>:
> 2017-05-24 13:36 GMT+09:00 Herbert Xu <herbert@...dor.apana.org.au>:
>> Taehee Yoo <ap420073@...il.com> wrote:
>>> rhltable_insert_key() inserts a node into list of element,
>>> if node's key is duplicated, so that it becomes the chain of
>>> element(as known as rhead). Also bucket table points that element directly.
>>> If a inserted node's element chain is located at third,
>>> rhltable misses first and second element chain.
>>> This issue is causion of to failture the rhltable_remove().
>>>
>>> After this patch, rhltable_insert_key() inserts a node into second of
>>> element's list, so that rhlist do not misses elements.
>>>
>>> Signed-off-by: Taehee Yoo <ap420073@...il.com>
>>
>> I'm sorry but I don't understand your description of the problem.
>> The new duplicate object is always inserted at the head of the
>> list and therefore replaces the previous list head in the hash
>> bucket chain.
>>
>> Do you have some code that reproduces the problem?
>>
> I tested this problem using netfilter, therefore a attached diff file
> modifies netfilter source code.
> That diff prints fail messages if return value of rhltable_remove() is -ENOENT.
>
> I used below commands
>
> sysctl -w net.netfilter.nf_conntrack_udp_timeout=100000
> sysctl -w net.netfilter.nf_conntrack_max=1000000
> iptables -t nat -A POSTROUTING -j MASQUERADE
>
> hping3 < ip address > -2 -p 1 --flood
> hping3 < ip address > -2 -p 2 --flood
> conntrack -D > /dev/null
> we can see "nf_nat_cleanup_conntrack, not found"
>
>>> diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
>>> index 7d56a7e..d3c24b9 100644
>>> --- a/include/linux/rhashtable.h
>>> +++ b/include/linux/rhashtable.h
>>> @@ -762,11 +762,9 @@ static inline void *__rhashtable_insert_fast(
>>>                list = container_of(obj, struct rhlist_head, rhead);
>>>                plist = container_of(head, struct rhlist_head, rhead);
>>>
>>> -               RCU_INIT_POINTER(list->next, plist);
>>> -               head = rht_dereference_bucket(head->next, tbl, hash);
>>> -               RCU_INIT_POINTER(list->rhead.next, head);
>>> -               rcu_assign_pointer(*pprev, obj);
>>> -
>>> +               RCU_INIT_POINTER(list->next, rht_dereference_bucket(plist->next,
>>> +                                                                   tbl, hash));
>>> +               RCU_INIT_POINTER(plist->next, list);
>>
>> Your second RCU_INIT_POINTER needs to be rcu_assign_pointer.
>>
> Thank you, I won't forget this.
>
>> Your approach of retaining the first duplicate object as the head
>> of the list should work too but I don't really see any point in
>> changing this.
>>
>> Cheers,
>> --
>> Email: Herbert Xu <herbert@...dor.apana.org.au>
>> Home Page: http://gondor.apana.org.au/~herbert/
>> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

I'm so sorry, I did not attach diff file.

View attachment "rhashtable_test.diff" of type "text/plain" (733 bytes)