diff mbox series

[v8,1/4] hash: fix race condition in iterate

Message ID 1540233588-202969-2-git-send-email-yipeng1.wang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series hash: add extendable bucket and partial key hashing |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wang, Yipeng1 Oct. 22, 2018, 6:39 p.m. UTC 
  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 reads out the position in the while condition,
which makes sure that the position will not be changed
to empty before entering the lock.

Fixes: f2e3001b53ec ("hash: support read/write concurrency")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Reported-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Bruce Richardson Oct. 23, 2018, 8:53 a.m. UTC | #1 
On Mon, Oct 22, 2018 at 11:39:45AM -0700, Yipeng Wang wrote:
> 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 reads out the position in the while condition,
> which makes sure that the position will not be changed
> to empty before entering the lock.
> 
> Fixes: f2e3001b53ec ("hash: support read/write concurrency")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> Reported-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
diff mbox series

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index f7b86c8..da8ddf4 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -1318,7 +1318,7 @@  rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
 	idx = *next % RTE_HASH_BUCKET_ENTRIES;
 
 	/* If current position is empty, go to the next one */
-	while (h->buckets[bucket_idx].key_idx[idx] == EMPTY_SLOT) {
+	while ((position = h->buckets[bucket_idx].key_idx[idx]) == EMPTY_SLOT) {
 		(*next)++;
 		/* End of table */
 		if (*next == total_entries)
@@ -1326,9 +1326,8 @@  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);
-	/* Get position of entry in key table */
-	position = h->buckets[bucket_idx].key_idx[idx];
 	next_key = (struct rte_hash_key *) ((char *)h->key_store +
 				position * h->key_entry_size);
 	/* Return key and data */