On 4/13/19 1:31 AM, Laszlo Ersek wrote:
> RH covscan emits the following false positive:
>
>> Error: CLANG_WARNING:
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:182:14:
>> warning: Dereference of undefined pointer value
>> # Status = FwVol->ReadSection (
>> # ^~~~~~~~~~~~~~~~~~
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:164:7: note:
>> Assuming the condition is false
>> # if (QemuDetected ()) {
>> # ^~~~~~~~~~~~~~~
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:164:3: note:
>> Taking false branch
>> # if (QemuDetected ()) {
>> # ^
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:174:3: note:
>> Taking false branch
>> # if (EFI_ERROR (Status)) {
>> # ^
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:180:10: note:
>> Assuming 'Status' is equal to EFI_SUCCESS
>> # while (Status == EFI_SUCCESS) {
>> # ^~~~~~~~~~~~~~~~~~~~~
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:180:3: note:
>> Loop condition is true. Entering loop body
>> # while (Status == EFI_SUCCESS) {
>> # ^
>> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:182:14: note:
>> Dereference of undefined pointer value
>> # Status = FwVol->ReadSection (
>> # ^~~~~~~~~~~~~~~~~~
>> # 180| while (Status == EFI_SUCCESS) {
>> # 181|
>> # 182|-> Status = FwVol->ReadSection (
>> # 183| FwVol,
>> # 184| (EFI_GUID*)PcdGetPtr
>> (PcdAcpiTableStorageFile),
>
> This is invalid because LocateFvInstanceWithTables() sets FwVol on
> success.
>
> Suppress the message by:
> - assigning FwVol NULL first (this would replace the original report with
> "nullptr deref"),
> - asserting that FwVol is no longer NULL, on success.
>
> What's important here is that ASSERT() ends with ANALYZER_UNREACHABLE() on
> failure.
>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Jordan Justen <[email protected]>
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
> Issue: scan-0991.txt
> Signed-off-by: Laszlo Ersek <[email protected]>
> ---
> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> index f2c49953950b..2b529d58a15c 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> @@ -156,23 +156,30 @@ InstallOvmfFvTables (
> TableHandle = 0;
>
> if (QemuDetected ()) {
> TableInstallFunction = QemuInstallAcpiTable;
> } else {
> TableInstallFunction = InstallAcpiTable;
> }
>
> + //
> + // set FwVol (and use an ASSERT() below) to suppress incorrect
> + // compiler/analyzer warnings
> + //
> + FwVol = NULL;
> //
> // Locate the firmware volume protocol
> //
> Status = LocateFvInstanceWithTables (&FwVol);
> if (EFI_ERROR (Status)) {
> return EFI_ABORTED;
> }
> + ASSERT (FwVol != NULL);
> +
> //
> // Read tables from the storage file.
> //
> while (Status == EFI_SUCCESS) {
>
> Status = FwVol->ReadSection (
> FwVol,
> (EFI_GUID*)PcdGetPtr (PcdAcpiTableStorageFile),
>
Reviewed-by: Philippe Mathieu-Daude <[email protected]>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39107): https://edk2.groups.io/g/devel/message/39107
Mute This Topic: https://groups.io/mt/31070307/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
