diff options
| author | Laszlo Ersek <lersek@redhat.com> | 2019-04-12 18:27:18 +0200 |
|---|---|---|
| committer | Laszlo Ersek <lersek@redhat.com> | 2019-04-18 16:05:24 +0200 |
| commit | e30991740d1874de71a3715a14b380a0a0cadc14 (patch) | |
| tree | b44e7bf2059b5d596c68f4d94554c4df30828125 /OvmfPkg | |
| parent | dc5bbf10741cb385f4357e33e214b7ba23085432 (diff) | |
| download | edk2-e30991740d1874de71a3715a14b380a0a0cadc14.tar.gz edk2-e30991740d1874de71a3715a14b380a0a0cadc14.tar.bz2 edk2-e30991740d1874de71a3715a14b380a0a0cadc14.zip | |
OvmfPkg/AcpiPlatformDxe: catch theoretical nullptr deref in Xen code
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 halts
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 = 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 <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-0993.txt
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Diffstat (limited to 'OvmfPkg')
| -rw-r--r-- | OvmfPkg/AcpiPlatformDxe/Xen.c | 9 |
1 files changed, 8 insertions, 1 deletions
diff --git a/OvmfPkg/AcpiPlatformDxe/Xen.c b/OvmfPkg/AcpiPlatformDxe/Xen.c index 357c60d23f..e4e47bf0e8 100644 --- a/OvmfPkg/AcpiPlatformDxe/Xen.c +++ b/OvmfPkg/AcpiPlatformDxe/Xen.c @@ -295,8 +295,15 @@ InstallXenTables ( }
//
- // Install DSDT table.
+ // Install DSDT table. If we reached this point without finding the DSDT,
+ // then we're out of sync with the hypervisor, and cannot continue.
//
+ if (DsdtTable == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+
Status = InstallAcpiTable (
AcpiProtocol,
DsdtTable,
|