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: <20170524043617.GA22594@gondor.apana.org.au>
Date:   Wed, 24 May 2017 12:36:17 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Taehee Yoo <ap420073@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, ap420073@...il.com
Subject: Re: [PATCH] rhashtable: Fix missing elements when inserting.

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?

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

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