Re: [edk2] [PATCH] MdeModulePkg: DxeUdpIoLib: fix non-empty payload path in UDP reception
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 mailto:srira...@hpe.com>> 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 mailto:edk2-de...@ml01.01.org>> > Cc: Siyuan Fu mailto:siyuan...@intel.com>>; Jiaxin Wu > mailto:jiaxin...@intel.com>>; Ting Ye > mailto:ting...@intel.com>>; Subramanian, Sriram (EG > Servers Platform SW) mailto:srira...@hpe.com>> > 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 mailto:siyuan...@intel.com>> > Cc: Jiaxin Wu mailto:jiaxin...@intel.com>> > Cc: Ting Ye mailto:ting...@intel.com>> > Cc: "Subramanian, Sriram (EG Servers Platform SW)" > mailto:srira...@hpe.com>> > Fixes: 1b31acb66c026f2791c959a4ec9b55c04d583c22 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek mailto:ler...@redhat.com>> > --- > 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, &EndPoint, 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
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, &EndPoint, 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
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, &EndPoint, 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