<html>
<head>
<base href="https://bugs.dpdk.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8" class="bz_new_table">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_UNCONFIRMED "
title="UNCONFIRMED - hash: internal hash key pointer overflow"
href="https://bugs.dpdk.org/show_bug.cgi?id=1306">1306</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>hash: internal hash key pointer overflow
</td>
</tr>
<tr>
<th>Product</th>
<td>DPDK
</td>
</tr>
<tr>
<th>Version</th>
<td>23.11
</td>
</tr>
<tr>
<th>Hardware</th>
<td>All
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Status</th>
<td>UNCONFIRMED
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</td>
</tr>
<tr>
<th>Priority</th>
<td>Normal
</td>
</tr>
<tr>
<th>Component</th>
<td>other
</td>
</tr>
<tr>
<th>Assignee</th>
<td>dev@dpdk.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>sz.kim@navercorp.com
</td>
</tr>
<tr>
<th>Target Milestone</th>
<td>---
</td>
</tr></table>
<p>
<div class="bz_comment_block">
<pre class="bz_comment_text">Hello,
I discovered that an overflow occurred in an internal variable used in the hash
library.
If the value of [slot_id * h->key_entry_size] is more than 32 bits,
the pointer of [new_k] will be entered with the wrong value.
Therefore, type casting to uint64_t is required for the calculation result.
- free_slots = 1 to total entries
- slot_id = total entries or less
- key_entry_size = sizeof(rte_hash_key) + sizeof(user key)
The above three variables are the internal values of the DPDK HASH.
The free_slots is a ring structure, filled with 1 to total entries.
The values of free_slots is used by slot_id.
And the key_entry_size becomes the size of the key value set by the user.
The following example shows a situation where the issue occurs.
Example)
- global variable :
entries = 2^28
key_entry_size = 32
- no issue case :
2^26(slot_id) * 32(key_entry_size) = 0x80000000 (32bit)
- issue case :
2^27(slot_id) * 32(key_entry_size) = 0x100000000 (33bit)
2^28(slot_id) * 32(key_entry_size) = 0x200000000 (34bit)
I think it should be changed as follows diff code. Is that correct?
Signed-off-by: SeongJoong Kim <<a href="mailto:sz.kim@navercorp.com">sz.kim@navercorp.com</a>>
---
lib/hash/rte_cuckoo_hash.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index d92a903bb3..10ffa500d2 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -636,7 +636,7 @@ rte_hash_reset(struct rte_hash *h)
}
memset(h->buckets, 0, h->num_buckets * sizeof(struct rte_hash_bucket));
- memset(h->key_store, 0, h->key_entry_size * (h->entries + 1));
+ memset(h->key_store, 0, (uint64_t) h->key_entry_size * (h->entries + 1));
*h->tbl_chng_cnt = 0;
/* reset the free ring */
@@ -705,7 +705,7 @@ search_and_update(const struct rte_hash *h, void *data,
const void *key,
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
if (bkt->sig_current[i] == sig) {
k = (struct rte_hash_key *) ((char *)keys +
- bkt->key_idx[i] * h->key_entry_size);
+ (uint64_t) bkt->key_idx[i] * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
/* The store to application data at *data
* should not leak after the store to pdata
@@ -1071,7 +1071,7 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h,
const void *key,
return -ENOSPC;
}
- new_k = RTE_PTR_ADD(keys, slot_id * h->key_entry_size);
+ new_k = RTE_PTR_ADD(keys, (uint64_t) slot_id * h->key_entry_size);
/* The store to application data (by the application) at *data should
* not leak after the store of pdata in the key store. i.e. pdata is
* the guard variable. Release the application data to the readers.
@@ -1256,7 +1256,7 @@ search_one_bucket_l(const struct rte_hash *h, const void
*key,
if (bkt->sig_current[i] == sig &&
bkt->key_idx[i] != EMPTY_SLOT) {
k = (struct rte_hash_key *) ((char *)keys +
- bkt->key_idx[i] * h->key_entry_size);
+ (uint64_t) bkt->key_idx[i] * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
if (data != NULL)
@@ -1294,7 +1294,7 @@ search_one_bucket_lf(const struct rte_hash *h, const void
*key, uint16_t sig,
__ATOMIC_ACQUIRE);
if (key_idx != EMPTY_SLOT) {
k = (struct rte_hash_key *) ((char *)keys +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
if (data != NULL) {
@@ -1492,7 +1492,7 @@ __hash_rcu_qsbr_free_resource(void *p, void *e, unsigned
int n)
keys = h->key_store;
k = (struct rte_hash_key *) ((char *)keys +
- rcu_dq_entry.key_idx * h->key_entry_size);
+ (uint64_t) rcu_dq_entry.key_idx * h->key_entry_size);
key_data = k->pdata;
if (h->hash_rcu_cfg->free_key_data_func)
h->hash_rcu_cfg->free_key_data_func(h->hash_rcu_cfg->key_data_ptr,
@@ -1654,7 +1654,7 @@ search_and_remove(const struct rte_hash *h, const void
*key,
__ATOMIC_ACQUIRE);
if (bkt->sig_current[i] == sig && key_idx != EMPTY_SLOT) {
k = (struct rte_hash_key *) ((char *)keys +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
bkt->sig_current[i] = NULL_SIGNATURE;
/* Free the key store index if
@@ -1806,8 +1806,8 @@ rte_hash_get_key_with_position(const struct rte_hash *h,
const int32_t position,
RETURN_IF_TRUE(((h == NULL) || (key == NULL)), -EINVAL);
struct rte_hash_key *k, *keys = h->key_store;
- k = (struct rte_hash_key *) ((char *) keys + (position + 1) *
- h->key_entry_size);
+ k = (struct rte_hash_key *) ((char *) keys +
+ (uint64_t) (position + 1) * h->key_entry_size);
*key = k->key;
if (position !=
@@ -1934,7 +1934,7 @@ __bulk_lookup_l(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
rte_prefetch0(key_slot);
continue;
}
@@ -1948,7 +1948,7 @@ __bulk_lookup_l(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
rte_prefetch0(key_slot);
}
}
@@ -1965,7 +1965,7 @@ __bulk_lookup_l(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
/*
* If key index is 0, do not compare key,
@@ -1993,7 +1993,7 @@ __bulk_lookup_l(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
/*
* If key index is 0, do not compare key,
@@ -2091,7 +2091,7 @@ __bulk_lookup_lf(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
rte_prefetch0(key_slot);
continue;
}
@@ -2105,7 +2105,7 @@ __bulk_lookup_lf(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
rte_prefetch0(key_slot);
}
}
@@ -2123,7 +2123,7 @@ __bulk_lookup_lf(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
/*
* If key index is 0, do not compare key,
@@ -2155,7 +2155,7 @@ __bulk_lookup_lf(const struct rte_hash *h, const void
**keys,
const struct rte_hash_key *key_slot =
(const struct rte_hash_key *)(
(const char *)h->key_store +
- key_idx * h->key_entry_size);
+ (uint64_t) key_idx * h->key_entry_size);
/*
* If key index is 0, do not compare key,
@@ -2506,7 +2506,7 @@ rte_hash_iterate(const struct rte_hash *h, const void
**key, void **data, uint32
__hash_rw_reader_lock(h);
next_key = (struct rte_hash_key *) ((char *)h->key_store +
- position * h->key_entry_size);
+ (uint64_t) position * h->key_entry_size);
/* Return key and data */
*key = next_key->key;
*data = next_key->pdata;
@@ -2537,7 +2537,7 @@ rte_hash_iterate(const struct rte_hash *h, const void
**key, void **data, uint32
}
__hash_rw_reader_lock(h);
next_key = (struct rte_hash_key *) ((char *)h->key_store +
- position * h->key_entry_size);
+ (uint64_t) position * h->key_entry_size);
/* Return key and data */
*key = next_key->key;
*data = next_key->pdata;
--
2.31.1
</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
<link itemprop="url" href="https://bugs.dpdk.org/show_bug.cgi?id=1306">
<meta itemprop="name" content="View bug">
</div>
<meta itemprop="description" content="Bugzilla bug update notification">
</div>
</body>
</html>