Re: [edk2] [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP reception

[Fu, Siyuan]

Laszlo,

Thanks for the quick fix.
Siyuan

From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Monday, April 4, 2016 7:56 PM
To: Subramanian, Sriram (EG Servers Platform SW) 
Cc: edk2-devel-01 ; Fu, Siyuan ; 
Wu, Jiaxin ; Ye, Ting 
Subject: Re: [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in 
UDP reception

On 04/04/16 05:45, Subramanian, Sriram (EG Servers Platform SW) wrote:
> Thanks for testing that, Laszlo.
>
> Reviewed-by: Sriram Subramanian >

Thanks a lot for the quick review!

Since this bug makes OVMF more or less unusable, I pushed the patch
(which is also very simple) with your R-b: 166a6552a829.

Thanks!
Laszlo

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Sunday, April 3, 2016 3:44 PM
> To: edk2-devel-01 >
> Cc: Siyuan Fu >; Jiaxin Wu 
> >; Ting Ye 
> >; Subramanian, Sriram (EG 
> Servers Platform SW) >
> Subject: [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP 
> reception
>
> Commit 1b31acb66c02 ("MdeModulePkg: Check received packet size before use
> it.") introduced a chunk of code under the new "Resume" label, in function
> UdpIoOnDgramRcvdDpc(). The new code is supposed to run only when the
> received packet has zero-length payload, but a "return" statement was
> forgotten, and the code is reached on the normal (nonzero-length payload)
> path as well, after the packet has been processed (and possibly freed) by
> RxToken->CallBack(). This is a logic bug, with the direct symptom being
> use-after-free / General Protection Fault.
>
> Cc: Siyuan Fu >
> Cc: Jiaxin Wu >
> Cc: Ting Ye >
> Cc: "Subramanian, Sriram (EG Servers Platform SW)" 
> >
> Fixes: 1b31acb66c026f2791c959a4ec9b55c04d583c22
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek >
> ---
>  MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c 
> b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> index 4f7126d3ce56..4861095435e6 100644
> --- a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> +++ b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> @@ -303,6 +303,7 @@ UdpIoOnDgramRcvdDpc (
>}
>
>RxToken->CallBack (Netbuf, , EFI_SUCCESS, RxToken->Context);
> +  return;
>
>  Resume:
>if (RxToken->UdpIo->UdpVersion == UDP_IO_UDP4_VERSION) {
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP reception

[Laszlo Ersek]

On 04/04/16 05:45, Subramanian, Sriram (EG Servers Platform SW) wrote:
> Thanks for testing that, Laszlo.
> 
> Reviewed-by: Sriram Subramanian 

Thanks a lot for the quick review!

Since this bug makes OVMF more or less unusable, I pushed the patch
(which is also very simple) with your R-b: 166a6552a829.

Thanks!
Laszlo

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Sunday, April 3, 2016 3:44 PM
> To: edk2-devel-01 
> Cc: Siyuan Fu ; Jiaxin Wu ; Ting Ye 
> ; Subramanian, Sriram (EG Servers Platform SW) 
> 
> Subject: [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP 
> reception
> 
> Commit 1b31acb66c02 ("MdeModulePkg: Check received packet size before use
> it.") introduced a chunk of code under the new "Resume" label, in function
> UdpIoOnDgramRcvdDpc(). The new code is supposed to run only when the
> received packet has zero-length payload, but a "return" statement was
> forgotten, and the code is reached on the normal (nonzero-length payload)
> path as well, after the packet has been processed (and possibly freed) by
> RxToken->CallBack(). This is a logic bug, with the direct symptom being
> use-after-free / General Protection Fault.
> 
> Cc: Siyuan Fu 
> Cc: Jiaxin Wu 
> Cc: Ting Ye 
> Cc: "Subramanian, Sriram (EG Servers Platform SW)" 
> Fixes: 1b31acb66c026f2791c959a4ec9b55c04d583c22
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c 
> b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> index 4f7126d3ce56..4861095435e6 100644
> --- a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> +++ b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> @@ -303,6 +303,7 @@ UdpIoOnDgramRcvdDpc (
>}
>  
>RxToken->CallBack (Netbuf, , EFI_SUCCESS, RxToken->Context);
> +  return;
>  
>  Resume:
>if (RxToken->UdpIo->UdpVersion == UDP_IO_UDP4_VERSION) {
> 

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

Re: [edk2] [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP reception

[Subramanian, Sriram (EG Servers Platform SW)]

Thanks for testing that, Laszlo.

Reviewed-by: Sriram Subramanian 

Thanks,
Sriram.

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Sunday, April 3, 2016 3:44 PM
To: edk2-devel-01 
Cc: Siyuan Fu ; Jiaxin Wu ; Ting Ye 
; Subramanian, Sriram (EG Servers Platform SW) 

Subject: [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP 
reception

Commit 1b31acb66c02 ("MdeModulePkg: Check received packet size before use
it.") introduced a chunk of code under the new "Resume" label, in function
UdpIoOnDgramRcvdDpc(). The new code is supposed to run only when the
received packet has zero-length payload, but a "return" statement was
forgotten, and the code is reached on the normal (nonzero-length payload)
path as well, after the packet has been processed (and possibly freed) by
RxToken->CallBack(). This is a logic bug, with the direct symptom being
use-after-free / General Protection Fault.

Cc: Siyuan Fu 
Cc: Jiaxin Wu 
Cc: Ting Ye 
Cc: "Subramanian, Sriram (EG Servers Platform SW)" 
Fixes: 1b31acb66c026f2791c959a4ec9b55c04d583c22
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c 
b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
index 4f7126d3ce56..4861095435e6 100644
--- a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
+++ b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
@@ -303,6 +303,7 @@ UdpIoOnDgramRcvdDpc (
   }
 
   RxToken->CallBack (Netbuf, , EFI_SUCCESS, RxToken->Context);
+  return;
 
 Resume:
   if (RxToken->UdpIo->UdpVersion == UDP_IO_UDP4_VERSION) {
-- 
1.8.3.1

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

[edk2] [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP reception

[Laszlo Ersek]

Commit 1b31acb66c02 ("MdeModulePkg: Check received packet size before use
it.") introduced a chunk of code under the new "Resume" label, in function
UdpIoOnDgramRcvdDpc(). The new code is supposed to run only when the
received packet has zero-length payload, but a "return" statement was
forgotten, and the code is reached on the normal (nonzero-length payload)
path as well, after the packet has been processed (and possibly freed) by
RxToken->CallBack(). This is a logic bug, with the direct symptom being
use-after-free / General Protection Fault.

Cc: Siyuan Fu 
Cc: Jiaxin Wu 
Cc: Ting Ye 
Cc: "Subramanian, Sriram (EG Servers Platform SW)" 
Fixes: 1b31acb66c026f2791c959a4ec9b55c04d583c22
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c 
b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
index 4f7126d3ce56..4861095435e6 100644
--- a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
+++ b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
@@ -303,6 +303,7 @@ UdpIoOnDgramRcvdDpc (
   }
 
   RxToken->CallBack (Netbuf, , EFI_SUCCESS, RxToken->Context);
+  return;
 
 Resume:
   if (RxToken->UdpIo->UdpVersion == UDP_IO_UDP4_VERSION) {
-- 
1.8.3.1

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