[PATCH v30 4/7] firmware: imx: device context dedicated to priv
From: pankaj . gupta
Date: Fri Jul 24 2026 - 04:17:24 EST
From: Pankaj Gupta <pankaj.gupta@xxxxxxx>
Add priv_dev_ctx to prepare enabling misc-device context based send-receive
path, to communicate with FW.
No functionality change.
Signed-off-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
---
Changes from v28 to v29:
Address both findings from the Sashiko AI review of patch 4/7
Addressed (drivers/firmware/imx/ele_common.c):
- [High] Data race / NULL pointer dereference when initializing
waiting_rsp_clbk_hdl in ele_msg_send_rcv():
dev_ctx, rx_msg_sz and rx_msg were published without holding
clbk_rx_lock. A delayed firmware response from a previous command could
run se_if_rx_callback() concurrently and, due to CPU/compiler
reordering, observe the new rx_msg before the new dev_ctx was visible,
then dereference se_clbk_hdl->dev_ctx->devname while dev_ctx was still
NULL from the previous cleanup.
Publish all three fields under clbk_rx_lock so the write of dev_ctx
(always non-NULL) is ordered before rx_msg. ele_msg_send_rcv_cleanup()
already clears dev_ctx, rx_msg and rx_msg_sz together under the same
lock, and se_if_rx_callback() reads them under the lock and only
dereferences dev_ctx after confirming rx_msg != NULL. The lock provides
the memory ordering, so the invariant "rx_msg != NULL implies dev_ctx
!= NULL" holds and the dereference is safe. The RX callback also reads
dev_ctx->devname under the lock (cmd_tag path additionally checks
dev_ctx explicitly) and caches it in a local before dropping the lock.
- [Low] Unused variable flags in ele_msg_send_rcv():
flags is now consumed by the spin_lock_irqsave()/spin_unlock_irqrestore()
pair added for the fix above, so it is no longer unused and the
-Wunused-variable warning is gone.
Follow-up Sashiko AI review of patch 4/7 (4 findings). All already
resolved in this patch; no functional change required beyond a cosmetic
whitespace cleanup:
- [High][NEW] Accessing se_clbk_hdl->dev_ctx->devname outside
clbk_rx_lock in the cmd_tag path (UAF / NULL deref):
Already fixed. In se_if_rx_callback(), the cmd_tag path holds
clbk_rx_lock across the dev_dbg() that reads dev_ctx->devname and only
proceeds after the explicit "!se_clbk_hdl->dev_ctx" check. The devname
used by the post-unlock dev_err() is cached into a local (devname)
while the lock is still held, so no dev_ctx dereference happens outside
the lock.
- [High] Incorrect devm registration order frees priv while the RX
mailbox channel is still active (NULL deref):
Already fixed in drivers/firmware/imx/se_ctrl.c. se_if_probe()
registers devm_add_action_or_reset(dev, se_if_probe_cleanup, pdev)
before requesting the tx/rx mailbox channels, so LIFO teardown frees
the channels (se_if_probe_cleanup) before priv is released.
- [High] Memory leak of priv when early probe steps fail:
Already fixed by the same change. priv is allocated, stored via
dev_set_drvdata(), and its release is tied to se_if_probe_cleanup(),
which is registered early; any subsequent probe failure unwinds through
the devres action that frees priv.
- [High] Unprotected read of rx_msg_sz creates a TOCTOU race leading to
response truncation on late interrupts:
Already fixed. The rsp_tag path in se_if_rx_callback() reads
exp_rx_msg_sz = se_clbk_hdl->rx_msg_sz only after acquiring
clbk_rx_lock (and after confirming rx_msg != NULL), so the size cannot
change between the check and the memcpy().
Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
Closes: https://sashiko.dev/#/patchset/20260721-imx-se-if-v29-0-04a362f4fcca@xxxxxxx?part=4
---
drivers/firmware/imx/ele_base_msg.c | 15 +++++-----
drivers/firmware/imx/ele_common.c | 58 ++++++++++++++++++++++---------------
drivers/firmware/imx/ele_common.h | 8 ++---
drivers/firmware/imx/se_ctrl.c | 36 +++++++++++++++++++++++
drivers/firmware/imx/se_ctrl.h | 9 ++++++
5 files changed, 92 insertions(+), 34 deletions(-)
diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
index a2ede9c01691..524ebc0f697e 100644
--- a/drivers/firmware/imx/ele_base_msg.c
+++ b/drivers/firmware/imx/ele_base_msg.c
@@ -66,8 +66,9 @@ int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
tx_msg->data[0] = upper_32_bits(get_info_addr);
tx_msg->data[1] = lower_32_bits(get_info_addr);
tx_msg->data[2] = sizeof(*s_info);
- ret = ele_msg_send_rcv(priv, tx_msg, ELE_GET_INFO_REQ_MSG_SZ, rx_msg,
- ELE_GET_INFO_RSP_MSG_SZ);
+
+ ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_GET_INFO_REQ_MSG_SZ,
+ rx_msg, ELE_GET_INFO_RSP_MSG_SZ);
if (ret < 0) {
ele_get_info_cleanup(priv, get_info_data, get_info_addr, ELE_GET_INFO_BUFF_SZ);
return ret;
@@ -112,8 +113,8 @@ int ele_ping(struct se_if_priv *priv)
se_fill_cmd_msg_hdr(priv, (struct se_msg_hdr *)&tx_msg->header,
ELE_PING_REQ, ELE_PING_REQ_SZ, true);
- ret = ele_msg_send_rcv(priv, tx_msg, ELE_PING_REQ_SZ, rx_msg,
- ELE_PING_RSP_SZ);
+ ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_PING_REQ_SZ,
+ rx_msg, ELE_PING_RSP_SZ);
if (ret < 0)
return ret;
@@ -160,7 +161,7 @@ int ele_service_swap(struct se_if_priv *priv,
if (ret)
return -EINVAL;
- ret = ele_msg_send_rcv(priv, tx_msg, ELE_SERVICE_SWAP_REQ_MSG_SZ,
+ ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_SERVICE_SWAP_REQ_MSG_SZ,
rx_msg, ELE_SERVICE_SWAP_RSP_MSG_SZ);
if (ret < 0)
return ret;
@@ -208,7 +209,7 @@ int ele_fw_authenticate(struct se_if_priv *priv, dma_addr_t contnr_addr,
tx_msg->data[1] = 0;
tx_msg->data[2] = lower_32_bits(img_addr);
- ret = ele_msg_send_rcv(priv, tx_msg, ELE_FW_AUTH_REQ_SZ, rx_msg,
+ ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_FW_AUTH_REQ_SZ, rx_msg,
ELE_FW_AUTH_RSP_MSG_SZ);
if (ret < 0)
return ret;
@@ -246,7 +247,7 @@ int ele_debug_dump(struct se_if_priv *priv)
do {
memset(rx_msg, 0x0, ELE_DEBUG_DUMP_RSP_SZ);
- ret = ele_msg_send_rcv(priv, tx_msg, ELE_DEBUG_DUMP_REQ_SZ,
+ ret = ele_msg_send_rcv(priv->priv_dev_ctx, tx_msg, ELE_DEBUG_DUMP_REQ_SZ,
rx_msg, ELE_DEBUG_DUMP_RSP_SZ);
if (ret < 0)
return ret;
diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
index f9be8b8556ad..3811c37d1754 100644
--- a/drivers/firmware/imx/ele_common.c
+++ b/drivers/firmware/imx/ele_common.c
@@ -46,8 +46,9 @@ int se_update_msg_chksum(u32 *msg, u32 msg_len)
return 0;
}
-int ele_msg_rcv(struct se_if_priv *priv, struct se_clbk_handle *se_clbk_hdl)
+int ele_msg_rcv(struct se_if_device_ctx *dev_ctx, struct se_clbk_handle *se_clbk_hdl)
{
+ struct se_if_priv *priv = dev_ctx->priv;
bool is_rsp_wait_with_timeout = false;
bool wait_uninterruptible = false;
unsigned long remaining_jiffies;
@@ -126,7 +127,7 @@ int ele_msg_rcv(struct se_if_priv *priv, struct se_clbk_handle *se_clbk_hdl)
return ret;
}
-int ele_msg_send(struct se_if_priv *priv,
+int ele_msg_send(struct se_if_device_ctx *dev_ctx,
void *tx_msg,
int tx_msg_sz)
{
@@ -138,9 +139,9 @@ int ele_msg_send(struct se_if_priv *priv,
* carried in the message.
*/
if (header->size << 2 != tx_msg_sz) {
- dev_err(priv->dev,
- "User buf hdr: 0x%x, sz mismatced with input-sz (%d != %d).",
- *(u32 *)header, header->size << 2, tx_msg_sz);
+ dev_err(dev_ctx->priv->dev,
+ "%s: User buf hdr: 0x%x, sz mismatched with input-sz (%d != %d).",
+ dev_ctx->devname, *(u32 *)header, header->size << 2, tx_msg_sz);
return -EINVAL;
}
@@ -150,9 +151,10 @@ int ele_msg_send(struct se_if_priv *priv,
* caller-provided tx_msg pointer after mbox_send_message() returns, so
* the caller-owned buffer may be released after a successful send.
*/
- err = mbox_send_message(priv->tx_chan, tx_msg);
+ err = mbox_send_message(dev_ctx->priv->tx_chan, tx_msg);
if (err < 0) {
- dev_err(priv->dev, "Error: mbox_send_message failure.\n");
+ dev_err(dev_ctx->priv->dev,
+ "%s: Error: mbox_send_message failure.", dev_ctx->devname);
return err;
}
@@ -164,34 +166,40 @@ static void ele_msg_send_rcv_cleanup(struct se_if_priv *priv)
unsigned long flags;
spin_lock_irqsave(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
+ priv->waiting_rsp_clbk_hdl.dev_ctx = NULL;
priv->waiting_rsp_clbk_hdl.rx_msg = NULL;
priv->waiting_rsp_clbk_hdl.rx_msg_sz = 0;
spin_unlock_irqrestore(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
}
-/* API used for send/receive blocking call. */
-int ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, int tx_msg_sz,
- void *rx_msg, int exp_rx_msg_sz)
+/* API used for send/receive blocking calls. */
+int ele_msg_send_rcv(struct se_if_device_ctx *dev_ctx, void *tx_msg,
+ int tx_msg_sz, void *rx_msg, int exp_rx_msg_sz)
{
+ struct se_if_priv *priv = dev_ctx->priv;
+ unsigned long flags;
int err;
guard(mutex)(&priv->se_if_cmd_lock);
if (atomic_read(&priv->fw_busy)) {
- dev_dbg(priv->dev, "ELE became unresponsive.\n");
+ dev_dbg(priv->dev, "%s: ELE became unresponsive.\n", dev_ctx->devname);
return -EBUSY;
}
reinit_completion(&priv->waiting_rsp_clbk_hdl.done);
+ spin_lock_irqsave(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
+ priv->waiting_rsp_clbk_hdl.dev_ctx = dev_ctx;
priv->waiting_rsp_clbk_hdl.rx_msg_sz = exp_rx_msg_sz;
priv->waiting_rsp_clbk_hdl.rx_msg = rx_msg;
+ spin_unlock_irqrestore(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
- err = ele_msg_send(priv, tx_msg, tx_msg_sz);
+ err = ele_msg_send(dev_ctx, tx_msg, tx_msg_sz);
if (err < 0) {
ele_msg_send_rcv_cleanup(priv);
return err;
}
- err = ele_msg_rcv(priv, &priv->waiting_rsp_clbk_hdl);
+ err = ele_msg_rcv(dev_ctx, &priv->waiting_rsp_clbk_hdl);
if (priv->waiting_rsp_clbk_hdl.signal_rcvd) {
/*
@@ -202,7 +210,8 @@ int ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, int tx_msg_sz,
if (err > 0)
err = -ERESTARTSYS;
priv->waiting_rsp_clbk_hdl.signal_rcvd = false;
- dev_dbg(priv->dev, "Err[0x%x]:Interrupted by signal.", err);
+ dev_dbg(priv->dev, "%s: Err[0x%x]:Interrupted by signal.",
+ dev_ctx->devname, err);
}
ele_msg_send_rcv_cleanup(priv);
@@ -231,6 +240,7 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
{
struct se_clbk_handle *se_clbk_hdl;
struct device *dev = mbox_cl->dev;
+ const char *devname = NULL;
struct se_msg_hdr *header;
bool sz_mismatch = false;
struct se_if_priv *priv;
@@ -254,7 +264,7 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
if (header->tag == priv->if_defs->cmd_tag) {
se_clbk_hdl = &priv->cmd_receiver_clbk_hdl;
spin_lock_irqsave(&se_clbk_hdl->clbk_rx_lock, flags);
- if (!se_clbk_hdl->rx_msg) {
+ if (!se_clbk_hdl->dev_ctx || !se_clbk_hdl->rx_msg) {
spin_unlock_irqrestore(&se_clbk_hdl->clbk_rx_lock, flags);
dev_warn(dev, "No command receiver registered for message: %.8x\n",
*((u32 *)header));
@@ -268,8 +278,8 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
* SE_IOCTL_ENABLE_CMD_RCV and is not subject to the timeout/circuit-
* breaker handling used for rsp_tag messages.
*/
- dev_dbg(dev, "Selecting cmd receiver: for mesg header:0x%x.",
- *(u32 *)header);
+ dev_dbg(dev, "Selecting cmd receiver:%s for mesg header:0x%x.",
+ se_clbk_hdl->dev_ctx->devname, *(u32 *)header);
/*
* Pre-allocated buffer of MAX_NVM_MSG_LEN
@@ -284,13 +294,14 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
* Clamp the copy length to the pre-allocated receiver buffer (MAX_NVM_MSG_LEN).
*/
se_clbk_hdl->rx_msg_sz = min_t(u32, rx_msg_sz, MAX_NVM_MSG_LEN);
+ devname = se_clbk_hdl->dev_ctx->devname;
memcpy(se_clbk_hdl->rx_msg, msg, se_clbk_hdl->rx_msg_sz);
complete(&se_clbk_hdl->done);
spin_unlock_irqrestore(&se_clbk_hdl->clbk_rx_lock, flags);
if (sz_mismatch)
dev_err(dev,
- "CMD-RCVER NVM: hdr(0x%x) with different sz(%d != %d).\n",
- *(u32 *)header,
+ "%s: CMD-RCVER NVM: hdr(0x%x) with different sz(%d != %d).\n",
+ devname, *(u32 *)header,
(header->size << 2), rx_msg_sz);
} else if (header->tag == priv->if_defs->rsp_tag) {
bool exception_for_sz_mismatch = check_hdr_exception_for_sz(priv, header);
@@ -312,8 +323,8 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
return;
}
exp_rx_msg_sz = se_clbk_hdl->rx_msg_sz;
- dev_dbg(dev, "Selecting resp waiter: for mesg header:0x%x.",
- *(u32 *)header);
+ dev_dbg(dev, "Selecting resp waiter:%s for mesg header:0x%x.",
+ se_clbk_hdl->dev_ctx->devname, *(u32 *)header);
/*
* For rsp_tag traffic, the sender provides the expected response
@@ -325,14 +336,15 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
sz_mismatch = true;
se_clbk_hdl->rx_msg_sz = min(rx_msg_sz, exp_rx_msg_sz);
+ devname = se_clbk_hdl->dev_ctx->devname;
memcpy(se_clbk_hdl->rx_msg, msg, se_clbk_hdl->rx_msg_sz);
complete(&se_clbk_hdl->done);
spin_unlock_irqrestore(&se_clbk_hdl->clbk_rx_lock, flags);
if (sz_mismatch)
dev_err(dev,
- "Rsp to CMD: hdr(0x%x) with different sz(%d != %d).\n",
- *(u32 *)header,
+ "%s: Rsp to CMD: hdr(0x%x) with different sz(%d != %d).\n",
+ devname, *(u32 *)header,
(header->size << 2), exp_rx_msg_sz);
} else {
dev_err(dev, "Failed to select a device for message: %.8x\n",
diff --git a/drivers/firmware/imx/ele_common.h b/drivers/firmware/imx/ele_common.h
index 7bf2febefc45..07e6b6a1bafa 100644
--- a/drivers/firmware/imx/ele_common.h
+++ b/drivers/firmware/imx/ele_common.h
@@ -16,12 +16,12 @@
int se_update_msg_chksum(u32 *msg, u32 msg_len);
-int ele_msg_rcv(struct se_if_priv *priv, struct se_clbk_handle *se_clbk_hdl);
+int ele_msg_rcv(struct se_if_device_ctx *dev_ctx, struct se_clbk_handle *se_clbk_hdl);
-int ele_msg_send(struct se_if_priv *priv, void *tx_msg, int tx_msg_sz);
+int ele_msg_send(struct se_if_device_ctx *dev_ctx, void *tx_msg, int tx_msg_sz);
-int ele_msg_send_rcv(struct se_if_priv *priv, void *tx_msg, int tx_msg_sz,
- void *rx_msg, int exp_rx_msg_sz);
+int ele_msg_send_rcv(struct se_if_device_ctx *dev_ctx, void *tx_msg,
+ int tx_msg_sz, void *rx_msg, int exp_rx_msg_sz);
void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg);
diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
index e9e2f48f4f71..b053dd00286c 100644
--- a/drivers/firmware/imx/se_ctrl.c
+++ b/drivers/firmware/imx/se_ctrl.c
@@ -284,6 +284,36 @@ static int get_se_soc_info(struct se_if_priv *priv, const struct se_soc_info *se
return 0;
}
+static int init_misc_device_context(struct se_if_priv *priv, int ch_id,
+ struct se_if_device_ctx **new_dev_ctx)
+{
+ const char *err_str = "Failed to allocate memory";
+ struct se_if_device_ctx *dev_ctx;
+ int ret = -ENOMEM;
+
+ dev_ctx = kzalloc_obj(*dev_ctx, GFP_KERNEL);
+
+ if (!dev_ctx)
+ return ret;
+
+ dev_ctx->devname = kasprintf(GFP_KERNEL, "%s0_ch%d",
+ get_se_if_name(priv->if_defs->se_if_type),
+ ch_id);
+ if (!dev_ctx->devname)
+ goto exit;
+
+ dev_ctx->priv = priv;
+ *new_dev_ctx = dev_ctx;
+
+ return 0;
+exit:
+ *new_dev_ctx = NULL;
+
+ kfree(dev_ctx->devname);
+ kfree(dev_ctx);
+ return dev_err_probe(priv->dev, ret, "%s", err_str);
+}
+
static int se_if_request_channel(struct device *dev, struct mbox_chan **chan,
struct mbox_client *cl, const char *name)
{
@@ -411,6 +441,12 @@ static int se_if_probe(struct platform_device *pdev)
load_fw->is_fw_tobe_loaded = true;
}
+ ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed[0x%x] to create device contexts.",
+ ret);
+
/* By default, there is no pending FW to be loaded.*/
if (se_info->imem_state_mgmt) {
load_fw = get_load_fw_instance(priv);
diff --git a/drivers/firmware/imx/se_ctrl.h b/drivers/firmware/imx/se_ctrl.h
index 00ac892997a6..24ef97d01be1 100644
--- a/drivers/firmware/imx/se_ctrl.h
+++ b/drivers/firmware/imx/se_ctrl.h
@@ -20,6 +20,7 @@
#define MESSAGING_VERSION_7 0x7
struct se_clbk_handle {
+ struct se_if_device_ctx *dev_ctx;
struct completion done;
bool signal_rcvd;
u32 rx_msg_sz;
@@ -44,6 +45,12 @@ struct se_imem_buf {
u32 state;
};
+/* Private struct for each char device instance. */
+struct se_if_device_ctx {
+ struct se_if_priv *priv;
+ const char *devname;
+};
+
/* Header of the messages exchange with the EdgeLock Enclave */
struct se_msg_hdr {
u8 ver;
@@ -106,6 +113,8 @@ struct se_if_priv {
struct se_fw_load_info load_fw;
atomic_t fw_busy;
+
+ struct se_if_device_ctx *priv_dev_ctx;
};
char *get_se_if_name(u8 se_if_id);
--
2.43.0