drivers/firmware/broadcom/tee_bnxt_fw.c | 38 ++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 12 deletions(-)
From: Jaidev Shastri <jaidevshastri@vt.edu>
tee_bnxt_fw_load() and tee_bnxt_copy_coredump() use pvt_data.ctx as the
ready gate: they return -ENODEV while it is NULL and otherwise invoke
the trusted application with pvt_data.session_id and
pvt_data.fw_shm_pool. bnxt_en calls both from its firmware reset and
coredump paths, on the NIC's workqueue, at any time after the NIC has
probed.
tee_bnxt_fw_probe() stores the context first and opens the session and
allocates the shared memory pool afterwards. A caller that arrives
between those stores passes the gate and invokes the TA with a zero
session id and a NULL pool, and tee_shm_get_va(NULL, 0) dereferences the
NULL. Both the gate and the payload are plain accesses on either side,
so the same stale view is reachable after probe has finished.
Keep the context in a local, store the session id and the pool first and
publish the context last with smp_store_release(). Read it with
smp_load_acquire() in both helpers and use the loaded value.
Found with MBCheck, a static herd7-based memory consistency checker.
Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
drivers/firmware/broadcom/tee_bnxt_fw.c | 38 ++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/broadcom/tee_bnxt_fw.c b/drivers/firmware/broadcom/tee_bnxt_fw.c
index a706c84eb..7bf544116 100644
--- a/drivers/firmware/broadcom/tee_bnxt_fw.c
+++ b/drivers/firmware/broadcom/tee_bnxt_fw.c
@@ -101,13 +101,16 @@ int tee_bnxt_fw_load(void)
int ret = 0;
struct tee_ioctl_invoke_arg arg;
struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB];
+ struct tee_context *ctx;
- if (!pvt_data.ctx)
+ /* Pairs with the smp_store_release() in tee_bnxt_fw_probe(). */
+ ctx = smp_load_acquire(&pvt_data.ctx);
+ if (!ctx)
return -ENODEV;
prepare_args(TA_CMD_BNXT_FASTBOOT, &arg, param);
- ret = tee_client_invoke_func(pvt_data.ctx, &arg, param);
+ ret = tee_client_invoke_func(ctx, &arg, param);
if (ret < 0 || arg.ret != 0) {
dev_err(pvt_data.dev,
"TA_CMD_BNXT_FASTBOOT invoke failed TEE err: %x, ret:%x\n",
@@ -136,8 +139,11 @@ int tee_bnxt_copy_coredump(void *buf, u32 offset, u32 size)
u32 rbytes = size;
u32 nbytes = 0;
int ret = 0;
+ struct tee_context *ctx;
- if (!pvt_data.ctx)
+ /* Pairs with the smp_store_release() in tee_bnxt_fw_probe(). */
+ ctx = smp_load_acquire(&pvt_data.ctx);
+ if (!ctx)
return -ENODEV;
prepare_args(TA_CMD_BNXT_COPY_COREDUMP, &arg, param);
@@ -151,7 +157,7 @@ int tee_bnxt_copy_coredump(void *buf, u32 offset, u32 size)
param[1].u.value.a = offset;
param[1].u.value.b = nbytes;
- ret = tee_client_invoke_func(pvt_data.ctx, &arg, param);
+ ret = tee_client_invoke_func(ctx, &arg, param);
if (ret < 0 || arg.ret != 0) {
dev_err(pvt_data.dev,
"TA_CMD_BNXT_COPY_COREDUMP invoke failed TEE err: %x, ret:%x\n",
@@ -187,13 +193,13 @@ static int tee_bnxt_fw_probe(struct tee_client_device *bnxt_device)
int ret, err = -ENODEV;
struct tee_ioctl_open_session_arg sess_arg;
struct tee_shm *fw_shm_pool;
+ struct tee_context *ctx;
memset(&sess_arg, 0, sizeof(sess_arg));
/* Open context with TEE driver */
- pvt_data.ctx = tee_client_open_context(NULL, optee_ctx_match, NULL,
- NULL);
- if (IS_ERR(pvt_data.ctx))
+ ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
+ if (IS_ERR(ctx))
return -ENODEV;
/* Open session with Bnxt load Trusted App */
@@ -201,7 +207,7 @@ static int tee_bnxt_fw_probe(struct tee_client_device *bnxt_device)
sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
sess_arg.num_params = 0;
- ret = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL);
+ ret = tee_client_open_session(ctx, &sess_arg, NULL);
if (ret < 0 || sess_arg.ret != 0) {
dev_err(dev, "tee_client_open_session failed, err: %x\n",
sess_arg.ret);
@@ -212,21 +218,29 @@ static int tee_bnxt_fw_probe(struct tee_client_device *bnxt_device)
pvt_data.dev = dev;
- fw_shm_pool = tee_shm_alloc_kernel_buf(pvt_data.ctx, MAX_SHM_MEM_SZ);
+ fw_shm_pool = tee_shm_alloc_kernel_buf(ctx, MAX_SHM_MEM_SZ);
if (IS_ERR(fw_shm_pool)) {
- dev_err(pvt_data.dev, "tee_shm_alloc_kernel_buf failed\n");
+ dev_err(dev, "tee_shm_alloc_kernel_buf failed\n");
err = PTR_ERR(fw_shm_pool);
goto out_sess;
}
pvt_data.fw_shm_pool = fw_shm_pool;
+ /*
+ * tee_bnxt_fw_load() and tee_bnxt_copy_coredump() test pvt_data.ctx
+ * and then use the session and the shared memory pool. Publish the
+ * context last, with release semantics, so that a caller that sees
+ * the context also sees the session and the pool.
+ */
+ smp_store_release(&pvt_data.ctx, ctx);
+
return 0;
out_sess:
- tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
+ tee_client_close_session(ctx, pvt_data.session_id);
out_ctx:
- tee_client_close_context(pvt_data.ctx);
+ tee_client_close_context(ctx);
return err;
}
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-tee-bnxt-5967e359c540
Best regards,
--
Jaidev Shastri <jaidevshastri@vt.edu>
© 2016 - 2026 Red Hat, Inc.