From: bugzilla@dpdk.org
To: dev@dpdk.org
Subject: [Bug 1306] hash: internal hash key pointer overflow
Date: Fri, 27 Oct 2023 02:45:21 +0000 [thread overview]
Message-ID: <bug-1306-3@http.bugs.dpdk.org/> (raw)
[-- Attachment #1: Type: text/plain, Size: 10023 bytes --]
https://bugs.dpdk.org/show_bug.cgi?id=1306
Bug ID: 1306
Summary: hash: internal hash key pointer overflow
Product: DPDK
Version: 23.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: other
Assignee: dev@dpdk.org
Reporter: sz.kim@navercorp.com
Target Milestone: ---
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 <sz.kim@navercorp.com>
---
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
--
You are receiving this mail because:
You are the assignee for the bug.
[-- Attachment #2: Type: text/html, Size: 12497 bytes --]
reply other threads:[~2023-10-27 2:45 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bug-1306-3@http.bugs.dpdk.org/ \
--to=bugzilla@dpdk.org \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.