From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@novell.com>
Subject: Re: Xen 4.0.0-rc7 problem/hang with vt-d DMAR
	 parsing
Date: Thu, 25 Mar 2010 08:43:38 +0000
Message-ID: <4BAB304A0200007800036D8B@vpn.id2.novell.com>
References: <20100323193748.GW1878@reaktio.net>
	<C7CECCF9.E36E%keir.fraser@eu.citrix.com>
	<20100323200515.GZ1878@reaktio.net>
	<ED3036A092A28F4C91B0B4360DD128EABE03D6BC@shzsmsx502.ccr.corp.intel.com>
	<4BA9DA400200007800036ABB@vpn.id2.novell.com>
	<4BA9D512.9090902@intel.com>
	<4BA9ED8D0200007800036B3F@vpn.id2.novell.com>
	<4BA9F0C1.7080809@intel.com>
	<4BAA01560200007800036B9B@vpn.id2.novell.com>
	<4BAAB488.3020007@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <4BAAB488.3020007@intel.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Weidong Han <weidong.han@intel.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Keir Fraser <keir.fraser@eu.citrix.com>, Dexuan Cui <dexuan.cui@intel.com>
List-Id: xen-devel@lists.xenproject.org

>>> Weidong Han <weidong.han@intel.com> 25.03.10 01:55 >>>
>Do you mean to know which case fails on length checking? How about =
below=20
>patch?

Yes, something like this. Although I'd prefer to have a general
sizeof(struct acpi_dmar_entry_header) check before the switch
statement (to avoid even looking at out of range header fields),
and "break"s instead of "goto disable"s.

Jan

diff -r a4eac162dcb9 xen/drivers/passthrough/vtd/dmar.c
--- a/xen/drivers/passthrough/vtd/dmar.c    Thu Mar 25 01:05:03 2010 +0800
+++ b/xen/drivers/passthrough/vtd/dmar.c    Thu Mar 25 17:46:03 2010 +0800
@@ -664,21 +664,57 @@ static int __init acpi_parse_dmar(struct
         case ACPI_DMAR_DRHD:
             if ( iommu_verbose )
                 dprintk(VTDPREFIX, "found ACPI_DMAR_DRHD:\n");
+
+            if ( entry_header->length < sizeof(struct acpi_table_drhd) )
+            {
+                dprintk(XENLOG_ERR VTDPREFIX,
+                        "  Invalid length: 0x%x\n", entry_header->length);=

+                ret =3D -EINVAL;
+                goto disable;
+            }
+
             ret =3D acpi_parse_one_drhd(entry_header);
             break;
         case ACPI_DMAR_RMRR:
             if ( iommu_verbose )
                 dprintk(VTDPREFIX, "found ACPI_DMAR_RMRR:\n");
+
+            if ( entry_header->length < sizeof(struct acpi_table_rmrr) )
+            {
+                dprintk(XENLOG_ERR VTDPREFIX,
+                        "  Invalid length: 0x%x\n", entry_header->length);=

+                ret =3D -EINVAL;
+                goto disable;
+            }
+
             ret =3D acpi_parse_one_rmrr(entry_header);
             break;
         case ACPI_DMAR_ATSR:
             if ( iommu_verbose )
                 dprintk(VTDPREFIX, "found ACPI_DMAR_ATSR:\n");
+
+            if ( entry_header->length < sizeof(struct acpi_table_atsr) )
+            {
+                dprintk(XENLOG_ERR VTDPREFIX,
+                        "  Invalid length: 0x%x\n", entry_header->length);=

+                ret =3D -EINVAL;
+                goto disable;
+            }
+
             ret =3D acpi_parse_one_atsr(entry_header);
             break;
         case ACPI_DMAR_RHSA:
             if ( iommu_verbose )
                 dprintk(VTDPREFIX, "found ACPI_DMAR_RHSA:\n");
+
+            if ( entry_header->length < sizeof(struct acpi_table_rhsa) )
+            {
+                dprintk(XENLOG_ERR VTDPREFIX,
+                        "  Invalid length: 0x%x\n", entry_header->length);=

+                ret =3D -EINVAL;
+                goto disable;
+            }
+
             ret =3D acpi_parse_one_rhsa(entry_header);
             break;
         default:
@@ -694,6 +730,7 @@ static int __init acpi_parse_dmar(struct
         entry_header =3D ((void *)entry_header + entry_header->length);
     }

+disable:
     if ( ret )
     {
         printk(XENLOG_WARNING