Xen devel - Aug 2013 - [PATCH] AMD IOMMU: add missing checks

For one we shouldn''t accept IVHD tables specifying IO-APIC IDs beyond
the limit we support (MAX_IO_APICS, currently 128).

And then we shouldn''t memset() a pointer allocation of which failed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -674,6 +674,13 @@ static u16 __init parse_ivhd_device_spec
             if ( IO_APIC_ID(apic) != special->handle )
                 continue;
 
+            if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
+            {
+                printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry beyond
bounds\n",
+                       special->handle);
+                return 0;
+            }
+
             if ( ioapic_sbdf[special->handle].pin_2_idx )
             {
                 if ( ioapic_sbdf[special->handle].bdf == bdf &&
@@ -943,13 +950,14 @@ static int __init parse_ivrs_table(struc
         {
             ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx = xmalloc_array(
                 u16, nr_ioapic_entries[apic]);
-            if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
+            if ( ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
+                memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
+                       nr_ioapic_entries[apic] *
sizeof(*ioapic_sbdf->pin_2_idx));
+            else
             {
                 printk(XENLOG_ERR "IVHD Error: Out of memory\n");
                 error = -ENOMEM;
             }
-            memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
-                   nr_ioapic_entries[apic] *
sizeof(*ioapic_sbdf->pin_2_idx));
         }
     }
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

>>> On 27.08.13 at 15:52, "Jan Beulich"
<JBeulich@suse.com> wrote:
> For one we shouldn''t accept IVHD tables specifying IO-APIC IDs
beyond
> the limit we support (MAX_IO_APICS, currently 128).
> 
> And then we shouldn''t memset() a pointer allocation of which
failed.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Just to clarify - this was broken out from the earlier sent "AMD
IOMMU: allow command line overrides for broken IVRS tables", as
the changes here are clear backporting candidates.

Jan
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -674,6 +674,13 @@ static u16 __init parse_ivhd_device_spec
>              if ( IO_APIC_ID(apic) != special->handle )
>                  continue;
>  
> +            if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
> +            {
> +                printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry
beyond bounds\n",
> +                       special->handle);
> +                return 0;
> +            }
> +
>              if ( ioapic_sbdf[special->handle].pin_2_idx )
>              {
>                  if ( ioapic_sbdf[special->handle].bdf == bdf &&
> @@ -943,13 +950,14 @@ static int __init parse_ivrs_table(struc
>          {
>              ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx = xmalloc_array(
>                  u16, nr_ioapic_entries[apic]);
> -            if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
> +            if ( ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
> +                memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
> +                       nr_ioapic_entries[apic] *
sizeof(*ioapic_sbdf->pin_2_idx));
> +            else
>              {
>                  printk(XENLOG_ERR "IVHD Error: Out of
memory\n");
>                  error = -ENOMEM;
>              }
> -            memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
> -                   nr_ioapic_entries[apic] *
sizeof(*ioapic_sbdf->pin_2_idx));
>          }
>      }
>

On 27/08/13 14:52, Jan Beulich wrote:
> For one we shouldn''t accept IVHD tables specifying IO-APIC IDs
beyond
> the limit we support (MAX_IO_APICS, currently 128).
>
> And then we shouldn''t memset() a pointer allocation of which
failed.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -674,6 +674,13 @@ static u16 __init parse_ivhd_device_spec
>              if ( IO_APIC_ID(apic) != special->handle )
>                  continue;
>  
> +            if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
> +            {
> +                printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry
beyond bounds\n",
> +                       special->handle);
> +                return 0;
> +            }
> +
>              if ( ioapic_sbdf[special->handle].pin_2_idx )
>              {
>                  if ( ioapic_sbdf[special->handle].bdf == bdf &&
> @@ -943,13 +950,14 @@ static int __init parse_ivrs_table(struc
>          {
>              ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx = xmalloc_array(
>                  u16, nr_ioapic_entries[apic]);
> -            if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
> +            if ( ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
> +                memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
> +                       nr_ioapic_entries[apic] *
sizeof(*ioapic_sbdf->pin_2_idx));
> +            else
>              {
>                  printk(XENLOG_ERR "IVHD Error: Out of
memory\n");
>                  error = -ENOMEM;
>              }
> -            memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
> -                   nr_ioapic_entries[apic] *
sizeof(*ioapic_sbdf->pin_2_idx));
>          }
>      }
>  
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel