Re: [PATCH v2 10/13] dmaengine: ste_dma40: Fix logical channel bounds check

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Changing DMA transfer direction after channel allocation corrupts allocation masks and leaks logical channels.
--

--- Patch [10]: [PATCH v2 10/13] dmaengine: ste_dma40: Fix logical channel bounds check ---
commit 77f491852f9c4972a650574c4a045eadf10238f0
Author: Linus Walleij <[email protected]>

dmaengine: ste_dma40: Fix logical channel bounds check

d40_validate_conf() checks the raw dev_type against num_log_chans, but
d40_allocate_channel() derives the lookup_log_chans index differently. This
commit validates the dev_type against the derived logical channel index limit
so channel allocation cannot write past lookup_log_chans.

Closes: https://lore.kernel.org/dmaengine/[email protected]/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 4e840846466b8..7c777e410c104 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1737,15 +1737,26 @@ static int d40_validate_conf(struct d40_chan *d40c,
>  {
>  	int res = 0;
>  	bool is_log = conf->mode == STEDMA40_MODE_LOGICAL;
> +	bool invalid_dev_type = conf->dev_type < 0;
>  
>  	if (!conf->dir) {
>  		chan_err(d40c, "Invalid direction.\n");
>  		res = -EINVAL;
>  	}
>  
> -	if ((is_log && conf->dev_type > d40c->base->num_log_chans)  ||
> -	    (!is_log && conf->dev_type > d40c->base->num_phy_chans) ||
> -	    (conf->dev_type < 0)) {
> +	if (!invalid_dev_type && is_log) {
> +		int max_dev_type;
> +
> +		if (conf->dir == DMA_DEV_TO_MEM)
> +			max_dev_type = DIV_ROUND_UP(d40c->base->num_log_chans, 2);

[Severity: High]
This is a pre-existing issue, but does changing the DMA transfer direction
after channel allocation corrupt allocation masks and leak logical channels?

If a DMA client allocates a channel configured for one direction (e.g.,
DMA_MEM_TO_DEV), and then prepares a transfer with the opposite direction,
drivers/dma/ste_dma40.c:d40_set_runtime_config_write() updates the internal
direction:

    cfg->dir = DMA_DEV_TO_MEM;

Later, during channel release, drivers/dma/ste_dma40.c:d40_free_dma() relies
on this mutated direction to determine which allocation mask to free:

    else if (d40c->dma_cfg.dir == DMA_DEV_TO_MEM)
        is_src = true;
    ...
    d40_alloc_mask_free(phy, is_src, chan_is_logical(d40c) ? event : 0);

Could this clear the bit in the wrong resource mask, leaking the originally
allocated resource and inadvertently freeing a mask that may be actively used
by another client?

> +		else
> +			max_dev_type = d40c->base->num_log_chans / 2;
> +
> +		invalid_dev_type = conf->dev_type >= max_dev_type;
> +	}
> +
> +	if (invalid_dev_type ||
> +	    (!is_log && conf->dev_type > d40c->base->num_phy_chans)) {
>  		chan_err(d40c, "Invalid device type (%d)\n", conf->dev_type);
>  		res = -EINVAL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10