From c2f643479eb3f00d930c461cdec3ba420a36072c Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Fri, 12 Apr 2019 19:00:52 +0200 Subject: [PATCH] OvmfPkg/BasePciCapLib: suppress invalid "nullptr deref" warning RH covscan reports the following "nullptr deref" warning: > Error: CLANG_WARNING: > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:312:5: > warning: Dereference of null pointer > # InstanceZero->NumInstancesUnion.NumInstances++; > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:509:7: > note: Assuming 'OutCapList' is not equal to NULL > # if (OutCapList == NULL) { > # ^~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:509:3: > note: Taking false branch > # if (OutCapList == NULL) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:518:7: > note: Assuming the condition is false > # if (OutCapList->Capabilities == NULL) { > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:518:3: > note: Taking false branch > # if (OutCapList->Capabilities == NULL) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:529:7: > note: Assuming 'CapHdrOffsets' is not equal to NULL > # if (CapHdrOffsets == NULL) { > # ^~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:529:3: > note: Taking false branch > # if (CapHdrOffsets == NULL) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:546:3: > note: Taking false branch > # if (RETURN_ERROR (Status)) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:549:7: > note: Assuming the condition is true > # if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) { > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:549:3: > note: Taking true branch > # if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:557:5: > note: Taking false branch > # if (RETURN_ERROR (Status)) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:565:12: > note: Assuming 'NormalCapHdrOffset' is > 0 > # while (NormalCapHdrOffset > 0) { > # ^~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:565:5: > note: Loop condition is true. Entering loop body > # while (NormalCapHdrOffset > 0) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:570:7: > note: Taking false branch > # if (RETURN_ERROR (Status)) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:574:16: > note: Calling 'InsertPciCap' > # Status = InsertPciCap (OutCapList, CapHdrOffsets, PciCapNormal, > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:235:3: > note: Null pointer value stored to 'InstanceZero' > # InstanceZero = NULL; > # ^~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:243:7: > note: Assuming 'PciCap' is not equal to NULL > # if (PciCap == NULL) { > # ^~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:243:3: > note: Taking false branch > # if (PciCap == NULL) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:259:3: > note: Taking false branch > # if (RETURN_ERROR (Status)) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:297:3: > note: Taking false branch > # if (RETURN_ERROR (Status)) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:311:7: > note: Assuming the condition is true > # if (PciCap->Key.Instance > 0) { > # ^~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:311:3: > note: Taking true branch > # if (PciCap->Key.Instance > 0) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:312:5: > note: Dereference of null pointer > # InstanceZero->NumInstancesUnion.NumInstances++; > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > # 310| // > # 311| if (PciCap->Key.Instance > 0) { > # 312|-> InstanceZero->NumInstancesUnion.NumInstances++; > # 313| } > # 314| return RETURN_SUCCESS; The warning is invalid: the flagged dereferencing of "InstanceZero" is gated by a condition that is only satisfied if we dereference "InstanceZero" *first*. (Perhaps the analyzer assumes that the OrderedCollectionInsert() call, just before line 259, can change the value of "PciCap->Key.Instance" via the last argument: 254 // 255 // Add PciCap to CapList. 256 // 257 Status = OrderedCollectionInsert (CapList->Capabilities, &PciCapEntry, 258 PciCap); 259 if (RETURN_ERROR (Status)) { That assumption is incorrect.) Add a comment and an ASSERT(). Cc: Ard Biesheuvel Cc: Jordan Justen Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 Issue: scan-0994.txt Signed-off-by: Laszlo Ersek Reviewed-by: Philippe Mathieu-Daude Acked-by: Ard Biesheuvel --- OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c index 4968d2b478..c6f2c72650 100644 --- a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c +++ b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c @@ -303,6 +303,13 @@ InsertPciCap ( // not the first instance of (Domain, CapId). // if (PciCap->Key.Instance > 0) { + // + // Suppress invalid "nullptr dereference" compiler/analyzer warnings: the + // only way for "PciCap->Key.Instance" to be positive here is for it to + // have been assigned *from* dereferencing "InstanceZero" above. + // + ASSERT (InstanceZero != NULL); + InstanceZero->NumInstancesUnion.NumInstances++; } return RETURN_SUCCESS; -- 2.39.2