From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Fri, 12 Apr 2019 16:31:51 -0700 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2649A3084242; Fri, 12 Apr 2019 23:31:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-65.rdu2.redhat.com [10.10.120.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 15F2D6090C; Fri, 12 Apr 2019 23:31:49 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Ard Biesheuvel , Jordan Justen Subject: [PATCH 10/10] OvmfPkg/BasePciCapLib: suppress invalid "nullptr deref" warning Date: Sat, 13 Apr 2019 01:31:28 +0200 Message-Id: <20190412233128.4756-11-lersek@redhat.com> In-Reply-To: <20190412233128.4756-1-lersek@redhat.com> References: <20190412233128.4756-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Fri, 12 Apr 2019 23:31:51 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 =3D=3D NULL) { > # ^~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:509:3: > note: Taking false branch > # if (OutCapList =3D=3D NULL) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:518:7: > note: Assuming the condition is false > # if (OutCapList->Capabilities =3D=3D NULL) { > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:518:3: > note: Taking false branch > # if (OutCapList->Capabilities =3D=3D NULL) { > # ^ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:529:7: > note: Assuming 'CapHdrOffsets' is not equal to NULL > # if (CapHdrOffsets =3D=3D NULL) { > # ^~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:529:3: > note: Taking false branch > # if (CapHdrOffsets =3D=3D 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) !=3D 0) { > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:549:3: > note: Taking true branch > # if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) !=3D 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 =3D InsertPciCap (OutCapList, CapHdrOffsets, PciCapNormal= , > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:235:3: > note: Null pointer value stored to 'InstanceZero' > # InstanceZero =3D NULL; > # ^~~~~~~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:243:7: > note: Assuming 'PciCap' is not equal to NULL > # if (PciCap =3D=3D NULL) { > # ^~~~~~~~~~~~~~ > edk2-89910a39dcfd/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c:243:3: > note: Taking false branch > # if (PciCap =3D=3D 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 =3D OrderedCollectionInsert (CapList->Capabilities, &Pci= CapEntry, 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=3D1710 Issue: scan-0994.txt Signed-off-by: Laszlo Ersek --- OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c b/OvmfPkg/Libr= ary/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; } =20 // // Now we can bump the instance count maintained in Instance#0, if Pci= Cap 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 !=3D NULL); + InstanceZero->NumInstancesUnion.NumInstances++; } return RETURN_SUCCESS; =20 DeletePciCapFromCapList: OrderedCollectionDelete (CapList->Capabilities, PciCapEntry, NULL); =20 FreePciCap: --=20 2.19.1.3.g30247aa5d201