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 <[email protected]>
Cc: Jordan Justen <[email protected]>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-0994.txt
Signed-off-by: Laszlo Ersek <[email protected]>
---
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 4968d2b478db..c6f2c726509f 100644
--- a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
+++ b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
@@ -298,16 +298,23 @@ InsertPciCap (
goto DeletePciCapFromCapList;
}
//
// Now we can bump the instance count maintained in Instance#0, if PciCap is
// 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;
DeletePciCapFromCapList:
OrderedCollectionDelete (CapList->Capabilities, PciCapEntry, NULL);
FreePciCap:
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#38995): https://edk2.groups.io/g/devel/message/38995
Mute This Topic: https://groups.io/mt/31070310/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
