[edk2] [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.

Wang Fan posted 2 patches 6 years, 11 months ago
[edk2] [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.
Posted by Wang Fan 6 years, 11 months ago
* When the packet is not normal packet or icmp error packet, the code
  does not recycle it by signal RecycleSignal event, and this will
  result some memory leak. This patch is to fix this issue.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
 MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
index a06c0b6..c7bc1aa 100644
--- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
+++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
@@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
     // The reception is actively aborted by the consumer, directly return.
     //
     return;
   }
 
-  if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL == RxData)) {
+  if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
     //
-    // @bug Only process the normal packets and the icmp error packets, if RxData is NULL
-    // @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume the receive although
-    // @bug this should be a bug of the low layer (IP).
+    // Only process the normal packets and the icmp error packets.
     //
+    if (RxData != NULL) {
+      goto CleanUp;
+    } else {
+      goto Resume;
+    }
+  }
+
+  //
+  // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this should be a code issue in the low layer (IP).
+  //
+  ASSERT (RxData != NULL);
+  if (RxData == NULL) {
     goto Resume;
   }
 
   if (NULL == IpIo->PktRcvdNotify) {
     goto CleanUp;
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.
Posted by Fu, Siyuan 6 years, 11 months ago
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>

> -----Original Message-----
> From: Wang, Fan
> Sent: Wednesday, January 10, 2018 11:16 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it
> is not expected.
> 
> * When the packet is not normal packet or icmp error packet, the code
>   does not recycle it by signal RecycleSignal event, and this will
>   result some memory leak. This patch is to fix this issue.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
>  MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> index a06c0b6..c7bc1aa 100644
> --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> @@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
>      // The reception is actively aborted by the consumer, directly return.
>      //
>      return;
>    }
> 
> -  if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL ==
> RxData)) {
> +  if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
>      //
> -    // @bug Only process the normal packets and the icmp error packets,
> if RxData is NULL
> -    // @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume the
> receive although
> -    // @bug this should be a bug of the low layer (IP).
> +    // Only process the normal packets and the icmp error packets.
>      //
> +    if (RxData != NULL) {
> +      goto CleanUp;
> +    } else {
> +      goto Resume;
> +    }
> +  }
> +
> +  //
> +  // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this
> should be a code issue in the low layer (IP).
> +  //
> +  ASSERT (RxData != NULL);
> +  if (RxData == NULL) {
>      goto Resume;
>    }
> 
>    if (NULL == IpIo->PktRcvdNotify) {
>      goto CleanUp;
> --
> 1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.
Posted by Wu, Jiaxin 6 years, 11 months ago
Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Wang, Fan
> Sent: Wednesday, January 10, 2018 11:16 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is
> not expected.
> 
> * When the packet is not normal packet or icmp error packet, the code
>   does not recycle it by signal RecycleSignal event, and this will
>   result some memory leak. This patch is to fix this issue.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
>  MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> index a06c0b6..c7bc1aa 100644
> --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> @@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
>      // The reception is actively aborted by the consumer, directly return.
>      //
>      return;
>    }
> 
> -  if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL ==
> RxData)) {
> +  if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
>      //
> -    // @bug Only process the normal packets and the icmp error packets, if
> RxData is NULL
> -    // @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume
> the receive although
> -    // @bug this should be a bug of the low layer (IP).
> +    // Only process the normal packets and the icmp error packets.
>      //
> +    if (RxData != NULL) {
> +      goto CleanUp;
> +    } else {
> +      goto Resume;
> +    }
> +  }
> +
> +  //
> +  // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this
> should be a code issue in the low layer (IP).
> +  //
> +  ASSERT (RxData != NULL);
> +  if (RxData == NULL) {
>      goto Resume;
>    }
> 
>    if (NULL == IpIo->PktRcvdNotify) {
>      goto CleanUp;
> --
> 1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel