* [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot.
@ 2016-12-16 8:18 Fu Siyuan
2016-12-16 8:18 ` [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver Fu Siyuan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Fu Siyuan @ 2016-12-16 8:18 UTC (permalink / raw)
To: edk2-devel
This patch remove the ASSERT when receive a DHCP packet large than the maximum
cache buffer size.
Fu Siyuan (2):
MdeModulePkg: Replace ASSERT with error return code in PXE driver.
NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot
driver.
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c | 99 +++++++++++------
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h | 5 +-
NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 32 +++++-
NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 29 +++--
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 119 +++++++++++++++------
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 67 ++++++++----
6 files changed, 249 insertions(+), 102 deletions(-)
--
2.7.4.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver.
2016-12-16 8:18 [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Fu Siyuan
@ 2016-12-16 8:18 ` Fu Siyuan
2016-12-16 8:59 ` Wu, Jiaxin
2016-12-16 8:18 ` [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver Fu Siyuan
2016-12-16 8:45 ` [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Ye, Ting
2 siblings, 1 reply; 6+ messages in thread
From: Fu Siyuan @ 2016-12-16 8:18 UTC (permalink / raw)
To: edk2-devel; +Cc: Yao Jiewen, Ye Ting, Wu Jiaxin
This patch remove the ASSERT when receive a DHCP packet large than the maximum
cache buffer size.
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c | 99 ++++++++++++++--------
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h | 5 +-
2 files changed, 68 insertions(+), 36 deletions(-)
diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
index f03176b..f0720e5 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
@@ -69,20 +69,26 @@ PxeBcInitSeedPacket (
/**
Copy the DCHP4 packet from srouce to destination.
- @param Dst Pointer to the EFI_DHCP4_PROTOCOL instance.
- @param Src Pointer to the EFI_DHCP4_PROTOCOL instance.
+ @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
PxeBcCopyEfiDhcp4Packet (
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;
}
@@ -93,8 +99,11 @@ PxeBcCopyEfiDhcp4Packet (
@param OfferIndex Index of cached packets as complements of pxe mode data,
the index is maximum offer number.
+ @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
@@ -102,6 +111,7 @@ PxeBcCopyProxyOffer (
{
EFI_PXE_BASE_CODE_MODE *Mode;
EFI_DHCP4_PACKET *Offer;
+ EFI_STATUS Status;
ASSERT (OfferIndex < Private->NumOffers);
ASSERT (OfferIndex < PXEBC_MAX_OFFER_NUM);
@@ -109,11 +119,15 @@ PxeBcCopyProxyOffer (
Mode = Private->PxeBc.Mode;
Offer = &Private->Dhcp4Offers[OfferIndex].Packet.Offer;
- PxeBcCopyEfiDhcp4Packet (&Private->ProxyOffer.Packet.Offer, Offer);
+ Status = PxeBcCopyEfiDhcp4Packet (&Private->ProxyOffer.Packet.Offer, Offer);
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
CopyMem (&Mode->ProxyOffer, &Offer->Dhcp4, Offer->Length);
Mode->ProxyOfferReceived = TRUE;
PxeBcParseCachedDhcpPacket (&Private->ProxyOffer);
+ return EFI_SUCCESS;
}
@@ -411,8 +425,9 @@ PxeBcTryBinlProxy (
@param Private Pointer to PxeBc private data.
- @retval EFI_SUCCESS Operational successful.
- @retval EFI_NO_RESPONSE Offer dhcp service failed.
+ @retval EFI_SUCCESS Operational successful.
+ @retval EFI_NO_RESPONSE Offer dhcp service failed.
+ @retval EFI_BUFFER_TOO_SMALL Failed to copy the packet to Pxe base code mode.
**/
EFI_STATUS
@@ -515,7 +530,7 @@ PxeBcCheckSelectedOffer (
//
// Copy the proxy offer to Mode and set the flag
//
- PxeBcCopyProxyOffer (Private, ProxyOfferIndex);
+ Status = PxeBcCopyProxyOffer (Private, ProxyOfferIndex);
}
} else {
//
@@ -543,7 +558,7 @@ PxeBcCheckSelectedOffer (
// Other type of ACK is already cached. Bootp is special that we should
// use the bootp reply as the ACK and put it into the DHCP_ONLY buffer.
//
- PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Offer);
+ Status = PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Offer);
}
PxeBcParseCachedDhcpPacket (&Private->Dhcp4Ack);
@@ -566,8 +581,11 @@ PxeBcCheckSelectedOffer (
@param Private Pointer to PxeBc private data.
@param RcvdOffer Pointer to the received Dhcp proxy offer packet.
+ @retval EFI_SUCCESS Cache and parse the packet successfully.
+ @retval Others Operation failed.
+
**/
-VOID
+EFI_STATUS
PxeBcCacheDhcpOffer (
IN PXEBC_PRIVATE_DATA *Private,
IN EFI_DHCP4_PACKET *RcvdOffer
@@ -576,6 +594,7 @@ PxeBcCacheDhcpOffer (
PXEBC_CACHED_DHCP4_PACKET *CachedOffer;
EFI_DHCP4_PACKET *Offer;
UINT8 OfferType;
+ EFI_STATUS Status;
CachedOffer = &Private->Dhcp4Offers[Private->NumOffers];
Offer = &CachedOffer->Packet.Offer;
@@ -583,18 +602,21 @@ PxeBcCacheDhcpOffer (
//
// Cache the orignal dhcp packet
//
- PxeBcCopyEfiDhcp4Packet (Offer, RcvdOffer);
+ Status = PxeBcCopyEfiDhcp4Packet (Offer, RcvdOffer);
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
//
// Parse and validate the options (including dhcp option and vendor option)
//
if (!PxeBcParseCachedDhcpPacket (CachedOffer)) {
- return ;
+ return EFI_ABORTED;
}
OfferType = CachedOffer->OfferType;
if (OfferType >= DHCP4_PACKET_TYPE_MAX) {
- return ;
+ return EFI_ABORTED;
}
if (OfferType == DHCP4_PACKET_TYPE_BOOTP) {
@@ -603,7 +625,7 @@ PxeBcCacheDhcpOffer (
//
// Only cache the first bootp offer, discard others.
//
- return ;
+ return EFI_ABORTED;
} else {
//
// Take as a dhcp only offer, but record index specifically.
@@ -628,7 +650,7 @@ PxeBcCacheDhcpOffer (
//
// Only cache the first pxe10/wfm11a offers each, discard the others.
//
- return ;
+ return EFI_ABORTED;
} else {
//
// Record index of the proxy dhcp offer with type other than binl.
@@ -649,6 +671,8 @@ PxeBcCacheDhcpOffer (
// Count the accepted offers.
//
Private->NumOffers++;
+
+ return EFI_SUCCESS;
}
/**
@@ -960,6 +984,7 @@ PxeBcDhcpCallBack (
if (Private->NumOffers < PXEBC_MAX_OFFER_NUM) {
//
// Cache the dhcp offers in Private->Dhcp4Offers[]
+ // If error happens, just ignore this packet and continue to wait more offer.
//
PxeBcCacheDhcpOffer (Private, Packet);
}
@@ -982,20 +1007,15 @@ PxeBcDhcpCallBack (
break;
case Dhcp4RcvdAck:
- if (Packet->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {
- //
- // Abort the DHCP if the ACK packet exceeds the maximum length.
- //
- Status = EFI_ABORTED;
- break;
- }
-
//
// Cache Ack
//
ASSERT (Private->SelectedOffer != 0);
- PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Packet);
+ Status = PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Packet);
+ if (EFI_ERROR (Status)) {
+ return EFI_ABORTED;
+ }
break;
default:
@@ -1340,6 +1360,12 @@ PxeBcDiscvBootService (
Response = Token.ResponseList;
while (RepIndex < Token.ResponseCount) {
+ if (Response->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {
+ SrvIndex = 0;
+ RepIndex++;
+ Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size);
+ continue;
+ }
while (SrvIndex < IpCount) {
@@ -1360,14 +1386,16 @@ PxeBcDiscvBootService (
SrvIndex = 0;
RepIndex++;
-
Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size);
}
if (RepIndex < Token.ResponseCount) {
if (Reply != NULL) {
- PxeBcCopyEfiDhcp4Packet (Reply, Response);
+ Status = PxeBcCopyEfiDhcp4Packet (Reply, Response);
+ if (EFI_ERROR(Status)) {
+ goto ON_EXIT;
+ }
}
if (IsDiscv) {
@@ -1380,18 +1408,21 @@ PxeBcDiscvBootService (
} else {
Status = EFI_NOT_FOUND;
}
+ }
- //
- // free the responselist
- //
- if (Token.ResponseList != NULL) {
- FreePool (Token.ResponseList);
- }
+ON_EXIT:
+ //
+ // free the responselist
+ //
+ if (Token.ResponseList != NULL) {
+ FreePool (Token.ResponseList);
}
//
// Free the dhcp packet
//
- FreePool (Token.Packet);
+ if (Token.Packet != NULL) {
+ FreePool (Token.Packet);
+ }
return Status;
}
diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
index 614ea75..d19d2a3 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
@@ -291,8 +291,9 @@ PxeBcParseCachedDhcpPacket (
@param Private Pointer to PxeBc private data.
- @retval EFI_SUCCESS Operational successful.
- @retval EFI_NO_RESPONSE Offer dhcp service failed.
+ @retval EFI_SUCCESS Operational successful.
+ @retval EFI_NO_RESPONSE Offer dhcp service failed.
+ @retval EFI_BUFFER_TOO_SMALL Failed to copy the packet to Pxe base code mode.
**/
EFI_STATUS
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver.
2016-12-16 8:18 [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Fu Siyuan
2016-12-16 8:18 ` [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver Fu Siyuan
@ 2016-12-16 8:18 ` Fu Siyuan
2016-12-16 8:57 ` Wu, Jiaxin
2016-12-16 8:45 ` [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Ye, Ting
2 siblings, 1 reply; 6+ messages in thread
From: Fu Siyuan @ 2016-12-16 8:18 UTC (permalink / raw)
To: edk2-devel; +Cc: Yao Jiewen, Ye Ting, Wu Jiaxin
This patch remove the ASSERT when receive a DHCP packet large than the maximum
cache buffer size.
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 32 +++++++--
NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 29 ++++++--
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 119 +++++++++++++++++++++++----------
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 67 +++++++++++++------
4 files changed, 181 insertions(+), 66 deletions(-)
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c b/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
index a47a8f4..fcea916 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
@@ -220,17 +220,24 @@ HttpBootParseDhcp4Options (
@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
HttpBootCacheDhcp4Packet (
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;
}
/**
@@ -429,8 +436,10 @@ HttpBootParseDhcp4Packet (
@param[in] Private Pointer to HTTP boot driver 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
HttpBootCacheDhcp4Offer (
IN HTTP_BOOT_PRIVATE_DATA *Private,
IN EFI_DHCP4_PACKET *RcvdOffer
@@ -439,6 +448,7 @@ HttpBootCacheDhcp4Offer (
HTTP_BOOT_DHCP4_PACKET_CACHE *Cache4;
EFI_DHCP4_PACKET *Offer;
HTTP_BOOT_OFFER_TYPE OfferType;
+ EFI_STATUS Status;
ASSERT (Private->OfferNum < HTTP_BOOT_OFFER_MAX_NUM);
Cache4 = &Private->OfferBuffer[Private->OfferNum].Dhcp4;
@@ -447,13 +457,16 @@ HttpBootCacheDhcp4Offer (
//
// Cache the content of DHCPv4 packet firstly.
//
- HttpBootCacheDhcp4Packet (Offer, RcvdOffer);
+ Status = HttpBootCacheDhcp4Packet (Offer, RcvdOffer);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
//
// Validate the DHCPv4 packet, and parse the options and offer type.
//
if (EFI_ERROR (HttpBootParseDhcp4Packet (Cache4))) {
- return;
+ return EFI_ABORTED;
}
//
@@ -465,6 +478,8 @@ HttpBootCacheDhcp4Offer (
Private->OfferIndex[OfferType][Private->OfferCount[OfferType]] = Private->OfferNum;
Private->OfferCount[OfferType]++;
Private->OfferNum++;
+
+ return EFI_SUCCESS;
}
/**
@@ -618,10 +633,17 @@ HttpBootDhcp4CallBack (
switch (Dhcp4Event) {
case Dhcp4RcvdOffer:
Status = EFI_NOT_READY;
+ if (Packet->Length > HTTP_BOOT_DHCP4_PACKET_MAX_SIZE) {
+ //
+ // Ignore the incoming packets which exceed the maximum length.
+ //
+ break;
+ }
if (Private->OfferNum < HTTP_BOOT_OFFER_MAX_NUM) {
//
// 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.
//
HttpBootCacheDhcp4Offer (Private, Packet);
}
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
index ca84f2a..f2b8195 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
@@ -329,17 +329,24 @@ HttpBootParseDhcp6Packet (
@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
HttpBootCacheDhcp6Packet (
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;
}
/**
@@ -348,8 +355,11 @@ HttpBootCacheDhcp6Packet (
@param[in] Private The pointer to HTTP_BOOT_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
HttpBootCacheDhcp6Offer (
IN HTTP_BOOT_PRIVATE_DATA *Private,
IN EFI_DHCP6_PACKET *RcvdOffer
@@ -358,6 +368,7 @@ HttpBootCacheDhcp6Offer (
HTTP_BOOT_DHCP6_PACKET_CACHE *Cache6;
EFI_DHCP6_PACKET *Offer;
HTTP_BOOT_OFFER_TYPE OfferType;
+ EFI_STATUS Status;
Cache6 = &Private->OfferBuffer[Private->OfferNum].Dhcp6;
Offer = &Cache6->Packet.Offer;
@@ -365,13 +376,16 @@ HttpBootCacheDhcp6Offer (
//
// Cache the content of DHCPv6 packet firstly.
//
- HttpBootCacheDhcp6Packet(Offer, RcvdOffer);
+ Status = HttpBootCacheDhcp6Packet(Offer, RcvdOffer);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
//
// Validate the DHCPv6 packet, and parse the options and offer type.
//
if (EFI_ERROR (HttpBootParseDhcp6Packet (Cache6))) {
- return ;
+ return EFI_ABORTED;
}
//
@@ -382,7 +396,9 @@ HttpBootCacheDhcp6Offer (
ASSERT (Private->OfferCount[OfferType] < HTTP_BOOT_OFFER_MAX_NUM);
Private->OfferIndex[OfferType][Private->OfferCount[OfferType]] = Private->OfferNum;
Private->OfferCount[OfferType]++;
- Private->OfferNum++;
+ Private->OfferNum++;
+
+ return EFI_SUCCESS;
}
/**
@@ -437,6 +453,7 @@ HttpBootDhcp6CallBack (
//
// Cache the dhcp 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.
//
HttpBootCacheDhcp6Offer (Private, Packet);
}
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
index 44b0714..5497390 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;
}
@@ -620,8 +627,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,
@@ -629,10 +639,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) {
//
@@ -642,6 +656,8 @@ PxeBcCopyDhcp4Ack (
CopyMem (&Mode->DhcpAck.Dhcpv4, &Ack->Dhcp4, Ack->Length);
Mode->DhcpAckReceived = TRUE;
}
+
+ return EFI_SUCCESS;
}
@@ -651,8 +667,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
@@ -660,6 +679,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);
@@ -670,7 +690,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);
//
@@ -678,6 +702,8 @@ PxeBcCopyProxyOffer (
//
CopyMem (&Mode->ProxyOffer.Dhcpv4, &Offer->Dhcp4, Offer->Length);
Mode->ProxyOfferReceived = TRUE;
+
+ return EFI_SUCCESS;
}
@@ -780,8 +806,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
@@ -790,6 +819,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;
@@ -798,13 +828,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;
}
//
@@ -821,7 +854,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);
@@ -844,7 +877,7 @@ PxeBcCacheDhcp4Offer (
Private->OfferIndex[OfferType][0] = Private->OfferNum;
Private->OfferCount[OfferType] = 1;
} else {
- return ;
+ return EFI_ABORTED;
}
} else {
//
@@ -856,6 +889,8 @@ PxeBcCacheDhcp4Offer (
}
Private->OfferNum++;
+
+ return EFI_SUCCESS;
}
@@ -980,11 +1015,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
@@ -1089,7 +1125,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 {
//
@@ -1116,7 +1152,10 @@ PxeBcHandleDhcp4Offer (
Ack = Offer;
}
- PxeBcCopyDhcp4Ack (Private, Ack, TRUE);
+ Status = PxeBcCopyDhcp4Ack (Private, Ack, TRUE);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
Mode->DhcpDiscoverValid = TRUE;
}
@@ -1258,6 +1297,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);
}
@@ -1278,21 +1318,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:
@@ -1491,6 +1526,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) {
@@ -1509,7 +1550,6 @@ PxeBcDhcp4Discover (
SrvIndex = 0;
RepIndex++;
-
Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response->Size);
}
@@ -1519,10 +1559,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 {
//
@@ -1530,12 +1576,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 6a08e9a..3672f13 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
@@ -181,17 +181,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;
}
@@ -749,8 +756,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,
@@ -758,10 +768,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) {
//
@@ -771,6 +785,8 @@ PxeBcCopyDhcp6Ack (
CopyMem (&Mode->DhcpAck.Dhcpv6, &Ack->Dhcp6, Ack->Length);
Mode->DhcpAckReceived = TRUE;
}
+
+ return EFI_SUCCESS;
}
@@ -780,8 +796,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
@@ -789,6 +808,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);
@@ -799,7 +819,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);
//
@@ -807,6 +830,8 @@ PxeBcCopyDhcp6Proxy (
//
CopyMem (&Mode->ProxyOffer.Dhcpv6, &Offer->Dhcp6, Offer->Length);
Mode->ProxyOfferReceived = TRUE;
+
+ return EFI_SUCCESS;
}
/**
@@ -1121,8 +1146,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
@@ -1131,6 +1158,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;
@@ -1138,13 +1166,13 @@ PxeBcCacheDhcp6Offer (
//
// Cache the content of DHCPv6 packet firstly.
//
- PxeBcCacheDhcp6Packet (Offer, RcvdOffer);
+ Status = PxeBcCacheDhcp6Packet (Offer, RcvdOffer);
//
// Validate the DHCPv6 packet, and parse the options and offer type.
//
if (EFI_ERROR (PxeBcParseDhcp6Packet (Cache6))) {
- return ;
+ return EFI_ABORTED;
}
//
@@ -1173,7 +1201,7 @@ PxeBcCacheDhcp6Offer (
Private->OfferIndex[OfferType][0] = Private->OfferNum;
Private->OfferCount[OfferType] = 1;
} else {
- return;
+ return EFI_ABORTED;
}
} else {
//
@@ -1184,6 +1212,8 @@ PxeBcCacheDhcp6Offer (
}
Private->OfferNum++;
+
+ return EFI_SUCCESS;
}
@@ -1301,6 +1331,7 @@ PxeBcSelectDhcp6Offer (
@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
@@ -1410,7 +1441,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 {
//
@@ -1424,7 +1455,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;
}
@@ -1997,19 +2028,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:
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot.
2016-12-16 8:18 [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Fu Siyuan
2016-12-16 8:18 ` [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver Fu Siyuan
2016-12-16 8:18 ` [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver Fu Siyuan
@ 2016-12-16 8:45 ` Ye, Ting
2 siblings, 0 replies; 6+ messages in thread
From: Ye, Ting @ 2016-12-16 8:45 UTC (permalink / raw)
To: Fu, Siyuan, edk2-devel@lists.01.org
Looks good to me.
Series Reviewed-by: Ye Ting <ting.ye@intel.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu Siyuan
Sent: Friday, December 16, 2016 4:19 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot.
This patch remove the ASSERT when receive a DHCP packet large than the maximum cache buffer size.
Fu Siyuan (2):
MdeModulePkg: Replace ASSERT with error return code in PXE driver.
NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot
driver.
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c | 99 +++++++++++------
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h | 5 +-
NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 32 +++++-
NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 29 +++--
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 119 +++++++++++++++------
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 67 ++++++++----
6 files changed, 249 insertions(+), 102 deletions(-)
--
2.7.4.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver.
2016-12-16 8:18 ` [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver Fu Siyuan
@ 2016-12-16 8:57 ` Wu, Jiaxin
0 siblings, 0 replies; 6+ messages in thread
From: Wu, Jiaxin @ 2016-12-16 8:57 UTC (permalink / raw)
To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, Ye, Ting
Hi siyuan,
In PxeBcCacheDhcp6Offer() function, 'Status' is set but not used, this may break the GCC build. Please double check that.
Others good to me.
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Thanks,
Jiaxin
> -----Original Message-----
> From: Fu, Siyuan
> Sent: Friday, December 16, 2016 4:19 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in
> PXE and HTTP boot driver.
>
> This patch remove the ASSERT when receive a DHCP packet large than the
> maximum cache buffer size.
>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
> NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 32 +++++++--
> NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 29 ++++++--
> NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 119
> +++++++++++++++++++++++----------
> NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 67 +++++++++++++------
> 4 files changed, 181 insertions(+), 66 deletions(-)
>
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
> b/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
> index a47a8f4..fcea916 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp4.c
> @@ -220,17 +220,24 @@ HttpBootParseDhcp4Options (
> @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
> HttpBootCacheDhcp4Packet (
> 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;
> }
>
> /**
> @@ -429,8 +436,10 @@ HttpBootParseDhcp4Packet (
> @param[in] Private Pointer to HTTP boot driver 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
> HttpBootCacheDhcp4Offer (
> IN HTTP_BOOT_PRIVATE_DATA *Private,
> IN EFI_DHCP4_PACKET *RcvdOffer
> @@ -439,6 +448,7 @@ HttpBootCacheDhcp4Offer (
> HTTP_BOOT_DHCP4_PACKET_CACHE *Cache4;
> EFI_DHCP4_PACKET *Offer;
> HTTP_BOOT_OFFER_TYPE OfferType;
> + EFI_STATUS Status;
>
> ASSERT (Private->OfferNum < HTTP_BOOT_OFFER_MAX_NUM);
> Cache4 = &Private->OfferBuffer[Private->OfferNum].Dhcp4;
> @@ -447,13 +457,16 @@ HttpBootCacheDhcp4Offer (
> //
> // Cache the content of DHCPv4 packet firstly.
> //
> - HttpBootCacheDhcp4Packet (Offer, RcvdOffer);
> + Status = HttpBootCacheDhcp4Packet (Offer, RcvdOffer); if (EFI_ERROR
> + (Status)) {
> + return Status;
> + }
>
> //
> // Validate the DHCPv4 packet, and parse the options and offer type.
> //
> if (EFI_ERROR (HttpBootParseDhcp4Packet (Cache4))) {
> - return;
> + return EFI_ABORTED;
> }
>
> //
> @@ -465,6 +478,8 @@ HttpBootCacheDhcp4Offer (
> Private->OfferIndex[OfferType][Private->OfferCount[OfferType]] =
> Private->OfferNum;
> Private->OfferCount[OfferType]++;
> Private->OfferNum++;
> +
> + return EFI_SUCCESS;
> }
>
> /**
> @@ -618,10 +633,17 @@ HttpBootDhcp4CallBack (
> switch (Dhcp4Event) {
> case Dhcp4RcvdOffer:
> Status = EFI_NOT_READY;
> + if (Packet->Length > HTTP_BOOT_DHCP4_PACKET_MAX_SIZE) {
> + //
> + // Ignore the incoming packets which exceed the maximum length.
> + //
> + break;
> + }
> if (Private->OfferNum < HTTP_BOOT_OFFER_MAX_NUM) {
> //
> // 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.
> //
> HttpBootCacheDhcp4Offer (Private, Packet);
> }
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
> b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
> index ca84f2a..f2b8195 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
> @@ -329,17 +329,24 @@ HttpBootParseDhcp6Packet (
> @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
> HttpBootCacheDhcp6Packet (
> 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;
> }
>
> /**
> @@ -348,8 +355,11 @@ HttpBootCacheDhcp6Packet (
> @param[in] Private The pointer to HTTP_BOOT_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
> HttpBootCacheDhcp6Offer (
> IN HTTP_BOOT_PRIVATE_DATA *Private,
> IN EFI_DHCP6_PACKET *RcvdOffer
> @@ -358,6 +368,7 @@ HttpBootCacheDhcp6Offer (
> HTTP_BOOT_DHCP6_PACKET_CACHE *Cache6;
> EFI_DHCP6_PACKET *Offer;
> HTTP_BOOT_OFFER_TYPE OfferType;
> + EFI_STATUS Status;
>
> Cache6 = &Private->OfferBuffer[Private->OfferNum].Dhcp6;
> Offer = &Cache6->Packet.Offer;
> @@ -365,13 +376,16 @@ HttpBootCacheDhcp6Offer (
> //
> // Cache the content of DHCPv6 packet firstly.
> //
> - HttpBootCacheDhcp6Packet(Offer, RcvdOffer);
> + Status = HttpBootCacheDhcp6Packet(Offer, RcvdOffer); if (EFI_ERROR
> + (Status)) {
> + return Status;
> + }
>
> //
> // Validate the DHCPv6 packet, and parse the options and offer type.
> //
> if (EFI_ERROR (HttpBootParseDhcp6Packet (Cache6))) {
> - return ;
> + return EFI_ABORTED;
> }
>
> //
> @@ -382,7 +396,9 @@ HttpBootCacheDhcp6Offer (
> ASSERT (Private->OfferCount[OfferType] <
> HTTP_BOOT_OFFER_MAX_NUM);
> Private->OfferIndex[OfferType][Private->OfferCount[OfferType]] =
> Private->OfferNum;
> Private->OfferCount[OfferType]++;
> - Private->OfferNum++;
> + Private->OfferNum++;
> +
> + return EFI_SUCCESS;
> }
>
> /**
> @@ -437,6 +453,7 @@ HttpBootDhcp6CallBack (
> //
> // Cache the dhcp 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.
> //
> HttpBootCacheDhcp6Offer (Private, Packet);
> }
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
> index 44b0714..5497390 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;
> }
>
>
> @@ -620,8 +627,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,
> @@ -629,10 +639,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) {
> //
> @@ -642,6 +656,8 @@ PxeBcCopyDhcp4Ack (
> CopyMem (&Mode->DhcpAck.Dhcpv4, &Ack->Dhcp4, Ack->Length);
> Mode->DhcpAckReceived = TRUE;
> }
> +
> + return EFI_SUCCESS;
> }
>
>
> @@ -651,8 +667,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
> @@ -660,6 +679,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); @@ -670,7 +690,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);
>
> //
> @@ -678,6 +702,8 @@ PxeBcCopyProxyOffer (
> //
> CopyMem (&Mode->ProxyOffer.Dhcpv4, &Offer->Dhcp4, Offer->Length);
> Mode->ProxyOfferReceived = TRUE;
> +
> + return EFI_SUCCESS;
> }
>
>
> @@ -780,8 +806,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
> @@ -790,6 +819,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;
> @@ -798,13 +828,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;
> }
>
> //
> @@ -821,7 +854,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);
> @@ -844,7 +877,7 @@ PxeBcCacheDhcp4Offer (
> Private->OfferIndex[OfferType][0] = Private->OfferNum;
> Private->OfferCount[OfferType] = 1;
> } else {
> - return ;
> + return EFI_ABORTED;
> }
> } else {
> //
> @@ -856,6 +889,8 @@ PxeBcCacheDhcp4Offer (
> }
>
> Private->OfferNum++;
> +
> + return EFI_SUCCESS;
> }
>
>
> @@ -980,11 +1015,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
> @@ -1089,7 +1125,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 {
> //
> @@ -1116,7 +1152,10 @@ PxeBcHandleDhcp4Offer (
> Ack = Offer;
> }
>
> - PxeBcCopyDhcp4Ack (Private, Ack, TRUE);
> + Status = PxeBcCopyDhcp4Ack (Private, Ack, TRUE);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> Mode->DhcpDiscoverValid = TRUE;
> }
>
> @@ -1258,6 +1297,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);
> }
> @@ -1278,21 +1318,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:
> @@ -1491,6 +1526,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) { @@ -1509,7 +1550,6 @@
> PxeBcDhcp4Discover (
>
> SrvIndex = 0;
> RepIndex++;
> -
> Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response-
> >Size);
> }
>
> @@ -1519,10 +1559,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 {
> //
> @@ -1530,12 +1576,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 6a08e9a..3672f13 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
> @@ -181,17 +181,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;
> }
>
>
> @@ -749,8 +756,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,
> @@ -758,10 +768,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) {
> //
> @@ -771,6 +785,8 @@ PxeBcCopyDhcp6Ack (
> CopyMem (&Mode->DhcpAck.Dhcpv6, &Ack->Dhcp6, Ack->Length);
> Mode->DhcpAckReceived = TRUE;
> }
> +
> + return EFI_SUCCESS;
> }
>
>
> @@ -780,8 +796,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
> @@ -789,6 +808,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); @@ -799,7 +819,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);
>
> //
> @@ -807,6 +830,8 @@ PxeBcCopyDhcp6Proxy (
> //
> CopyMem (&Mode->ProxyOffer.Dhcpv6, &Offer->Dhcp6, Offer->Length);
> Mode->ProxyOfferReceived = TRUE;
> +
> + return EFI_SUCCESS;
> }
>
> /**
> @@ -1121,8 +1146,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
> @@ -1131,6 +1158,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;
> @@ -1138,13 +1166,13 @@ PxeBcCacheDhcp6Offer (
> //
> // Cache the content of DHCPv6 packet firstly.
> //
> - PxeBcCacheDhcp6Packet (Offer, RcvdOffer);
> + Status = PxeBcCacheDhcp6Packet (Offer, RcvdOffer);
>
> //
> // Validate the DHCPv6 packet, and parse the options and offer type.
> //
> if (EFI_ERROR (PxeBcParseDhcp6Packet (Cache6))) {
> - return ;
> + return EFI_ABORTED;
> }
>
> //
> @@ -1173,7 +1201,7 @@ PxeBcCacheDhcp6Offer (
> Private->OfferIndex[OfferType][0] = Private->OfferNum;
> Private->OfferCount[OfferType] = 1;
> } else {
> - return;
> + return EFI_ABORTED;
> }
> } else {
> //
> @@ -1184,6 +1212,8 @@ PxeBcCacheDhcp6Offer (
> }
>
> Private->OfferNum++;
> +
> + return EFI_SUCCESS;
> }
>
>
> @@ -1301,6 +1331,7 @@ PxeBcSelectDhcp6Offer (
> @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
> @@ -1410,7 +1441,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 {
> //
> @@ -1424,7 +1455,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;
> }
>
> @@ -1997,19 +2028,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:
> --
> 2.7.4.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver.
2016-12-16 8:18 ` [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver Fu Siyuan
@ 2016-12-16 8:59 ` Wu, Jiaxin
0 siblings, 0 replies; 6+ messages in thread
From: Wu, Jiaxin @ 2016-12-16 8:59 UTC (permalink / raw)
To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, Ye, Ting
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Thanks,
Jiaxin
> -----Original Message-----
> From: Fu, Siyuan
> Sent: Friday, December 16, 2016 4:19 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 1/2] 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.
>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
> .../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c | 99
> ++++++++++++++--------
> .../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h | 5 +-
> 2 files changed, 68 insertions(+), 36 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
> b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
> index f03176b..f0720e5 100644
> --- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
> +++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
> @@ -69,20 +69,26 @@ PxeBcInitSeedPacket (
> /**
> Copy the DCHP4 packet from srouce to destination.
>
> - @param Dst Pointer to the EFI_DHCP4_PROTOCOL instance.
> - @param Src Pointer to the EFI_DHCP4_PROTOCOL instance.
> + @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
> PxeBcCopyEfiDhcp4Packet (
> 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;
> }
>
>
> @@ -93,8 +99,11 @@ PxeBcCopyEfiDhcp4Packet (
> @param OfferIndex Index of cached packets as complements of pxe
> mode data,
> the index is maximum offer number.
>
> + @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
> @@ -102,6 +111,7 @@ PxeBcCopyProxyOffer ( {
> EFI_PXE_BASE_CODE_MODE *Mode;
> EFI_DHCP4_PACKET *Offer;
> + EFI_STATUS Status;
>
> ASSERT (OfferIndex < Private->NumOffers);
> ASSERT (OfferIndex < PXEBC_MAX_OFFER_NUM); @@ -109,11 +119,15
> @@ PxeBcCopyProxyOffer (
> Mode = Private->PxeBc.Mode;
> Offer = &Private->Dhcp4Offers[OfferIndex].Packet.Offer;
>
> - PxeBcCopyEfiDhcp4Packet (&Private->ProxyOffer.Packet.Offer, Offer);
> + Status = PxeBcCopyEfiDhcp4Packet (&Private->ProxyOffer.Packet.Offer,
> + Offer); if (EFI_ERROR(Status)) {
> + return Status;
> + }
> CopyMem (&Mode->ProxyOffer, &Offer->Dhcp4, Offer->Length);
> Mode->ProxyOfferReceived = TRUE;
>
> PxeBcParseCachedDhcpPacket (&Private->ProxyOffer);
> + return EFI_SUCCESS;
> }
>
>
> @@ -411,8 +425,9 @@ PxeBcTryBinlProxy (
>
> @param Private Pointer to PxeBc private data.
>
> - @retval EFI_SUCCESS Operational successful.
> - @retval EFI_NO_RESPONSE Offer dhcp service failed.
> + @retval EFI_SUCCESS Operational successful.
> + @retval EFI_NO_RESPONSE Offer dhcp service failed.
> + @retval EFI_BUFFER_TOO_SMALL Failed to copy the packet to Pxe base
> code mode.
>
> **/
> EFI_STATUS
> @@ -515,7 +530,7 @@ PxeBcCheckSelectedOffer (
> //
> // Copy the proxy offer to Mode and set the flag
> //
> - PxeBcCopyProxyOffer (Private, ProxyOfferIndex);
> + Status = PxeBcCopyProxyOffer (Private, ProxyOfferIndex);
> }
> } else {
> //
> @@ -543,7 +558,7 @@ PxeBcCheckSelectedOffer (
> // Other type of ACK is already cached. Bootp is special that we should
> // use the bootp reply as the ACK and put it into the DHCP_ONLY buffer.
> //
> - PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Offer);
> + Status = PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack,
> + Offer);
> }
>
> PxeBcParseCachedDhcpPacket (&Private->Dhcp4Ack); @@ -566,8 +581,11
> @@ PxeBcCheckSelectedOffer (
> @param Private Pointer to PxeBc private data.
> @param RcvdOffer Pointer to the received Dhcp proxy offer packet.
>
> + @retval EFI_SUCCESS Cache and parse the packet successfully.
> + @retval Others Operation failed.
> +
> **/
> -VOID
> +EFI_STATUS
> PxeBcCacheDhcpOffer (
> IN PXEBC_PRIVATE_DATA *Private,
> IN EFI_DHCP4_PACKET *RcvdOffer
> @@ -576,6 +594,7 @@ PxeBcCacheDhcpOffer (
> PXEBC_CACHED_DHCP4_PACKET *CachedOffer;
> EFI_DHCP4_PACKET *Offer;
> UINT8 OfferType;
> + EFI_STATUS Status;
>
> CachedOffer = &Private->Dhcp4Offers[Private->NumOffers];
> Offer = &CachedOffer->Packet.Offer;
> @@ -583,18 +602,21 @@ PxeBcCacheDhcpOffer (
> //
> // Cache the orignal dhcp packet
> //
> - PxeBcCopyEfiDhcp4Packet (Offer, RcvdOffer);
> + Status = PxeBcCopyEfiDhcp4Packet (Offer, RcvdOffer); if
> + (EFI_ERROR(Status)) {
> + return Status;
> + }
>
> //
> // Parse and validate the options (including dhcp option and vendor option)
> //
> if (!PxeBcParseCachedDhcpPacket (CachedOffer)) {
> - return ;
> + return EFI_ABORTED;
> }
>
> OfferType = CachedOffer->OfferType;
> if (OfferType >= DHCP4_PACKET_TYPE_MAX) {
> - return ;
> + return EFI_ABORTED;
> }
>
> if (OfferType == DHCP4_PACKET_TYPE_BOOTP) { @@ -603,7 +625,7 @@
> PxeBcCacheDhcpOffer (
> //
> // Only cache the first bootp offer, discard others.
> //
> - return ;
> + return EFI_ABORTED;
> } else {
> //
> // Take as a dhcp only offer, but record index specifically.
> @@ -628,7 +650,7 @@ PxeBcCacheDhcpOffer (
> //
> // Only cache the first pxe10/wfm11a offers each, discard the others.
> //
> - return ;
> + return EFI_ABORTED;
> } else {
> //
> // Record index of the proxy dhcp offer with type other than binl.
> @@ -649,6 +671,8 @@ PxeBcCacheDhcpOffer (
> // Count the accepted offers.
> //
> Private->NumOffers++;
> +
> + return EFI_SUCCESS;
> }
>
> /**
> @@ -960,6 +984,7 @@ PxeBcDhcpCallBack (
> if (Private->NumOffers < PXEBC_MAX_OFFER_NUM) {
> //
> // Cache the dhcp offers in Private->Dhcp4Offers[]
> + // If error happens, just ignore this packet and continue to wait more
> offer.
> //
> PxeBcCacheDhcpOffer (Private, Packet);
> }
> @@ -982,20 +1007,15 @@ PxeBcDhcpCallBack (
> break;
>
> case Dhcp4RcvdAck:
> - if (Packet->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {
> - //
> - // Abort the DHCP if the ACK packet exceeds the maximum length.
> - //
> - Status = EFI_ABORTED;
> - break;
> - }
> -
> //
> // Cache Ack
> //
> ASSERT (Private->SelectedOffer != 0);
>
> - PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack, Packet);
> + Status = PxeBcCopyEfiDhcp4Packet (&Private->Dhcp4Ack.Packet.Ack,
> Packet);
> + if (EFI_ERROR (Status)) {
> + return EFI_ABORTED;
> + }
> break;
>
> default:
> @@ -1340,6 +1360,12 @@ PxeBcDiscvBootService (
> Response = Token.ResponseList;
>
> while (RepIndex < Token.ResponseCount) {
> + if (Response->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {
> + SrvIndex = 0;
> + RepIndex++;
> + Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response-
> >Size);
> + continue;
> + }
>
> while (SrvIndex < IpCount) {
>
> @@ -1360,14 +1386,16 @@ PxeBcDiscvBootService (
>
> SrvIndex = 0;
> RepIndex++;
> -
> Response = (EFI_DHCP4_PACKET *) ((UINT8 *) Response + Response-
> >Size);
> }
>
> if (RepIndex < Token.ResponseCount) {
>
> if (Reply != NULL) {
> - PxeBcCopyEfiDhcp4Packet (Reply, Response);
> + Status = PxeBcCopyEfiDhcp4Packet (Reply, Response);
> + if (EFI_ERROR(Status)) {
> + goto ON_EXIT;
> + }
> }
>
> if (IsDiscv) {
> @@ -1380,18 +1408,21 @@ PxeBcDiscvBootService (
> } else {
> Status = EFI_NOT_FOUND;
> }
> + }
>
> - //
> - // free the responselist
> - //
> - if (Token.ResponseList != NULL) {
> - FreePool (Token.ResponseList);
> - }
> +ON_EXIT:
> + //
> + // free the responselist
> + //
> + if (Token.ResponseList != NULL) {
> + FreePool (Token.ResponseList);
> }
> //
> // Free the dhcp packet
> //
> - FreePool (Token.Packet);
> + if (Token.Packet != NULL) {
> + FreePool (Token.Packet);
> + }
>
> return Status;
> }
> diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
> b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
> index 614ea75..d19d2a3 100644
> --- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
> +++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
> @@ -291,8 +291,9 @@ PxeBcParseCachedDhcpPacket (
>
> @param Private Pointer to PxeBc private data.
>
> - @retval EFI_SUCCESS Operational successful.
> - @retval EFI_NO_RESPONSE Offer dhcp service failed.
> + @retval EFI_SUCCESS Operational successful.
> + @retval EFI_NO_RESPONSE Offer dhcp service failed.
> + @retval EFI_BUFFER_TOO_SMALL Failed to copy the packet to Pxe base
> code mode.
>
> **/
> EFI_STATUS
> --
> 2.7.4.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-16 8:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-16 8:18 [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Fu Siyuan
2016-12-16 8:18 ` [Patch 1/2] MdeModulePkg: Replace ASSERT with error return code in PXE driver Fu Siyuan
2016-12-16 8:59 ` Wu, Jiaxin
2016-12-16 8:18 ` [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver Fu Siyuan
2016-12-16 8:57 ` Wu, Jiaxin
2016-12-16 8:45 ` [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Ye, Ting
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox