Re: [edk2-devel] [PATCH 07/10] OvmfPkg/AcpiPlatformDxe: suppress invalid "deref of undef pointer" warning
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 > Cc: Jordan Justen > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 > Issue: scan-0991.txt > Signed-off-by: Laszlo Ersek > --- > 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 (); >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 -=-=-=-=-=-=-=-=-=-=-=- 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: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 07/10] OvmfPkg/AcpiPlatformDxe: suppress invalid "deref of undef pointer" warning
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 Cc: Jordan Justen Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 Issue: scan-0991.txt Signed-off-by: Laszlo Ersek --- 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 (); 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), -- 2.19.1.3.g30247aa5d201 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38992): https://edk2.groups.io/g/devel/message/38992 Mute This Topic: https://groups.io/mt/31070307/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-