Re: [PATCH v12 13/20] drivers: soc: ti: k3-ringacc: handle absence of tisci

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma: k3-ringacc: DMA use-after-free if hardware size clear times out during teardown
--

commit 48061d2f6a019df3afcaa81ed2b4f501e491cdef
Author: Sai Sree Kartheek Adivi <s-adivi@ti.com>

drivers: soc: ti: k3-ringacc: handle absence of tisci

This patch handles the absence of TISCI by falling back to direct register
writes. This adds support for platforms that do not have TISCI firmware 
like AM62L.

> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> index 5966db4327b13..9195d91fae732 100644
> --- a/drivers/soc/ti/k3-ringacc.c
> +++ b/drivers/soc/ti/k3-ringacc.c

[ ... ]

> @@ -576,10 +665,40 @@ static void k3_ringacc_ring_free_sci(struct k3_ring *ring)
>  	ring_cfg.index = ring->ring_id;
>  	ring_cfg.valid_params = TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER;
>  
> -	ret = ringacc->tisci_ring_ops->set_cfg(ringacc->tisci, &ring_cfg);
> -	if (ret)
> -		dev_err(ringacc->dev, "TISCI ring free fail (%d) ring_idx %d\n",
> -			ret, ring->ring_id);
> +	if (!ringacc->tisci) {
> +		u32 reg;
> +
> +		writel(K3_RINGACC_RT_INT_ENABLE_SET_COMPLETE | K3_RINGACC_RT_INT_ENABLE_SET_TR,
> +		       &ring->intr->clr);
> +
> +		reg = readl(&ring->cfg->size);
> +		reg &= ~K3_DMARING_CFG_SIZE_MASK;
> +		writel(reg, &ring->cfg->size);
> +
> +		ret = readl_poll_timeout(&ring->cfg->size, reg,
> +					  !(reg & K3_DMARING_CFG_SIZE_MASK),
> +					  1, 100);
> +		if (ret) {
> +			dev_err(ringacc->dev, "ring %d size clear timeout\n",
> +				ring->ring_id);
> +			return;

[Severity: High]
If readl_poll_timeout() times out here, the early return prevents clearing
the ring base address registers (ba_lo and ba_hi).

Since the caller k3_ringacc_ring_free() subsequently frees the ring memory
unconditionally via dma_free_coherent(), could this lead to a DMA 
use-after-free if the hardware continues to access those still-programmed 
addresses?

Does the teardown path need to ensure the DMA addresses are cleared even
if the size clearing operation times out?

> +		}
> +
> +		reg = readl(&ring->cfg->size);
> +		reg |= ring_cfg.count & K3_DMARING_CFG_SIZE_MASK;
> +
> +		writel(reg, &ring->cfg->size);
> +
> +		writel(ring_cfg.addr_lo, &ring->cfg->ba_lo);
> +		writel((ring_cfg.addr_hi & K3_DMARING_CFG_ADDR_HI_MASK) +
> +				(ring_cfg.asel << K3_DMARING_CFG_ASEL_SHIFT),
> +				&ring->cfg->ba_hi);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007102936.2402427-1-s-adivi@ti.com?part=13