From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] AMD IOMMU: add missing checks Date: Wed, 28 Aug 2013 09:48:13 +0100 Message-ID: <521DB94D.1040504@citrix.com> References: <521CCB3102000078000EECAC@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7183295439553224463==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VEbQX-0001WG-3o for xen-devel@lists.xenproject.org; Wed, 28 Aug 2013 08:48:33 +0000 In-Reply-To: <521CCB3102000078000EECAC@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Jacob Shin , suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org --===============7183295439553224463== Content-Type: multipart/alternative; boundary="------------050000020003000806040905" --------------050000020003000806040905 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit 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 Reviewed-by: Andrew Cooper > > --- 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 --------------050000020003000806040905 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
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

--------------050000020003000806040905-- --===============7183295439553224463== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7183295439553224463==--