RE: [PATCH v33 4/7] firmware: imx: device context dedicated to priv

"Pankaj Gupta (OSS)" <[email protected]>
Newsgroups dev.linux.lists.imx,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID <GV2PR04MB122712931FDB5134BED3DB4FE95D32@GV2PR04MB12271.eurprd04.prod.outlook.com>
> > b/drivers/firmware/imx/ele_base_msg.c
> > index 724f6e913ce79..b70e3ef88a168 100644
> > --- a/drivers/firmware/imx/ele_base_msg.c
> > +++ b/drivers/firmware/imx/ele_base_msg.c
> > @@ -71,8 +71,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,
> get_info_len);
> >  		return ret;
> 
> [Severity: Critical]
> Does this code risk a DMA use-after-free if the firmware command times out?
> 
> If ele_msg_send_rcv() times out and returns -ETIMEDOUT, the error path calls
> ele_get_info_cleanup(), which frees the DMA buffer back to the system.
> 
> Since the firmware command was not aborted, if the firmware is delayed and
> responds later, it may complete the operation and perform a DMA write of
> the device info directly into the now-freed memory. This could cause silent
> memory corruption of whichever subsystem reallocated the physical page.
> 
Duplicate comment as that of 3/7.
Already taken care in 5/7. 

At 3/7 the cleanup frees unconditionally (gen_pool_free/dma_free_coherent) and the timeout leg only does a bare atomic_set(\&priv->fw_busy, 1), which cannot fence an in-flight enclave DMA. The symbols that fix it (se_is_fw_busy_ctx/se_mark_fw_busy, priv_dev_ctx) do not exist at 3/7. It is resolved by PATCH v33 5/7 ("firmware: imx: adds miscdev"), which adds per-context fw_busy ownership and rewrites ele_get_info_cleanup() to quarantine (return without freeing) the buffer while the owning context is still busy.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.