Re: [PATCH v7 19/19] dmaengine: ti: k3-udma: Validate resource ID and fix logging in reservation

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

Pre-existing issues:
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race condition during resource allocation in `__udma_reserve_##res`.
--

--- Patch [19]: [PATCH v7 19/19] dmaengine: ti: k3-udma: Validate resource ID and fix logging in reservation ---
commit 86a1fdf2aa8a699bc22b0d6deb37073e3a9709cd
Author: Sai Sree Kartheek Adivi <[email protected]>

dmaengine: ti: k3-udma: Validate resource ID and fix logging in reservation

This commit adds bounds checking for resource IDs in the reservation macro,
returns -EBUSY instead of -ENOENT for in-use resources, and fixes the
resource name logging.

> diff --git a/drivers/dma/ti/k3-udma-common.c b/drivers/dma/ti/k3-udma-common.c
> index d7c148ad694f9..dec4c2ed24e04 100644
> --- a/drivers/dma/ti/k3-udma-common.c
> +++ b/drivers/dma/ti/k3-udma-common.c
> @@ -2011,9 +2011,14 @@ struct udma_##res *__udma_reserve_##res(struct udma_dev *ud,	\
>  					       int id)			\
>  {									\
>  	if (id >= 0) {							\
> +		if (id >= ud->res##_cnt) {				\
> +			dev_err(ud->dev,				\
> +				#res " id %d is out of bounds.\n", id);	\
> +			return ERR_PTR(-EINVAL);			\
> +		}							\
>  		if (test_bit(id, ud->res##_map)) {			\
> -			dev_err(ud->dev, "res##%d is in use\n", id);	\
> -			return ERR_PTR(-ENOENT);			\
> +			dev_err(ud->dev, #res "%d is in use\n", id);	\
> +			return ERR_PTR(-EBUSY);			\
>  		}							\

[Severity: High]
This is a pre-existing issue, but does this reservation logic contain a
Time-Of-Check to Time-Of-Use (TOCTOU) race condition?

If two networking drivers execute their probe or initialization functions
asynchronously, they could both call into __udma_reserve_##res()
simultaneously. 

Whether using an explicit ID and calling test_bit(), or passing -1 and
calling find_next_zero_bit() later in this macro, both threads could
observe the same channel bit as 0. Both would then independently execute:

    set_bit(id, ud->res##_map);
    return &ud->res##s[id];

This would assign the exact same hardware channel pointer to independent
subsystems, which could lead to undefined behavior when both drivers attempt
to configure or transmit on the same channel. 

Are these bit operations protected by a lock higher up in the call chain?

>  	} else {							\
>  		int start;						\

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