From 3e8a51b577ba69022f33ed66a4ee55d1e3e80931 Mon Sep 17 00:00:00 2001 From: Fu Siyuan Date: Fri, 16 Dec 2016 15:56:42 +0800 Subject: NetworkPkg: 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 Reviewed-by: Ye Ting Reviewed-by: Wu Jiaxin (cherry picked from commit a35dc6499beb0b76c340379a06dff74a8d38095a) --- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 119 ++++++++++++++++++++++++----------- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 77 ++++++++++++++++------- 2 files changed, 138 insertions(+), 58 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c index f4c79cf1e9..8d63ee40d7 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c @@ -424,17 +424,24 @@ PxeBcSeedDhcp4Packet ( @param[in] Dst Pointer to the cache buffer for DHCPv4 packet. @param[in] Src Pointer to the DHCPv4 packet to be cached. + @retval EFI_SUCCESS Packet is copied. + @retval EFI_BUFFER_TOO_SMALL Cache buffer is not big enough to hold the packet. + **/ -VOID +EFI_STATUS PxeBcCacheDhcp4Packet ( IN EFI_DHCP4_PACKET *Dst, IN EFI_DHCP4_PACKET *Src ) { - ASSERT (Dst->Size >= Src->Length); - + if (Dst->Size < Src->Length) { + return EFI_BUFFER_TOO_SMALL; + } + CopyMem (&Dst->Dhcp4, &Src->Dhcp4, Src->Length); Dst->Length = Src->Length; + + return EFI_SUCCESS; } @@ -617,8 +624,11 @@ PxeBcParseDhcp4Packet ( @param[in] Ack Pointer to the DHCPv4 ack packet. @param[in] Verified If TRUE, parse the ACK packet and store info into mode data. + @retval EFI_SUCCESS Cache and parse the packet successfully. + @retval EFI_BUFFER_TOO_SMALL Cache buffer is not big enough to hold the packet. + **/ -VOID +EFI_STATUS PxeBcCopyDhcp4Ack ( IN PXEBC_PRIVATE_DATA *Private, IN EFI_DHCP4_PACKET *Ack, @@ -626,10 +636,14 @@ PxeBcCopyDhcp4Ack ( ) { EFI_PXE_BASE_CODE_MODE *Mode; + EFI_STATUS Status; Mode = Private->PxeBc.Mode; - PxeBcCacheDhcp4Packet (&Private->DhcpAck.Dhcp4.Packet.Ack, Ack); + Status = PxeBcCacheDhcp4Packet (&Private->DhcpAck.Dhcp4.Packet.Ack, Ack); + if (EFI_ERROR (Status)) { + return Status; + } if (Verified) { // @@ -639,6 +653,8 @@ PxeBcCopyDhcp4Ack ( CopyMem (&Mode->DhcpAck.Dhcpv4, &Ack->Dhcp4, Ack->Length); Mode->DhcpAckReceived = TRUE; } + + return EFI_SUCCESS; } @@ -648,8 +664,11 @@ PxeBcCopyDhcp4Ack ( @param[in] Private Pointer to PxeBc private data. @param[in] OfferIndex The received order of offer packets. + @retval EFI_SUCCESS Cache and parse the packet successfully. + @retval EFI_BUFFER_TOO_SMALL Cache buffer is not big enough to hold the packet. + **/ -VOID +EFI_STATUS PxeBcCopyProxyOffer ( IN PXEBC_PRIVATE_DATA *Private, IN UINT32 OfferIndex @@ -657,6 +676,7 @@ PxeBcCopyProxyOffer ( { EFI_PXE_BASE_CODE_MODE *Mode; EFI_DHCP4_PACKET *Offer; + EFI_STATUS Status; ASSERT (OfferIndex < Private->OfferNum); ASSERT (OfferIndex < PXEBC_OFFER_MAX_NUM); @@ -667,7 +687,11 @@ PxeBcCopyProxyOffer ( // // Cache the proxy offer packet and parse it. // - PxeBcCacheDhcp4Packet (&Private->ProxyOffer.Dhcp4.Packet.Offer, Offer); + Status = PxeBcCacheDhcp4Packet (&Private->ProxyOffer.Dhcp4.Packet.Offer, Offer); + if (EFI_ERROR(Status)) { + return Status; + } + PxeBcParseDhcp4Packet (&Private->ProxyOffer.Dhcp4); // @@ -675,6 +699,8 @@ PxeBcCopyProxyOffer ( // CopyMem (&Mode->ProxyOffer.Dhcpv4, &Offer->Dhcp4, Offer->Length); Mode->ProxyOfferReceived = TRUE; + + return EFI_SUCCESS; } @@ -777,8 +803,11 @@ PxeBcRetryBinlOffer ( @param[in] Private Pointer to PxeBc private data. @param[in] RcvdOffer Pointer to the received offer packet. + @retval EFI_SUCCESS Cache and parse the packet successfully. + @retval Others Operation failed. + **/ -VOID +EFI_STATUS PxeBcCacheDhcp4Offer ( IN PXEBC_PRIVATE_DATA *Private, IN EFI_DHCP4_PACKET *RcvdOffer @@ -787,6 +816,7 @@ PxeBcCacheDhcp4Offer ( PXEBC_DHCP4_PACKET_CACHE *Cache4; EFI_DHCP4_PACKET *Offer; PXEBC_OFFER_TYPE OfferType; + EFI_STATUS Status; ASSERT (Private->OfferNum < PXEBC_OFFER_MAX_NUM); Cache4 = &Private->OfferBuffer[Private->OfferNum].Dhcp4; @@ -795,13 +825,16 @@ PxeBcCacheDhcp4Offer ( // // Cache the content of DHCPv4 packet firstly. // - PxeBcCacheDhcp4Packet (Offer, RcvdOffer); + Status = PxeBcCacheDhcp4Packet (Offer, RcvdOffer); + if (EFI_ERROR(Status)) { + return Status; + } // // Validate the DHCPv4 packet, and parse the options and offer type. // if (EFI_ERROR (PxeBcParseDhcp4Packet (Cache4))) { - return; + return EFI_ABORTED; } // @@ -818,7 +851,7 @@ PxeBcCacheDhcp4Offer ( Private->OfferIndex[OfferType][0] = Private->OfferNum; Private->OfferCount[OfferType] = 1; } else { - return; + return EFI_ABORTED; } } else { ASSERT (Private->OfferCount[OfferType] < PXEBC_OFFER_MAX_NUM); @@ -841,7 +874,7 @@ PxeBcCacheDhcp4Offer ( Private->OfferIndex[OfferType][0] = Private->OfferNum; Private->OfferCount[OfferType] = 1; } else { - return ; + return EFI_ABORTED; } } else { // @@ -853,6 +886,8 @@ PxeBcCacheDhcp4Offer ( } Private->OfferNum++; + + return EFI_SUCCESS; } @@ -977,11 +1012,12 @@ PxeBcSelectDhcp4Offer ( /** Handle the DHCPv4 offer packet. - @param[in] Private Pointer to PxeBc private data. + @param[in] Private Pointer to PxeBc private data. - @retval EFI_SUCCESS Handled the DHCPv4 offer packet successfully. - @retval EFI_NO_RESPONSE No response to the following request packet. - @retval EFI_NOT_FOUND No boot filename received. + @retval EFI_SUCCESS Handled the DHCPv4 offer packet successfully. + @retval EFI_NO_RESPONSE No response to the following request packet. + @retval EFI_NOT_FOUND No boot filename received. + @retval EFI_BUFFER_TOO_SMALL Can't cache the offer pacet. **/ EFI_STATUS @@ -1086,7 +1122,7 @@ PxeBcHandleDhcp4Offer ( // // Success to try to request by a ProxyPxe10 or ProxyWfm11a offer, copy and parse it. // - PxeBcCopyProxyOffer (Private, ProxyIndex); + Status = PxeBcCopyProxyOffer (Private, ProxyIndex); } } else { // @@ -1113,7 +1149,10 @@ PxeBcHandleDhcp4Offer ( Ack = Offer; } - PxeBcCopyDhcp4Ack (Private, Ack, TRUE); + Status = PxeBcCopyDhcp4Ack (Private, Ack, TRUE); + if (EFI_ERROR (Status)) { + return Status; + } Mode->DhcpDiscoverValid = TRUE; } @@ -1255,6 +1294,7 @@ PxeBcDhcp4CallBack ( // // Cache the DHCPv4 offers to OfferBuffer[] for select later, and record // the OfferIndex and OfferCount. + // If error happens, just ignore this packet and continue to wait more offer. // PxeBcCacheDhcp4Offer (Private, Packet); } @@ -1275,21 +1315,16 @@ PxeBcDhcp4CallBack ( break; case Dhcp4RcvdAck: - if (Packet->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) { - // - // Abort the DHCP if the ACK packet exceeds the maximum length. - // - Status = EFI_ABORTED; - break; - } - // // Cache the DHCPv4 ack to Private->Dhcp4Ack, but it's not the final ack in mode data // without verification. // ASSERT (Private->SelectIndex != 0); - PxeBcCopyDhcp4Ack (Private, Packet, FALSE); + Status = PxeBcCopyDhcp4Ack (Private, Packet, FALSE); + if (EFI_ERROR (Status)) { + Status = EFI_ABORTED; + } break; default: @@ -1488,6 +1523,12 @@ PxeBcDhcp4Discover ( // Find the right PXE Reply according to server address. // while (RepIndex < Token.ResponseCount) { + if (Response->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) { + SrvIndex = 0; + RepIndex++; + Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size); + continue; + } while (SrvIndex < IpCount) { if (SrvList[SrvIndex].AcceptAnyResponse) { @@ -1506,7 +1547,6 @@ PxeBcDhcp4Discover ( SrvIndex = 0; RepIndex++; - Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size); } @@ -1516,10 +1556,16 @@ PxeBcDhcp4Discover ( // Especially for PXE discover packet, store it into mode data here. // if (Private->IsDoDiscover) { - PxeBcCacheDhcp4Packet (&Private->PxeReply.Dhcp4.Packet.Ack, Response); + Status = PxeBcCacheDhcp4Packet (&Private->PxeReply.Dhcp4.Packet.Ack, Response); + if (EFI_ERROR(Status)) { + goto ON_EXIT; + } CopyMem (&Mode->PxeDiscover, &Token.Packet->Dhcp4, Token.Packet->Length); } else { - PxeBcCacheDhcp4Packet (&Private->ProxyOffer.Dhcp4.Packet.Offer, Response); + Status = PxeBcCacheDhcp4Packet (&Private->ProxyOffer.Dhcp4.Packet.Offer, Response); + if (EFI_ERROR(Status)) { + goto ON_EXIT; + } } } else { // @@ -1527,12 +1573,15 @@ PxeBcDhcp4Discover ( // Status = EFI_NOT_FOUND; } - if (Token.ResponseList != NULL) { - FreePool (Token.ResponseList); - } } - - FreePool (Token.Packet); +ON_EXIT: + + if (Token.ResponseList != NULL) { + FreePool (Token.ResponseList); + } + if (Token.Packet != NULL) { + FreePool (Token.Packet); + } return Status; } diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c index daae9ad9c0..963a876e65 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c @@ -1,7 +1,7 @@ /** @file Functions implementation related with DHCPv6 for UefiPxeBc Driver. - Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -179,17 +179,24 @@ PxeBcBuildDhcp6Options ( @param[in] Dst The pointer to the cache buffer for DHCPv6 packet. @param[in] Src The pointer to the DHCPv6 packet to be cached. + @retval EFI_SUCCESS Packet is copied. + @retval EFI_BUFFER_TOO_SMALL Cache buffer is not big enough to hold the packet. + **/ -VOID +EFI_STATUS PxeBcCacheDhcp6Packet ( IN EFI_DHCP6_PACKET *Dst, IN EFI_DHCP6_PACKET *Src ) { - ASSERT (Dst->Size >= Src->Length); + if (Dst->Size < Src->Length) { + return EFI_BUFFER_TOO_SMALL; + } CopyMem (&Dst->Dhcp6, &Src->Dhcp6, Src->Length); Dst->Length = Src->Length; + + return EFI_SUCCESS; } @@ -544,8 +551,11 @@ PxeBcParseDhcp6Packet ( @param[in] Ack The pointer to the DHCPv6 ack packet. @param[in] Verified If TRUE, parse the ACK packet and store info into mode data. + @retval EFI_SUCCESS Cache and parse the packet successfully. + @retval EFI_BUFFER_TOO_SMALL Cache buffer is not big enough to hold the packet. + **/ -VOID +EFI_STATUS PxeBcCopyDhcp6Ack ( IN PXEBC_PRIVATE_DATA *Private, IN EFI_DHCP6_PACKET *Ack, @@ -553,10 +563,14 @@ PxeBcCopyDhcp6Ack ( ) { EFI_PXE_BASE_CODE_MODE *Mode; + EFI_STATUS Status; Mode = Private->PxeBc.Mode; - PxeBcCacheDhcp6Packet (&Private->DhcpAck.Dhcp6.Packet.Ack, Ack); + Status = PxeBcCacheDhcp6Packet (&Private->DhcpAck.Dhcp6.Packet.Ack, Ack); + if (EFI_ERROR (Status)) { + return Status; + } if (Verified) { // @@ -566,6 +580,8 @@ PxeBcCopyDhcp6Ack ( CopyMem (&Mode->DhcpAck.Dhcpv6, &Ack->Dhcp6, Ack->Length); Mode->DhcpAckReceived = TRUE; } + + return EFI_SUCCESS; } @@ -575,8 +591,11 @@ PxeBcCopyDhcp6Ack ( @param[in] Private The pointer to PxeBc private data. @param[in] OfferIndex The received order of offer packets. + @retval EFI_SUCCESS Cache and parse the packet successfully. + @retval EFI_BUFFER_TOO_SMALL Cache buffer is not big enough to hold the packet. + **/ -VOID +EFI_STATUS PxeBcCopyDhcp6Proxy ( IN PXEBC_PRIVATE_DATA *Private, IN UINT32 OfferIndex @@ -584,6 +603,7 @@ PxeBcCopyDhcp6Proxy ( { EFI_PXE_BASE_CODE_MODE *Mode; EFI_DHCP6_PACKET *Offer; + EFI_STATUS Status; ASSERT (OfferIndex < Private->OfferNum); ASSERT (OfferIndex < PXEBC_OFFER_MAX_NUM); @@ -594,7 +614,10 @@ PxeBcCopyDhcp6Proxy ( // // Cache the proxy offer packet and parse it. // - PxeBcCacheDhcp6Packet (&Private->ProxyOffer.Dhcp6.Packet.Offer, Offer); + Status = PxeBcCacheDhcp6Packet (&Private->ProxyOffer.Dhcp6.Packet.Offer, Offer); + if (EFI_ERROR(Status)) { + return Status; + } PxeBcParseDhcp6Packet (&Private->ProxyOffer.Dhcp6); // @@ -602,6 +625,8 @@ PxeBcCopyDhcp6Proxy ( // CopyMem (&Mode->ProxyOffer.Dhcpv6, &Offer->Dhcp6, Offer->Length); Mode->ProxyOfferReceived = TRUE; + + return EFI_SUCCESS; } /** @@ -917,8 +942,10 @@ PxeBcRetryDhcp6Binl ( @param[in] Private The pointer to PXEBC_PRIVATE_DATA. @param[in] RcvdOffer The pointer to the received offer packet. + @retval EFI_SUCCESS Cache and parse the packet successfully. + @retval Others Operation failed. **/ -VOID +EFI_STATUS PxeBcCacheDhcp6Offer ( IN PXEBC_PRIVATE_DATA *Private, IN EFI_DHCP6_PACKET *RcvdOffer @@ -927,6 +954,7 @@ PxeBcCacheDhcp6Offer ( PXEBC_DHCP6_PACKET_CACHE *Cache6; EFI_DHCP6_PACKET *Offer; PXEBC_OFFER_TYPE OfferType; + EFI_STATUS Status; Cache6 = &Private->OfferBuffer[Private->OfferNum].Dhcp6; Offer = &Cache6->Packet.Offer; @@ -934,13 +962,16 @@ PxeBcCacheDhcp6Offer ( // // Cache the content of DHCPv6 packet firstly. // - PxeBcCacheDhcp6Packet (Offer, RcvdOffer); + Status = PxeBcCacheDhcp6Packet (Offer, RcvdOffer); + if (EFI_ERROR (Status)) { + return Status; + } // // Validate the DHCPv6 packet, and parse the options and offer type. // if (EFI_ERROR (PxeBcParseDhcp6Packet (Cache6))) { - return ; + return EFI_ABORTED; } // @@ -969,7 +1000,7 @@ PxeBcCacheDhcp6Offer ( Private->OfferIndex[OfferType][0] = Private->OfferNum; Private->OfferCount[OfferType] = 1; } else { - return; + return EFI_ABORTED; } } else { // @@ -980,6 +1011,8 @@ PxeBcCacheDhcp6Offer ( } Private->OfferNum++; + + return EFI_SUCCESS; } @@ -1094,8 +1127,10 @@ PxeBcSelectDhcp6Offer ( @param[in] Private The pointer to PXEBC_PRIVATE_DATA. - @retval EFI_SUCCESS Handled the DHCPv6 offer packet successfully. - @retval EFI_NO_RESPONSE No response to the following request packet. + @retval EFI_SUCCESS Handled the DHCPv6 offer packet successfully. + @retval EFI_NO_RESPONSE No response to the following request packet. + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources. + @retval EFI_BUFFER_TOO_SMALL Can't cache the offer pacet. **/ EFI_STATUS @@ -1194,7 +1229,7 @@ PxeBcHandleDhcp6Offer ( // // Success to try to request by a ProxyPxe10 or ProxyWfm11a offer, copy and parse it. // - PxeBcCopyDhcp6Proxy (Private, ProxyIndex); + Status = PxeBcCopyDhcp6Proxy (Private, ProxyIndex); } } else { // @@ -1208,7 +1243,7 @@ PxeBcHandleDhcp6Offer ( // // All PXE boot information is ready by now. // - PxeBcCopyDhcp6Ack (Private, &Private->DhcpAck.Dhcp6.Packet.Ack, TRUE); + Status = PxeBcCopyDhcp6Ack (Private, &Private->DhcpAck.Dhcp6.Packet.Ack, TRUE); Private->PxeBc.Mode->DhcpDiscoverValid = TRUE; } @@ -1547,19 +1582,15 @@ PxeBcDhcp6CallBack ( break; case Dhcp6RcvdReply: - if (Packet->Length > PXEBC_DHCP6_PACKET_MAX_SIZE) { - // - // Abort the DHCP if the Peply packet exceeds the maximum length. - // - Status = EFI_ABORTED; - break; - } // // Cache the dhcp ack to Private->Dhcp6Ack, but it's not the final ack in mode data // without verification. // ASSERT (Private->SelectIndex != 0); - PxeBcCopyDhcp6Ack (Private, Packet, FALSE); + Status = PxeBcCopyDhcp6Ack (Private, Packet, FALSE); + if (EFI_ERROR (Status)) { + Status = EFI_ABORTED; + } break; default: -- cgit