[dpdk-dev] [PATCH v4 1/4] hash: fix race condition in iterate

Honnappa Nagarahalli Honnappa.Nagarahalli at arm.com
Mon Oct 1 22:23:04 CEST 2018


> 
> In rte_hash_iterate, the reader lock did not protect the while loop which
> checks empty entry. This created a race condition that the entry may become
> empty when enters the lock, then a wrong key data value would be read out.
> 
> This commit extends the protected region.
> 
> Fixes: f2e3001b53ec ("hash: support read/write concurrency")
> Cc: stable at dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang at intel.com>
> Reported-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> b/lib/librte_hash/rte_cuckoo_hash.c
> index f7b86c8..eba13e9 100644
> --- a/lib/librte_hash/rte_cuckoo_hash.c
> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> @@ -1317,16 +1317,19 @@ rte_hash_iterate(const struct rte_hash *h, const
> void **key, void **data, uint32
>  	bucket_idx = *next / RTE_HASH_BUCKET_ENTRIES;
>  	idx = *next % RTE_HASH_BUCKET_ENTRIES;
> 
> +	__hash_rw_reader_lock(h);
This does not work well with the lock-less changes I am making.  We should leave the lock in its original position. Instead change the while loop as follows:

while ((position = h->buckets[bucket_idx].key_idx[idx]) == EMPTY_SLOT)

>  	/* If current position is empty, go to the next one */
>  	while (h->buckets[bucket_idx].key_idx[idx] == EMPTY_SLOT) {
>  		(*next)++;
>  		/* End of table */
> -		if (*next == total_entries)
> +		if (*next == total_entries) {
> +			__hash_rw_reader_unlock(h);
>  			return -ENOENT;
> +		}
>  		bucket_idx = *next / RTE_HASH_BUCKET_ENTRIES;
>  		idx = *next % RTE_HASH_BUCKET_ENTRIES;
>  	}
> -	__hash_rw_reader_lock(h);
> +
>  	/* Get position of entry in key table */
>  	position = h->buckets[bucket_idx].key_idx[idx];
If we change the while loop as I suggested as above, we can remove this line.

>  	next_key = (struct rte_hash_key *) ((char *)h->key_store +
> --
> 2.7.4



More information about the dev mailing list