From mboxrd@z Thu Jan  1 00:00:00 1970
Subject: [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix FORWARD_NULL Coverity issues
To: devel@edk2.groups.io
From: "Ranbir Singh" <Ranbir.Singh3@Dell.com>
X-Originating-Location: Bengaluru, Karnataka, IN (122.172.85.38)
X-Originating-Platform: Windows Chrome 108
User-Agent: GROUPS.IO Web Poster
MIME-Version: 1.0
Date: Wed, 04 Jan 2023 00:54:34 -0800
Message-ID: <cVZ8.1672822474661449806.PPuA@groups.io>
Content-Type: multipart/alternative; boundary="dJwipm7XRrK9QZAt7AsA"

--dJwipm7XRrK9QZAt7AsA
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

The functions UsbHcGetHostAddrForPciAddr, UsbHcGetPciAddrForHostAddr
and UsbHcFreeMem do have

ASSERT ((Block !=3D NULL));

statements after for loop, but these are applicable only in DEBUG mode.
In RELEASE mode, if for whatever reasons there is no match inside for
loop and the loop exits because of Block !=3D NULL; condition, then there
is no "Block" NULL pointer check afterwards and the code proceeds to do
dereferencing "Block" which will lead to CRASH.

Hence, for safety add NULL pointer checks always.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4221
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
---
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c b/MdeModulePkg/Bus/Pci=
/XhciDxe/UsbHcMem.c
index d0ad1582e4..869ae6ec8a 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
@@ -261,6 +261,10 @@ UsbHcGetPciAddrForHostAddr (
}

ASSERT ((Block !=3D NULL));
+=C2=A0 if (Block =3D=3D NULL) {
+=C2=A0 =C2=A0 return 0;
+=C2=A0 }
+
//
// calculate the pci memory address for host memory address.
//
@@ -310,6 +314,10 @@ UsbHcGetHostAddrForPciAddr (
}

ASSERT ((Block !=3D NULL));
+=C2=A0 if (Block =3D=3D NULL) {
+=C2=A0 =C2=A0 return 0;
+=C2=A0 }
+
//
// calculate the pci memory address for host memory address.
//
@@ -590,6 +598,9 @@ UsbHcFreeMem (
// the caller has passed in a wrong memory point
//
ASSERT (Block !=3D NULL);
+=C2=A0 if (Block =3D=3D NULL) {
+=C2=A0 =C2=A0 return;
+=C2=A0 }

//
// Release the current memory block if it is empty and not the head
--
2.36.1.windows.1

--dJwipm7XRrK9QZAt7AsA
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

<div>The functions UsbHcGetHostAddrForPciAddr, UsbHcGetPciAddrForHostAddr</=
div>
<div>and UsbHcFreeMem do have</div>
<div>&nbsp;</div>
<div>&nbsp; &nbsp; ASSERT ((Block !=3D NULL));</div>
<div>&nbsp;</div>
<div>statements after for loop, but these are applicable only in DEBUG mode=
.</div>
<div>In RELEASE mode, if for whatever reasons there is no match inside for<=
/div>
<div>loop and the loop exits because of Block !=3D NULL; condition, then th=
ere</div>
<div>is no "Block" NULL pointer check afterwards and the code proceeds to d=
o</div>
<div>dereferencing "Block" which will lead to CRASH.</div>
<div>&nbsp;</div>
<div>Hence, for safety add NULL pointer checks always.</div>
<div>&nbsp;</div>
<div>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4221</div>
<div>Signed-off-by: Ranbir Singh &lt;Ranbir.Singh3@Dell.com&gt;</div>
<div>---</div>
<div>&nbsp;MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 11 +++++++++++</div>
<div>&nbsp;1 file changed, 11 insertions(+)</div>
<div>&nbsp;</div>
<div>diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c b/MdeModulePkg/Bu=
s/Pci/XhciDxe/UsbHcMem.c</div>
<div>index d0ad1582e4..869ae6ec8a 100644</div>
<div>--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c</div>
<div>+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c</div>
<div>@@ -261,6 +261,10 @@ UsbHcGetPciAddrForHostAddr (</div>
<div>&nbsp; &nbsp;}</div>
<div>&nbsp;</div>
<div>&nbsp; &nbsp;ASSERT ((Block !=3D NULL));</div>
<div>+&nbsp; if (Block =3D=3D NULL) {</div>
<div>+&nbsp; &nbsp; return 0;</div>
<div>+&nbsp; }</div>
<div>+</div>
<div>&nbsp; &nbsp;//</div>
<div>&nbsp; &nbsp;// calculate the pci memory address for host memory addre=
ss.</div>
<div>&nbsp; &nbsp;//</div>
<div>@@ -310,6 +314,10 @@ UsbHcGetHostAddrForPciAddr (</div>
<div>&nbsp; &nbsp;}</div>
<div>&nbsp;</div>
<div>&nbsp; &nbsp;ASSERT ((Block !=3D NULL));</div>
<div>+&nbsp; if (Block =3D=3D NULL) {</div>
<div>+&nbsp; &nbsp; return 0;</div>
<div>+&nbsp; }</div>
<div>+</div>
<div>&nbsp; &nbsp;//</div>
<div>&nbsp; &nbsp;// calculate the pci memory address for host memory addre=
ss.</div>
<div>&nbsp; &nbsp;//</div>
<div>@@ -590,6 +598,9 @@ UsbHcFreeMem (</div>
<div>&nbsp; &nbsp;// the caller has passed in a wrong memory point</div>
<div>&nbsp; &nbsp;//</div>
<div>&nbsp; &nbsp;ASSERT (Block !=3D NULL);</div>
<div>+&nbsp; if (Block =3D=3D NULL) {</div>
<div>+&nbsp; &nbsp; return;</div>
<div>+&nbsp; }</div>
<div>&nbsp;</div>
<div>&nbsp; &nbsp;//</div>
<div>&nbsp; &nbsp;// Release the current memory block if it is empty and no=
t the head</div>
<div>--</div>
<div>2.36.1.windows.1</div>

--dJwipm7XRrK9QZAt7AsA--