drivers: dma: esp32: dma_esp32_get_status() uses the hardware descriptor address with only a lower-bound check
bugarea: DMAplatform: ESP32
## Describe the bug
`dma_esp32_get_status()` takes the descriptor address the hardware reports and uses it after checking only that it is not below the descriptor array. There is no upper bound and no alignment check, so an address above the array passes the guard and is then used three ways, each of which is a problem on its own.
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/dma/dma_esp32_gdma.c#L720-L722
The RX branch invalidates a cache line at that address before reading through it:
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/dma/dma_esp32_gdma.c#L729-L731
Invalidating a line the CPU has dirtied discards that write, so this one can corrupt memory the driver never intended to touch rather than merely read the wrong number. `desc->dw0.length` then reads outside the array, and `desc - dma_channel->desc_list` subtracts two pointers that need not point into the same object, which is undefined behaviour and in practice yields a `read_position` that has no meaning. That value is fed straight into `total_copied` on the next line.
The TX branch has the same guard and the same subtraction, without the cache operation:
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/dma/dma_esp32_gdma.c#L738-L741
Alignment is not checked either, so an address that does land inside the array but is not on a descriptor boundary still produces a non-integral index.
## Steps to reproduce
This comes from reading the driver rather than from a captured run, so please treat it as the path through the code rather than as a measurement. I have not reproduced it on hardware.
Calling `dma_get_status()` on a channel whose prefetched-descriptor register does not currently hold an address inside that channel's `desc_list` is enough. A channel that has not been configured since reset, or one whose register still holds a value left by a previous user of the same hardware channel, are the two cases that look reachable from the API, since nothing in `dma_esp32_get_status()` requires the channel to have been configured first.
## Impact
Functional Limitation
## Environment
- Zephyr `main` at `697b3239`
- Any Espressif SoC using `drivers/dma/dma_esp32_gdma.c`
## Additional Context
Found while preparing #115507, which fixes a different out-of-bounds access in the same file, in `dma_esp32_config_descriptor()` and `dma_esp32_reload()`. This one is deliberately not in that PR: it is in a different function, it needs a different fix, and folding it in would widen a small correctness change.
A fix probably wants to validate as an integer rather than as a pointer, since forming the out-of-range pointer is itself the undefined part. Something along the lines of taking the reported address as `uintptr_t`, requiring it to fall within `[desc_list, desc_list + ARRAY_SIZE(desc_list))` and to be a multiple of `sizeof(desc_list[0])`, and leaving `read_position` and `write_position` untouched when it does not, rather than comparing or subtracting C pointers that may not share an object.
0 条评论