Re: [Patch] MdeModulePkg/IpIoLib: add more error handling code to DxeIpIoLib.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu,
> Siyuan
> Sent: Wednesday, December 13, 2017 3:07 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wang, Fan <fan.wang@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch] MdeModulePkg/IpIoLib: add more error handling
> code to DxeIpIoLib.
> 
> In DxeIpIo, there are several places not check the returned value of called
> functions.
> This patch is to add the error handling to these functions.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Cc: Wang Fan <fan.wang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
>  MdeModulePkg/Include/Library/IpIoLib.h       |  4 +-
>  MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 83
> +++++++++++++++++-----------
>  2 files changed, 54 insertions(+), 33 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/IpIoLib.h
> b/MdeModulePkg/Include/Library/IpIoLib.h
> index aab0c68059..a8496f729d 100644
> --- a/MdeModulePkg/Include/Library/IpIoLib.h
> +++ b/MdeModulePkg/Include/Library/IpIoLib.h
> @@ -2,7 +2,7 @@
>    This library is only intended to be used by UEFI network stack modules.
>    It provides the combined IpIo layer on the EFI IP4 Protocol and EFI IP6
> protocol.
> 
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -426,7 +426,7 @@ IpIoSend (
>    IN     IP_IO_IP_INFO  *Sender        OPTIONAL,
>    IN     VOID           *Context       OPTIONAL,
>    IN     VOID           *NotifyData    OPTIONAL,
> -  IN     EFI_IP_ADDRESS *Dest,
> +  IN     EFI_IP_ADDRESS *Dest          OPTIONAL,
>    IN     IP_IO_OVERRIDE *OverrideData  OPTIONAL
>    );
> 
> diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> index abc07fb0ff..6f312cccb2 100644
> --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> @@ -2,7 +2,7 @@
>    IpIo Library.
> 
>  (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -234,24 +234,25 @@ IpIoCloseProtocolDestroyIpChild (
>    //
>    // Close the previously openned IP protocol.
>    //
> -  gBS->CloseProtocol (
> -         ChildHandle,
> -         IpProtocolGuid,
> -         ImageHandle,
> -         ControllerHandle
> -         );
> +  Status = gBS->CloseProtocol (
> +                  ChildHandle,
> +                  IpProtocolGuid,
> +                  ImageHandle,
> +                  ControllerHandle
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> 
>    //
>    // Destroy the IP child.
>    //
> -  Status = NetLibDestroyServiceChild (
> -             ControllerHandle,
> -             ImageHandle,
> -             ServiceBindingGuid,
> -             ChildHandle
> -             );
> -
> -  return Status;
> +  return NetLibDestroyServiceChild (
> +           ControllerHandle,
> +           ImageHandle,
> +           ServiceBindingGuid,
> +           ChildHandle
> +           );
>  }
> 
>  /**
> @@ -377,8 +378,14 @@ IpIoIcmpv4Handler (
>    TrimBytes  = (UINT32) (PayLoadHdr - (UINT8 *) IcmpHdr);
> 
>    NetbufTrim (Pkt, TrimBytes, TRUE);
> -
> -  IpIo->PktRcvdNotify (EFI_ICMP_ERROR, IcmpErr, Session, Pkt, IpIo-
> >RcvdContext);
> +
> +  //
> +  // If the input packet has invalid format, and TrimBytes is larger than
> +  // the packet size, the NetbufTrim might trim the packet to zero.
> +  //
> +  if (Pkt->TotalSize != 0) {
> +    IpIo->PktRcvdNotify (EFI_ICMP_ERROR, IcmpErr, Session, Pkt, IpIo-
> >RcvdContext);
> +  }
> 
>    return EFI_SUCCESS;
>  }
> @@ -539,7 +546,13 @@ IpIoIcmpv6Handler (
> 
>    NetbufTrim (Pkt, TrimBytes, TRUE);
> 
> -  IpIo->PktRcvdNotify (EFI_ICMP_ERROR, IcmpErr, Session, Pkt, IpIo-
> >RcvdContext);
> +  //
> +  // If the input packet has invalid format, and TrimBytes is larger than
> +  // the packet size, the NetbufTrim might trim the packet to zero.
> +  //
> +  if (Pkt->TotalSize != 0) {
> +    IpIo->PktRcvdNotify (EFI_ICMP_ERROR, IcmpErr, Session, Pkt, IpIo-
> >RcvdContext);
> +  }
> 
>    return EFI_SUCCESS;
>  }
> @@ -1540,7 +1553,7 @@ IpIoSend (
>    IN     IP_IO_IP_INFO  *Sender        OPTIONAL,
>    IN     VOID           *Context       OPTIONAL,
>    IN     VOID           *NotifyData    OPTIONAL,
> -  IN     EFI_IP_ADDRESS *Dest,
> +  IN     EFI_IP_ADDRESS *Dest          OPTIONAL,
>    IN     IP_IO_OVERRIDE *OverrideData  OPTIONAL
>    )
>  {
> @@ -1791,7 +1804,7 @@ IpIoConfigIp (
>    }
> 
>    if (IpConfigData != NULL) {
> -    if (IpInfo->IpVersion == IP_VERSION_4){
> +    if (IpInfo->IpVersion == IP_VERSION_4) {
> 
>        if (((EFI_IP4_CONFIG_DATA *) IpConfigData)->UseDefaultAddress) {
>          Ip.Ip4->GetModeData (
> @@ -1800,10 +1813,14 @@ IpIoConfigIp (
>                    NULL,
>                    NULL
>                    );
> +        if (EFI_ERROR (Status)) {
> +          Ip.Ip4->Configure (Ip.Ip4, NULL);
> +          goto OnExit;
> +        }
> 
>          IP4_COPY_ADDRESS (&((EFI_IP4_CONFIG_DATA*) IpConfigData)-
> >StationAddress, &Ip4ModeData.ConfigData.StationAddress);
>          IP4_COPY_ADDRESS (&((EFI_IP4_CONFIG_DATA*) IpConfigData)-
> >SubnetMask, &Ip4ModeData.ConfigData.SubnetMask);
> -    }
> +      }
> 
>        CopyMem (
>          &IpInfo->Addr.Addr,
> @@ -1820,16 +1837,20 @@ IpIoConfigIp (
>                           Ip.Ip4,
>                           &IpInfo->DummyRcvToken.Ip4Token
>                           );
> -    if (EFI_ERROR (Status)) {
> -      Ip.Ip4->Configure (Ip.Ip4, NULL);
> -    }
> -  } else {
> -    Ip.Ip6->GetModeData (
> -              Ip.Ip6,
> -              &Ip6ModeData,
> -              NULL,
> -              NULL
> -              );
> +      if (EFI_ERROR (Status)) {
> +        Ip.Ip4->Configure (Ip.Ip4, NULL);
> +      }
> +    } else {
> +      Ip.Ip6->GetModeData (
> +                Ip.Ip6,
> +                &Ip6ModeData,
> +                NULL,
> +                NULL
> +                );
> +      if (EFI_ERROR (Status)) {
> +        Ip.Ip6->Configure (Ip.Ip6, NULL);
> +        goto OnExit;
> +      }
> 
>        if (Ip6ModeData.IsConfigured) {
>          CopyMem (
> --
> 2.13.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel