diff mbox series

hw/i386/intel_iommu: Fix out-of-bounds access on guest IRT

Message ID 4b15b728-bdfe-3bbe-3a5c-ca3baeef3c5c@siemens.com (mailing list archive)
State New, archived
Headers show
Series hw/i386/intel_iommu: Fix out-of-bounds access on guest IRT | expand

Commit Message

Jan Kiszka March 10, 2020, 5:42 p.m. UTC 
From: Jan Kiszka <jan.kiszka@siemens.com>

vtd_irte_get failed to check the index against the configured table
size, causing an out-of-bounds access on guest memory and potentially
misinterpreting the result.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

BTW, we still miss error reporting emulation, right? Therefore, I added 
that simple error_report_once thing, like the other paths do.

 hw/i386/intel_iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Paolo Bonzini March 10, 2020, 5:53 p.m. UTC | #1 
On 10/03/20 18:42, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> vtd_irte_get failed to check the index against the configured table
> size, causing an out-of-bounds access on guest memory and potentially
> misinterpreting the result.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> BTW, we still miss error reporting emulation, right? Therefore, I added 
> that simple error_report_once thing, like the other paths do.
> 
>  hw/i386/intel_iommu.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 204b6841ec..df7ad254ac 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3094,6 +3094,12 @@ static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index,
>      uint16_t mask, source_id;
>      uint8_t bus, bus_max, bus_min;
>  
> +    if (index >= iommu->intr_size) {
> +        error_report_once("%s: index too large: ind=0x%x",
> +                          __func__, index);
> +        return -VTD_FR_IR_INDEX_OVER;
> +    }
> +
>      addr = iommu->intr_root + index * sizeof(*entry);
>      if (dma_memory_read(&address_space_memory, addr, entry,
>                          sizeof(*entry))) {
> 

Queued, thanks.

Paolo
Peter Xu March 10, 2020, 6:04 p.m. UTC | #2 
On Tue, Mar 10, 2020 at 06:42:11PM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> vtd_irte_get failed to check the index against the configured table
> size, causing an out-of-bounds access on guest memory and potentially
> misinterpreting the result.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

> ---
> 
> BTW, we still miss error reporting emulation, right? Therefore, I added 
> that simple error_report_once thing, like the other paths do.

Yes...

Thanks,
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 204b6841ec..df7ad254ac 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3094,6 +3094,12 @@  static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index,
     uint16_t mask, source_id;
     uint8_t bus, bus_max, bus_min;
 
+    if (index >= iommu->intr_size) {
+        error_report_once("%s: index too large: ind=0x%x",
+                          __func__, index);
+        return -VTD_FR_IR_INDEX_OVER;
+    }
+
     addr = iommu->intr_root + index * sizeof(*entry);
     if (dma_memory_read(&address_space_memory, addr, entry,
                         sizeof(*entry))) {