MdeModulePkg: Replace ASSERT with error return code in PXE driver.

This patch remove the ASSERT when receive a DHCP packet large than the maximum
cache buffer size.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
index f03176b7cae07e3482b46443debe33ce63a78bf5..f0720e5a8cf2a89ffb014da5099eb2b96bbf0cb9 100644 (file)
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
@@ -69,20 +69,26 @@ PxeBcInitSeedPacket (
 /**\r
   Copy the DCHP4 packet from srouce to destination.\r
 \r
-  @param  Dst   Pointer to the EFI_DHCP4_PROTOCOL instance.\r
-  @param  Src   Pointer to the EFI_DHCP4_PROTOCOL instance.\r
+  @param[in]  Dst          Pointer to the cache buffer for DHCPv4 packet.\r
+  @param[in]  Src          Pointer to the DHCPv4 packet to be cached.\r
+\r
+  @retval     EFI_SUCCESS                Packet is copied.\r
+  @retval     EFI_BUFFER_TOO_SMALL       Cache buffer is not big enough to hold the packet.\r
 \r
 **/\r
-VOID\r
+EFI_STATUS\r
 PxeBcCopyEfiDhcp4Packet (\r
   IN EFI_DHCP4_PACKET  *Dst,\r
   IN EFI_DHCP4_PACKET  *Src\r
   )\r
 {\r
-  ASSERT (Dst->Size >= Src->Length);\r
+  if (Dst->Size < Src->Length) {\r
+    return EFI_BUFFER_TOO_SMALL;\r
+  }\r
 \r
   CopyMem (&Dst->Dhcp4, &Src->Dhcp4, Src->Length);\r
   Dst->Length = Src->Length;\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 \r
@@ -93,8 +99,11 @@ PxeBcCopyEfiDhcp4Packet (
   @param  OfferIndex    Index of cached packets as complements of pxe mode data,\r
                         the index is maximum offer number.\r
 \r
+  @retval     EFI_SUCCESS                Cache and parse the packet successfully.\r
+  @retval     EFI_BUFFER_TOO_SMALL       Cache buffer is not big enough to hold the packet.\r
+\r
 **/\r
-VOID\r
+EFI_STATUS\r
 PxeBcCopyProxyOffer (\r
   IN PXEBC_PRIVATE_DATA  *Private,\r
   IN UINT32              OfferIndex\r
@@ -102,6 +111,7 @@ PxeBcCopyProxyOffer (
 {\r
   EFI_PXE_BASE_CODE_MODE  *Mode;\r
   EFI_DHCP4_PACKET        *Offer;\r
+  EFI_STATUS              Status;\r
 \r
   ASSERT (OfferIndex < Private->NumOffers);\r
   ASSERT (OfferIndex < PXEBC_MAX_OFFER_NUM);\r
@@ -109,11 +119,15 @@ PxeBcCopyProxyOffer (
   Mode  = Private->PxeBc.Mode;\r
   Offer = &Private->Dhcp4Offers[OfferIndex].Packet.Offer;\r
 \r
-  PxeBcCopyEfiDhcp4Packet (&Private->ProxyOffer.Packet.Offer, Offer);\r
+  Status = PxeBcCopyEfiDhcp4Packet (&Private->ProxyOffer.Packet.Offer, Offer);\r
+  if (EFI_ERROR(Status)) {\r
+    return Status;\r
+  }\r
   CopyMem (&Mode->ProxyOffer, &Offer->Dhcp4, Offer->Length);\r
   Mode->ProxyOfferReceived = TRUE;\r
 \r
   PxeBcParseCachedDhcpPacket (&Private->ProxyOffer);\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 \r
@@ -411,8 +425,9 @@ PxeBcTryBinlProxy (
 \r
   @param  Private          Pointer to PxeBc private data.\r
 \r
-  @retval EFI_SUCCESS      Operational successful.\r
-  @retval EFI_NO_RESPONSE  Offer dhcp service failed.\r
+  @retval EFI_SUCCESS                Operational successful.\r
+  @retval EFI_NO_RESPONSE            Offer dhcp service failed.\r
+  @retval EFI_BUFFER_TOO_SMALL       Failed to copy the packet to Pxe base code mode.\r
 \r
 **/\r
 EFI_STATUS\r
@@ -515,7 +530,7 @@ PxeBcCheckSelectedOffer (
         //\r
         // Copy the proxy offer to Mode and set the flag\r
         //\r
-        PxeBcCopyProxyOffer (Private, ProxyOfferIndex);\r
+        Status = PxeBcCopyProxyOffer (Private, ProxyOfferIndex);\r
       }\r
     } else {\r
       //\r
@@ -543,7 +558,7 @@ PxeBcCheckSelectedOffer (
       // Other type of ACK is already cached. Bootp is special that we should\r
       // use the bootp reply as the ACK and put it into the DHCP_ONLY buffer.\r
       //\r
-      PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Offer);\r
+      Status = PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Offer);\r
     }\r
 \r
     PxeBcParseCachedDhcpPacket (&Private->Dhcp4Ack);\r
@@ -566,8 +581,11 @@ PxeBcCheckSelectedOffer (
   @param  Private    Pointer to PxeBc private data.\r
   @param  RcvdOffer  Pointer to the received Dhcp proxy offer packet.\r
 \r
+  @retval     EFI_SUCCESS      Cache and parse the packet successfully.\r
+  @retval     Others           Operation failed.\r
+\r
 **/\r
-VOID\r
+EFI_STATUS\r
 PxeBcCacheDhcpOffer (\r
   IN PXEBC_PRIVATE_DATA  *Private,\r
   IN EFI_DHCP4_PACKET    *RcvdOffer\r
@@ -576,6 +594,7 @@ PxeBcCacheDhcpOffer (
   PXEBC_CACHED_DHCP4_PACKET *CachedOffer;\r
   EFI_DHCP4_PACKET          *Offer;\r
   UINT8                     OfferType;\r
+  EFI_STATUS                Status;\r
 \r
   CachedOffer = &Private->Dhcp4Offers[Private->NumOffers];\r
   Offer       = &CachedOffer->Packet.Offer;\r
@@ -583,18 +602,21 @@ PxeBcCacheDhcpOffer (
   //\r
   // Cache the orignal dhcp packet\r
   //\r
-  PxeBcCopyEfiDhcp4Packet (Offer, RcvdOffer);\r
+  Status = PxeBcCopyEfiDhcp4Packet (Offer, RcvdOffer);\r
+  if (EFI_ERROR(Status)) {\r
+    return Status;\r
+  }\r
 \r
   //\r
   // Parse and validate the options (including dhcp option and vendor option)\r
   //\r
   if (!PxeBcParseCachedDhcpPacket (CachedOffer)) {\r
-    return ;\r
+    return EFI_ABORTED;\r
   }\r
 \r
   OfferType = CachedOffer->OfferType;\r
   if (OfferType >= DHCP4_PACKET_TYPE_MAX) {\r
-    return ;\r
+    return EFI_ABORTED;\r
   }\r
 \r
   if (OfferType == DHCP4_PACKET_TYPE_BOOTP) {\r
@@ -603,7 +625,7 @@ PxeBcCacheDhcpOffer (
       //\r
       // Only cache the first bootp offer, discard others.\r
       //\r
-      return ;\r
+      return EFI_ABORTED;\r
     } else {\r
       //\r
       // Take as a dhcp only offer, but record index specifically.\r
@@ -628,7 +650,7 @@ PxeBcCacheDhcpOffer (
         //\r
         // Only cache the first pxe10/wfm11a offers each, discard the others.\r
         //\r
-        return ;\r
+        return EFI_ABORTED;\r
       } else {\r
         //\r
         // Record index of the proxy dhcp offer with type other than binl.\r
@@ -649,6 +671,8 @@ PxeBcCacheDhcpOffer (
   // Count the accepted offers.\r
   //\r
   Private->NumOffers++;\r
+\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 /**\r
@@ -960,6 +984,7 @@ PxeBcDhcpCallBack (
     if (Private->NumOffers < PXEBC_MAX_OFFER_NUM) {\r
       //\r
       // Cache the dhcp offers in Private->Dhcp4Offers[]\r
+      // If error happens, just ignore this packet and continue to wait more offer.\r
       //\r
       PxeBcCacheDhcpOffer (Private, Packet);\r
     }\r
@@ -982,20 +1007,15 @@ PxeBcDhcpCallBack (
     break;\r
 \r
   case Dhcp4RcvdAck:\r
-    if (Packet->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {\r
-      //\r
-      // Abort the DHCP if the ACK packet exceeds the maximum length.\r
-      //\r
-         Status = EFI_ABORTED;\r
-      break;\r
-    }\r
-\r
     //\r
     // Cache Ack\r
     //\r
     ASSERT (Private->SelectedOffer != 0);\r
 \r
-    PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Packet);\r
+    Status = PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Packet);\r
+    if (EFI_ERROR (Status)) {\r
+      return EFI_ABORTED;\r
+    }\r
     break;\r
 \r
   default:\r
@@ -1340,6 +1360,12 @@ PxeBcDiscvBootService (
     Response  = Token.ResponseList;\r
 \r
     while (RepIndex < Token.ResponseCount) {\r
+      if (Response->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {\r
+        SrvIndex = 0;\r
+        RepIndex++;\r
+        Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size);\r
+        continue;\r
+      }\r
 \r
       while (SrvIndex < IpCount) {\r
 \r
@@ -1360,14 +1386,16 @@ PxeBcDiscvBootService (
 \r
       SrvIndex = 0;\r
       RepIndex++;\r
-\r
       Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size);\r
     }\r
 \r
     if (RepIndex < Token.ResponseCount) {\r
 \r
       if (Reply != NULL) {\r
-        PxeBcCopyEfiDhcp4Packet (Reply, Response);\r
+        Status = PxeBcCopyEfiDhcp4Packet (Reply, Response);\r
+        if (EFI_ERROR(Status)) {\r
+          goto ON_EXIT;\r
+        }\r
       }\r
 \r
       if (IsDiscv) {\r
@@ -1380,18 +1408,21 @@ PxeBcDiscvBootService (
     } else {\r
       Status = EFI_NOT_FOUND;\r
     }\r
+  }\r
 \r
-    //\r
-    // free the responselist\r
-    //\r
-    if (Token.ResponseList != NULL) {\r
-      FreePool (Token.ResponseList);\r
-    }\r
+ON_EXIT:\r
+  //\r
+  // free the responselist\r
+  //\r
+  if (Token.ResponseList != NULL) {\r
+    FreePool (Token.ResponseList);\r
   }\r
   //\r
   // Free the dhcp packet\r
   //\r
-  FreePool (Token.Packet);\r
+  if (Token.Packet != NULL) {\r
+    FreePool (Token.Packet);\r
+  }\r
 \r
   return Status;\r
 }\r

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
index 614ea75e759b0194a682dc787d22bb5ac6cc8965..d19d2a318f3c154961ed4af54d097158436e9951 100644 (file)
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
@@ -291,8 +291,9 @@ PxeBcParseCachedDhcpPacket (
 \r
   @param  Private          Pointer to PxeBc private data.\r
 \r
-  @retval EFI_SUCCESS      Operational successful.\r
-  @retval EFI_NO_RESPONSE  Offer dhcp service failed.\r
+  @retval EFI_SUCCESS                Operational successful.\r
+  @retval EFI_NO_RESPONSE            Offer dhcp service failed.\r
+  @retval EFI_BUFFER_TOO_SMALL       Failed to copy the packet to Pxe base code mode.\r
 \r
 **/\r
 EFI_STATUS\r