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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMArcTVrGoopQseeOLFQoHheve-qMV2iE-KCPDT8UG5o97sa7g@mail.gmail.com>
Date:   Thu, 25 May 2017 01:13:05 +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-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