On 10/7/26 12:59, Peter Maydell wrote:
The soc_dma code has a fastpath for when DMA transfers are from RAM
to RAM. The current implementation of this has the caller of
soc_dma_port_add_mem() pass the underlying host address of the RAM
block that the DMA port is connected to (obtained via
memory_region_get_ram_ptr()). Then the actual transfer function does
a simple memcpy(). This has several problems.
Most importantly, no bounds checking is done on the address and size
passed by the guest, so the memcpy source and destination might be
outside the backing host RAM entirely. Secondly, because the DMA
access is done via this back door, there is no updating of the dirty
region when memory is written this way (there is a TODO comment
in omap_dma.c noting this).
Fix both of these by making the memory to memory transfer function
use physical_memory_map() to get the host addresses for the memory
copy. That function will automatically give us the bounds check that
we want and return a short length if the transfer would run off the
end of the RAM MemoryRegion it starts in. Since the OMAP DMA
documentation states that it's a guest error to misprogram the
addresses so that they fall outside the range that is valid for the
particular DMA port being addressed and that this can result in guest
memory corruption , we don't need to loop for short transfers, but
can simply log them and continue.
Note that we don't need to update addresses or bytecount here in the
transfer function, because when soc_dma_ch_update() selects
transfer_mem2mem it also sets ch->update to 1, which tells the
omap_dma_transfer_setup() code that it is responsible for updating
all the guest visible fields to match "transfer completed".
(We use physical_memory_map() here to match the use of
physical_memory_read() and physical_memory_write() in omap_dma.c;
making the DMA controller use an explicit AddressSpace would be
a separate cleanup task.)
Together with the preceding commits that fixed some integer overflow
problems, this fixes the "guest can provoke a bad memcpy() operation"
reported in issue #3204.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3204
Signed-off-by: Peter Maydell <[email protected]>
---
hw/dma/omap_dma.c | 5 -----
hw/dma/soc_dma.c | 51 ++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 48 insertions(+), 8 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
