From mboxrd@z Thu Jan  1 00:00:00 1970
Subject: [PATCH] NetworkPkg/Dhcp6Dxe: Fix FORWARD_NULL Coverity issue
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 01:00:33 -0800
Message-ID: <UQsa.1672822833659194099.UwmN@groups.io>
Content-Type: multipart/alternative; boundary="He0IO25CqVxbhhHiZpLN"

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

The function Dhcp6HandleStateful checks

if (Instance->Config =3D=3D NULL) {
goto ON_CONTINUE;
}

At label ON_CONTINUE, UdpIoRecvDatagram function is called and if for
whatever reasons its return value is not EFI_SUCCESS, then the check
if (EFI_ERROR (Status)) at label ON_EXIT passes leading to invokation
of Dhcp6CleanupSession function in which

ASSERT (Instance->Config);

will get hit in DEBUG mode and in RELEASE mode, the code continues to
dereference Instance->Config in the check

if (Instance->Config->IaInfoEvent !=3D NULL) {

which will lead to CRASH as Instance->Config is NULL.

Hence, for safety add Instance->Config NULL pointer check before
calling Dhcp6CleanupSession.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4223
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
---
NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c
index dcd01e6268..2c924d373f 100644
--- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c
+++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c
@@ -2636,7 +2636,7 @@ ON_CONTINUE:
0
);
ON_EXIT:
-=C2=A0 if (EFI_ERROR (Status)) {
+=C2=A0 if (EFI_ERROR (Status) && (Instance->Config !=3D NULL)) {
Dhcp6CleanupSession (Instance, Status);
}
}
--
2.36.1.windows.1

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

<div>The function Dhcp6HandleStateful checks</div>
<div>&nbsp;</div>
<div>&nbsp; if (Instance-&gt;Config =3D=3D NULL) {</div>
<div>&nbsp; &nbsp; goto ON_CONTINUE;</div>
<div>&nbsp; }</div>
<div>&nbsp;</div>
<div>At label ON_CONTINUE, UdpIoRecvDatagram function is called and if for<=
/div>
<div>whatever reasons its return value is not EFI_SUCCESS, then the check</=
div>
<div>if (EFI_ERROR (Status)) at label ON_EXIT passes leading to invokation<=
/div>
<div>of Dhcp6CleanupSession function in which</div>
<div>&nbsp;</div>
<div>&nbsp; ASSERT (Instance-&gt;Config);</div>
<div>&nbsp;</div>
<div>will get hit in DEBUG mode and in RELEASE mode, the code continues to<=
/div>
<div>dereference Instance-&gt;Config in the check</div>
<div>&nbsp;</div>
<div>&nbsp; if (Instance-&gt;Config-&gt;IaInfoEvent !=3D NULL) {</div>
<div>&nbsp;</div>
<div>which will lead to CRASH as Instance-&gt;Config is NULL.</div>
<div>&nbsp;</div>
<div>Hence, for safety add Instance-&gt;Config NULL pointer check before</d=
iv>
<div>calling Dhcp6CleanupSession.</div>
<div>&nbsp;</div>
<div>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4223</div>
<div>Signed-off-by: Ranbir Singh &lt;Ranbir.Singh3@Dell.com&gt;</div>
<div>---</div>
<div>&nbsp;NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 2 +-</div>
<div>&nbsp;1 file changed, 1 insertion(+), 1 deletion(-)</div>
<div>&nbsp;</div>
<div>diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6=
Io.c</div>
<div>index dcd01e6268..2c924d373f 100644</div>
<div>--- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c</div>
<div>+++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c</div>
<div>@@ -2636,7 +2636,7 @@ ON_CONTINUE:</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 0</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; );</div>
<div>&nbsp;ON_EXIT:</div>
<div>-&nbsp; if (EFI_ERROR (Status)) {</div>
<div>+&nbsp; if (EFI_ERROR (Status) &amp;&amp; (Instance-&gt;Config !=3D NU=
LL)) {</div>
<div>&nbsp; &nbsp; &nbsp;Dhcp6CleanupSession (Instance, Status);</div>
<div>&nbsp; &nbsp;}</div>
<div>&nbsp;}</div>
<div>--</div>
<div>2.36.1.windows.1</div>

--He0IO25CqVxbhhHiZpLN--