public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Fu, Siyuan" <siyuan.fu@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>, "Ye, Ting" <ting.ye@intel.com>
Subject: Re: [Patch 2/2] NetworkPkg: Replace ASSERT with error return code in PXE and HTTP boot driver.
Date: Fri, 16 Dec 2016 08:57:47 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B727416280C9D@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1481876334-130492-3-git-send-email-siyuan.fu@intel.com>

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



  reply	other threads:[~2016-12-16  8:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2016-12-16  8:45 ` [Patch 0/2] Replace ASSERT with error return code in PXE and HTTP boot Ye, Ting

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895558F6EA4E3B41AC93A00D163B727416280C9D@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox