]> git.proxmox.com Git - mirror_edk2.git/commit
OvmfPkg/AcpiPlatformDxe: catch theoretical nullptr deref in Xen code
authorLaszlo Ersek <lersek@redhat.com>
Fri, 12 Apr 2019 16:27:18 +0000 (18:27 +0200)
committerLaszlo Ersek <lersek@redhat.com>
Thu, 18 Apr 2019 14:05:24 +0000 (16:05 +0200)
commite30991740d1874de71a3715a14b380a0a0cadc14
treeb44e7bf2059b5d596c68f4d94554c4df30828125
parentdc5bbf10741cb385f4357e33e214b7ba23085432
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>
OvmfPkg/AcpiPlatformDxe/Xen.c