From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Fri, 12 Apr 2019 16:31:50 -0700 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A97704628E; Fri, 12 Apr 2019 23:31:49 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-65.rdu2.redhat.com [10.10.120.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 33B556092F; Fri, 12 Apr 2019 23:31:48 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Anthony Perard , Ard Biesheuvel , Jordan Justen , Julien Grall Subject: [PATCH 09/10] OvmfPkg/AcpiPlatformDxe: catch theoretical nullptr deref in Xen code Date: Sat, 13 Apr 2019 01:31:27 +0200 Message-Id: <20190412233128.4756-10-lersek@redhat.com> In-Reply-To: <20190412233128.4756-1-lersek@redhat.com> References: <20190412233128.4756-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 12 Apr 2019 23:31:49 +0000 (UTC) Content-Transfer-Encoding: quoted-printable RH covscan justifiedly reports a path through InstallXenTables() where DsdtTable can technically remain NULL. If this occurs in practice, then the guest and the VMM are out of sync on the interface contract. Catch the situation with a code snippet that halt= s in RELEASE builds, and in DEBUG builds lets the platform DSC control the assert disposition first (i.e. CPU exception, deadloop, or nothing). > Error: CLANG_WARNING: > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:309:14: warning: Access > to field 'Length' results in a dereference of a null pointer (loaded > from variable 'DsdtTable') > # DsdtTable->Length, > # ^~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:154:3: note: Null > pointer value stored to 'DsdtTable' > # DsdtTable =3D NULL; > # ^~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:162:3: note: Taking > false branch > # if (EFI_ERROR (Status)) { > # ^ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:170:7: note: Assuming > the condition is false > # if (XenAcpiRsdpStructurePtr->XsdtAddress) { > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:170:3: note: Taking > false branch > # if (XenAcpiRsdpStructurePtr->XsdtAddress) { > # ^ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:220:12: note: Assuming > the condition is false > # else if (XenAcpiRsdpStructurePtr->RsdtAddress) { > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:220:8: note: Taking > false branch > # else if (XenAcpiRsdpStructurePtr->RsdtAddress) { > # ^ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:274:3: note: Taking > false branch > # if (Fadt2Table) { > # ^ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:288:8: note: Taking > false branch > # else if (Fadt1Table) { > # ^ > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:309:14: note: Access to > field 'Length' results in a dereference of a null pointer (loaded from > variable 'DsdtTable') > # DsdtTable->Length, > # ^~~~~~~~~ > # 307| AcpiProtocol, > # 308| DsdtTable, > # 309|-> DsdtTable->Length, > # 310| &TableHandle > # 311| ); Cc: Anthony Perard Cc: Ard Biesheuvel Cc: Jordan Justen Cc: Julien Grall Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1710 Issue: scan-0993.txt Signed-off-by: Laszlo Ersek --- OvmfPkg/AcpiPlatformDxe/Xen.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/OvmfPkg/AcpiPlatformDxe/Xen.c b/OvmfPkg/AcpiPlatformDxe/Xen.= c index c8f275a8ee84..b21d3db74dc8 100644 --- a/OvmfPkg/AcpiPlatformDxe/Xen.c +++ b/OvmfPkg/AcpiPlatformDxe/Xen.c @@ -295,18 +295,25 @@ InstallXenTables ( &TableHandle ); if (EFI_ERROR (Status)) { return Status; } } =20 // - // Install DSDT table. + // Install DSDT table. If we reached this point without finding the DS= DT, + // then we're out of sync with the hypervisor, and cannot continue. // + if (DsdtTable =3D=3D NULL) { + DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__)); + ASSERT (FALSE); + CpuDeadLoop (); + } + Status =3D InstallAcpiTable ( AcpiProtocol, DsdtTable, DsdtTable->Length, &TableHandle ); if (EFI_ERROR (Status)) { return Status; --=20 2.19.1.3.g30247aa5d201