Re: [PATCH v28 4/7] firmware: imx: device context dedicated to priv

[Frank Li <Frank.li@oss.nxp.com>]

On Fri, Jul 17, 2026 at 11:13:53AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Data race in `ele_msg_send_rcv()` when initializing `waiting_rsp_clbk_hdl` state, leading to a potential NULL pointer dereference in the RX interrupt handler.
> - [Low] Unused variable `flags` in `ele_msg_send_rcv()`.
> --
>
> --- Patch [4]: [PATCH v28 4/7] firmware: imx: device context dedicated to priv ---
> commit 64e554919e156aa88c2007ef9dd9fafde735e583
> Author: Pankaj Gupta <pankaj.gupta@nxp.com>
>
> firmware: imx: device context dedicated to priv
>
> Add priv_dev_ctx to prepare enabling misc-device context based send-receive
> path, to communicate with FW. No functionality change.
>
> Closes: https://sashiko.dev/#/patchset/20260715-imx-se-if-v27-0-bb7c45952f06@nxp.com?part=4
>
> > diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
> > index ca8a32a82689b..1ca22c53b55c4 100644
> > --- a/drivers/firmware/imx/ele_common.c
> > +++ b/drivers/firmware/imx/ele_common.c
>
> [ ... ]
>
> > @@ -149,34 +151,38 @@ 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)
> > +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;
>
> [Severity: Low]
> Is this flags variable used anywhere in this function?
>
> It appears to be declared but never utilized in the function scope, which
> will trigger a -Wunused-variable compiler warning.

Yes, fix it.

>
> >  	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);
> > +	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;
>
> [Severity: High]
> Could these lockless updates result in a data race with the interrupt
> handler?
>
> They are updated here without acquiring clbk_rx_lock. If a delayed firmware
> response from a previous command arrives concurrently, the interrupt handler
> se_if_rx_callback() will execute.
>
> Due to CPU reordering or compiler optimizations, can the interrupt handler
> observe the new rx_msg pointer before the new dev_ctx pointer becomes
> visible?

Please check this

Frank

>
> > -	err = ele_msg_send(priv, tx_msg, tx_msg_sz);
> > +	err = ele_msg_send(dev_ctx, tx_msg, tx_msg_sz);
>
> [ ... ]
>
> > @@ -302,8 +310,8 @@ void se_if_rx_callback(struct mbox_client *mbox_cl, void *msg)
> >  			dev_info(dev, "ELE responded (late), recovery FW available.");
> >  			return;
> >  		}
> > -		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);
>
> If the interrupt handler sees a non-NULL rx_msg, it assumes the transaction
> is active and attempts to access se_clbk_hdl->dev_ctx->devname here. If
> dev_ctx is still NULL from the previous cleanup, won't this result in a NULL
> pointer dereference?
>
> >
> >  		/*
> >  		 * For rsp_tag traffic, the sender provides the expected response
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717-imx-se-if-v28-0-0a9659c7e69d@nxp.com?part=4