From: Kishore Padmanabha <[email protected]>
Seed the CRC32 hash used for TruFlow flow-offload table lookups
with a per-table random value instead of computing an unkeyed
CRC32 directly. An unkeyed hash lets an attacker who can influence
flow-key contents craft inputs that collide in the same bucket,
degrading flow insertion into worst-case behavior.
Also track whether the ULP context lock was actually acquired
during hot-upgrade failover cleanup, and only release it if so,
instead of releasing unconditionally on every cleanup path. Releasing
a lock that was never acquired corrupts the lock state and can lead
to a use-after-free on a later acquire.
Fixes: b14da6540294 ("net/bnxt/tf_ulp: use optimized CRC32 hash")
Fixes: eeefaecba000 ("net/bnxt/tf_ulp: support hot upgrade")
Cc: [email protected]
Signed-off-by: Kishore Padmanabha <[email protected]>
Signed-off-by: Manish Kurup <[email protected]>
---
drivers/net/bnxt/tf_ulp/ulp_gen_hash.c | 22 +++++++++++++++-------
drivers/net/bnxt/tf_ulp/ulp_gen_hash.h | 6 ++++++
drivers/net/bnxt/tf_ulp/ulp_tfc_ha_mgr.c | 5 ++++-
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bnxt/tf_ulp/ulp_gen_hash.c
b/drivers/net/bnxt/tf_ulp/ulp_gen_hash.c
index 74cdd5c1da..215c8f39f8 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_gen_hash.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_gen_hash.c
@@ -6,6 +6,7 @@
#include <rte_bitops.h>
#include <rte_log.h>
#include <rte_malloc.h>
+#include <rte_random.h>
#include <rte_hash_crc.h>
#include "bnxt_tf_common.h"
@@ -98,6 +99,12 @@ ulp_gen_hash_tbl_list_init(struct ulp_hash_create_params
*cparams,
return -ENOMEM;
}
*hash_table = hash_tbl;
+
+ /* Random per-table seed so the CRC hash cannot be predicted offline
+ * from attacker-controlled flow match fields.
+ */
+ hash_tbl->hash_seed = (uint32_t)rte_rand();
+
/* allocate the memory for the hash key table */
hash_tbl->num_key_entries = cparams->num_key_entries;
hash_tbl->key_tbl.data_size = cparams->key_size;
@@ -204,31 +211,32 @@ ulp_gen_hash_tbl_list_key_search(struct ulp_gen_hash_tbl
*hash_tbl,
return -EINVAL;
}
- /* calculate the hash */
+ /* calculate the hash, keyed with the table's random per-instance
+ * seed so the bucket a key lands in cannot be predicted offline.
+ */
switch (hash_tbl->key_tbl.data_size) {
case 1:
hash_id = rte_hash_crc_1byte(*entry->key_data,
- ~0U);
+ hash_tbl->hash_seed);
break;
case 2:
hash_id = rte_hash_crc_2byte(*((uint16_t *)entry->key_data),
- ~0U);
+ hash_tbl->hash_seed);
break;
case 4:
hash_id = rte_hash_crc_4byte(*((uint32_t *)entry->key_data),
- ~0U);
+ hash_tbl->hash_seed);
break;
case 8:
hash_id = rte_hash_crc_8byte(*((uint64_t *)entry->key_data),
- ~0U);
+ hash_tbl->hash_seed);
break;
default:
hash_id = rte_hash_crc(entry->key_data,
hash_tbl->key_tbl.data_size,
- ~0U);
+ hash_tbl->hash_seed);
break;
}
- hash_id = (uint16_t)(((hash_id >> 16) & 0xffff) ^ (hash_id & 0xffff));
hash_id &= hash_tbl->hash_mask;
hash_id = hash_id * hash_tbl->hash_bkt_num;
diff --git a/drivers/net/bnxt/tf_ulp/ulp_gen_hash.h
b/drivers/net/bnxt/tf_ulp/ulp_gen_hash.h
index d3f3840cbe..3aabea07ac 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_gen_hash.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_gen_hash.h
@@ -57,6 +57,12 @@ struct ulp_gen_hash_tbl {
uint32_t hash_bkt_num;
struct ulp_hash_bucket_entry *hash_list;
uint32_t hash_mask;
+ /* Per-table random seed for the CRC hash, set at init time so the
+ * bucket a given key lands in cannot be predicted/targeted offline
+ * by an attacker who controls flow match fields (hash-flooding
+ * mitigation).
+ */
+ uint32_t hash_seed;
/* Bit allocator - to allocate key_res index */
struct bit_alloc_list bit_list;
diff --git a/drivers/net/bnxt/tf_ulp/ulp_tfc_ha_mgr.c
b/drivers/net/bnxt/tf_ulp/ulp_tfc_ha_mgr.c
index be14b65804..694de307de 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_tfc_ha_mgr.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_tfc_ha_mgr.c
@@ -60,6 +60,7 @@ ulp_tfc_hot_upgrade_mgr_timer_cb(void *arg)
struct tfc *tfcp = NULL;
uint16_t fw_fid = 0;
int32_t rc = 0;
+ bool ctx_locked = false;
ulp_ctx = bnxt_ulp_cntxt_entry_acquire(arg);
if (ulp_ctx == NULL) {
@@ -67,6 +68,7 @@ ulp_tfc_hot_upgrade_mgr_timer_cb(void *arg)
ulp_tfc_hot_upgrade_mgr_timer_cb, arg);
return;
}
+ ctx_locked = true;
tfcp = bnxt_ulp_cntxt_tfcp_get(ulp_ctx);
if (unlikely(tfcp == NULL)) {
@@ -120,7 +122,8 @@ ulp_tfc_hot_upgrade_mgr_timer_cb(void *arg)
}
cleanup:
- bnxt_ulp_cntxt_entry_release();
+ if (ctx_locked)
+ bnxt_ulp_cntxt_entry_release();
if (restart_timer)
rte_eal_alarm_set(US_PER_S * ULP_HOT_UPGRADE_TIMER_SEC,
ulp_tfc_hot_upgrade_mgr_timer_cb, arg);
--
2.31.1
