From: Mohammad Shuab Siddique <[email protected]>
tfc_tbl_scope_mem_free() contains a race window: the
tfo_ts_get_mem_cfg() / tfo_ts_set_mem_cfg() pair allows concurrent
teardown paths to each observe a non-zero mem_cfg and both proceed
to call unlink_and_free() on the same backing-store memory, causing
a use-after-free.
Replace the get/set pair with tfo_ts_get_and_clear_mem_cfg(), which
atomically reads and zeros the slot under rte_spinlock in a single
operation. A concurrent caller receives num_lvl == 0 and skips the
free. Memory release happens outside the lock so the spinlock is
never held across blocking operations.
Fixes: 80317ff6adfd ("net/bnxt/tf_core: support Thor2")
Cc: [email protected]
Signed-off-by: Mohammad Shuab Siddique <[email protected]>
Signed-off-by: Manish Kurup <[email protected]>
---
drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c | 17 +++---
drivers/net/bnxt/tf_core/v3/tfo.c | 57 +++++++++++++++++++++
drivers/net/bnxt/tf_core/v3/tfo.h | 30 +++++++++++
3 files changed, 95 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c
b/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c
index c06099af12..0a5dce43fa 100644
--- a/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c
+++ b/drivers/net/bnxt/tf_core/v3/tfc_tbl_scope.c
@@ -1437,22 +1437,21 @@ int tfc_tbl_scope_mem_free(struct tfc *tfcp, uint16_t
fid, uint8_t tsid,
for (region = 0; region < CFA_REGION_TYPE_MAX; region++) {
for (dir = 0; dir < CFA_DIR_MAX; dir++) {
- lrc = tfo_ts_get_mem_cfg(tfcp->tfo, tsid, dir, region,
&local,
- &mem_cfg);
+ lrc = tfo_ts_get_and_clear_mem_cfg(tfcp->tfo, tsid,
dir, region,
+ &local, &mem_cfg);
if (lrc) {
rc = lrc;
continue;
}
+ if (!mem_cfg.num_lvl) {
+ PMD_DRV_LOG_LINE(DEBUG,
+ "tsid(%d) dir(%d) region(%d)
already freed",
+ tsid, dir, region);
+ continue;
+ }
/* memory only allocated on PF */
if (is_pf)
unlink_and_free(&mem_cfg,
mem_cfg.pg_tbl[0].pg_size);
-
- memset(&mem_cfg, 0, sizeof(mem_cfg));
-
- /* memory freed, set local to false */
- local = false;
- (void)tfo_ts_set_mem_cfg(tfcp->tfo, tsid, dir, region,
local,
- &mem_cfg);
}
}
if (rc) {
diff --git a/drivers/net/bnxt/tf_core/v3/tfo.c
b/drivers/net/bnxt/tf_core/v3/tfo.c
index 681d1dd8d3..927b3a7640 100644
--- a/drivers/net/bnxt/tf_core/v3/tfo.c
+++ b/drivers/net/bnxt/tf_core/v3/tfo.c
@@ -36,6 +36,9 @@ struct tfc_global_object {
uint8_t gtsid;
struct tfc_tsid_db gtsid_db;
void *gts_tim;
+ rte_spinlock_t mem_cfg_lock; /**< serialises mem_cfg take/set across
+ * ports sharing this global scope
+ */
};
struct tfc_global_object tfc_global;
@@ -58,6 +61,7 @@ struct tfc_object {
uint16_t sid; /**< Session ID */
bool is_pf; /**< port is a PF */
struct cfa_bld_mpcinfo mpc_info; /**< MPC ops handle */
+ rte_spinlock_t mem_cfg_lock; /**< serialises mem_cfg take/set */
struct tfc_tsid_db tsid_db[TFC_TBL_SCOPE_MAX]; /**< tsid database */
/** TIM instance pointer (PF) - this is where the 4 instances
* of the TPM (rx/tx_lkup, rx/tx_act) will be stored per shared
@@ -87,6 +91,7 @@ void tfo_open(void **tfo, bool is_pf)
tfco->is_pf = is_pf;
tfco->sid = INVALID_SID;
tfco->ts_tim = NULL;
+ rte_spinlock_init(&tfco->mem_cfg_lock);
/* Bind to the MPC builder */
rc = cfa_bld_mpc_bind(CFA_P70, &tfco->mpc_info);
@@ -408,6 +413,58 @@ int tfo_ts_get_mem_cfg(void *tfo, uint8_t ts_tsid, enum
cfa_dir dir,
return rc;
}
+/** Get and atomically clear the table scope memory configuration for this
+ * direction
+ */
+int tfo_ts_get_and_clear_mem_cfg(void *tfo, uint8_t ts_tsid, enum cfa_dir dir,
+ enum cfa_region_type region, bool
*is_bs_owner,
+ struct tfc_ts_mem_cfg *mem_cfg)
+{
+ struct tfc_ts_mem_cfg empty = { .num_lvl = 0 };
+ struct tfc_object *tfco = (struct tfc_object *)tfo;
+ struct tfc_global_object *tfgo;
+ struct tfc_tsid_db *tsid_db;
+ rte_spinlock_t *lock;
+ bool bs_owner;
+
+ if (tfo == NULL) {
+ PMD_DRV_LOG_LINE(ERR, "Invalid tfo pointer");
+ return -EINVAL;
+ }
+ if (tfco->signature != TFC_OBJ_SIGNATURE) {
+ PMD_DRV_LOG_LINE(ERR, "Invalid tfo object");
+ return -EINVAL;
+ }
+ if (mem_cfg == NULL) {
+ PMD_DRV_LOG_LINE(ERR, "Invalid mem_cfg pointer");
+ return -EINVAL;
+ }
+ if (ts_tsid >= TFC_TBL_SCOPE_MAX) {
+ PMD_DRV_LOG_LINE(ERR, "Invalid tsid %d", ts_tsid);
+ return -EINVAL;
+ }
+
+ tfgo = tfco->tfgo;
+ if (tfgo && tfgo->gtsid == ts_tsid) {
+ tsid_db = &tfgo->gtsid_db;
+ lock = &tfgo->mem_cfg_lock;
+ } else {
+ tsid_db = &tfco->tsid_db[ts_tsid];
+ lock = &tfco->mem_cfg_lock;
+ }
+
+ rte_spinlock_lock(lock);
+ *mem_cfg = tsid_db->ts_mem[region][dir];
+ tsid_db->ts_mem[region][dir] = empty;
+ bs_owner = tsid_db->ts_is_bs_owner;
+ rte_spinlock_unlock(lock);
+
+ if (is_bs_owner)
+ *is_bs_owner = bs_owner;
+
+ return 0;
+}
+
/** Get the Pool Manager instance
*/
int tfo_ts_get_cpm_inst(void *tfo, uint8_t ts_tsid, enum cfa_dir dir,
diff --git a/drivers/net/bnxt/tf_core/v3/tfo.h
b/drivers/net/bnxt/tf_core/v3/tfo.h
index 93a6a5c064..a4c83a4ef4 100644
--- a/drivers/net/bnxt/tf_core/v3/tfo.h
+++ b/drivers/net/bnxt/tf_core/v3/tfo.h
@@ -268,6 +268,36 @@ int tfo_ts_get_mem_cfg(void *tfo, uint8_t ts_tsid, enum
cfa_dir dir,
enum cfa_region_type region, bool *is_bs_owner,
struct tfc_ts_mem_cfg *mem_cfg);
+/**
+ * Get and atomically clear the table scope memory configuration.
+ *
+ * @param[in] tfo
+ * Pointer to TFC object
+ *
+ * @param[in] ts_tsid
+ * The table scope ID
+ *
+ * @param[in] dir
+ * The direction (RX/TX)
+ *
+ * @param[in] region
+ * The memory region type (lookup/action)
+ *
+ * @param[out] is_bs_owner
+ * True if the caller is the owner of the backing store
+ *
+ * @param[out] mem_cfg
+ * Receives the mem_cfg that was cleared from the database.
+ * If a concurrent caller already claimed the slot,
+ * mem_cfg->num_lvl will be 0 and the caller must skip the free.
+ *
+ * @return
+ * 0 for SUCCESS, negative error value for FAILURE (errno.h)
+ */
+int tfo_ts_get_and_clear_mem_cfg(void *tfo, uint8_t ts_tsid, enum cfa_dir dir,
+ enum cfa_region_type region, bool
*is_bs_owner,
+ struct tfc_ts_mem_cfg *mem_cfg);
+
/**
* Set the pool memory configuration for this direction.
*
--
2.31.1
