Re: [edk2-devel] [PATCH] NetworkPkg:MnpDxe:CoverityIssues

["Saloni Kasbekar" <saloni.kasbekar@intel.com>]

Both the cases have ASSERTs checking that the value is greater than 0. Move the checks to the same location as the ASSERT. I'd also prefer if we return with a failure instead of forcing continuation of the function with an invalid value.

Thanks,
Saloni

-----Original Message-----
From: Santhosh Kumar V <santhoshkumarv@ami.com> 
Sent: Friday, May 10, 2024 3:05 AM
To: devel@edk2.groups.io; Santhosh Kumar V <santhoshkumarv@ami.com>
Cc: Sivaraman Nainar <sivaramann@ami.com>; Raj V Akilan <rajva@ami.com>; Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>
Subject: [PATCH] NetworkPkg:MnpDxe:CoverityIssues

Resolved INTEGER_OVERFLOW Coverity Issues in MNP Dxe 1.MnpStop,MnpInstanceDeliverPacket
Expression "MnpDeviceData->ConfiguredChildrenNumber--" and Instance->RcvdPacketQueueSize--  where Both variables are known to be equal to 0, underflows the type that receives it.


Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>

Signed-off-by: SanthoshKumarV <santhoshkumarv@ami.com>
---
 NetworkPkg/MnpDxe/MnpConfig.c | 3 ++-
 NetworkPkg/MnpDxe/MnpIo.c     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/MnpDxe/MnpConfig.c b/NetworkPkg/MnpDxe/MnpConfig.c index 93587d53aa..80253e88de 100644
--- a/NetworkPkg/MnpDxe/MnpConfig.c
+++ b/NetworkPkg/MnpDxe/MnpConfig.c
@@ -1313,7 +1313,8 @@ MnpStop (
   //

   // Decrease the children number.

   //

-  MnpDeviceData->ConfiguredChildrenNumber--;

+  if (MnpDeviceData->ConfiguredChildrenNumber > 0)

+      MnpDeviceData->ConfiguredChildrenNumber--;



   if (MnpDeviceData->ConfiguredChildrenNumber > 0) {

     //

diff --git a/NetworkPkg/MnpDxe/MnpIo.c b/NetworkPkg/MnpDxe/MnpIo.c index 087c879c46..492edd9b66 100644
--- a/NetworkPkg/MnpDxe/MnpIo.c
+++ b/NetworkPkg/MnpDxe/MnpIo.c
@@ -351,7 +351,8 @@ MnpInstanceDeliverPacket (
   // All resources are OK, remove the packet from the queue.

   //

   NetListRemoveHead (&Instance->RcvdPacketQueue);

-  Instance->RcvdPacketQueueSize--;

+  if (Instance->RcvdPacketQueueSize != 0)

+      Instance->RcvdPacketQueueSize--;



   RxData  = &RxDataWrap->RxData;

   SnpMode = MnpDeviceData->Snp->Mode;

--
2.42.0.windows.2
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119057): https://edk2.groups.io/g/devel/message/119057
Mute This Topic: https://groups.io/mt/106018089/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-