On Wed, 5 Nov 2025 at 14:39, Torin Carey <[email protected]> wrote:
>
> The EDU device doesn't enforce any bound checks on the addresses provided,
> allowing users of the device to perform arbitrary reads and writes to QEMU's
> address space.
>
> Signed-off-by: Torin Carey <[email protected]>
> ---
>  hw/misc/edu.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)

This patch got lost because of a tangent conversation, but the
bug was recently re-reported as
https://gitlab.com/qemu-project/qemu/-/work_items/3852
and I think the patch itself is good and we should apply it.

Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3852
Fixes: 7b608e5d6c1 ("hw: misc: edu: use qemu_log_mask instead of hw_error")

Reviewed-by: Peter Maydell <[email protected]>

thanks
-- PMM

> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index cece633e11..a4b01269e8 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -103,7 +103,7 @@ static void edu_lower_irq(EduState *edu, uint32_t val)
>      }
>  }
>
> -static void edu_check_range(uint64_t xfer_start, uint64_t xfer_size,
> +static bool edu_check_range(uint64_t xfer_start, uint64_t xfer_size,
>                  uint64_t dma_start, uint64_t dma_size)
>  {
>      uint64_t xfer_end = xfer_start + xfer_size;
> @@ -115,13 +115,15 @@ static void edu_check_range(uint64_t xfer_start, 
> uint64_t xfer_size,
>       */
>      if (dma_end >= dma_start && xfer_end >= xfer_start &&
>          xfer_start >= dma_start && xfer_end <= dma_end) {
> -        return;
> +        return true;
>      }
>
>      qemu_log_mask(LOG_GUEST_ERROR,
>                    "EDU: DMA range 0x%016"PRIx64"-0x%016"PRIx64
>                    " out of bounds (0x%016"PRIx64"-0x%016"PRIx64")!",
>                    xfer_start, xfer_end - 1, dma_start, dma_end - 1);
> +
> +    return false;
>  }
>
>  static dma_addr_t edu_clamp_addr(const EduState *edu, dma_addr_t addr)
> @@ -148,16 +150,18 @@ static void edu_dma_timer(void *opaque)
>
>      if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_FROM_PCI) {
>          uint64_t dst = edu->dma.dst;
> -        edu_check_range(dst, edu->dma.cnt, DMA_START, DMA_SIZE);
> -        dst -= DMA_START;
> -        pci_dma_read(&edu->pdev, edu_clamp_addr(edu, edu->dma.src),
> -                edu->dma_buf + dst, edu->dma.cnt);
> +        if (edu_check_range(dst, edu->dma.cnt, DMA_START, DMA_SIZE)) {
> +            dst -= DMA_START;
> +            pci_dma_read(&edu->pdev, edu_clamp_addr(edu, edu->dma.src),
> +                         edu->dma_buf + dst, edu->dma.cnt);
> +        }
>      } else {
>          uint64_t src = edu->dma.src;
> -        edu_check_range(src, edu->dma.cnt, DMA_START, DMA_SIZE);
> -        src -= DMA_START;
> -        pci_dma_write(&edu->pdev, edu_clamp_addr(edu, edu->dma.dst),
> -                edu->dma_buf + src, edu->dma.cnt);
> +        if (edu_check_range(src, edu->dma.cnt, DMA_START, DMA_SIZE)) {
> +            src -= DMA_START;
> +            pci_dma_write(&edu->pdev, edu_clamp_addr(edu, edu->dma.dst),
> +                          edu->dma_buf + src, edu->dma.cnt);
> +        }
>      }
>
>      edu->dma.cmd &= ~EDU_DMA_RUN;

Reply via email to