Also include NetworkPkg reviewer to collect the feedback for this change. Thanks Liming > -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: Friday, February 28, 2020 1:40 AM > To: devel@edk2.groups.io; maciej.rab...@linux.intel.com; Gao, Liming > <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Andrew Fish <af...@apple.com>; Leif Lindholm > (Linaro address) <leif.lindh...@linaro.org> > Cc: Ni, Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; Armour, > Nicholas <nicholas.arm...@intel.com> > Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive > flow. > > On 02/27/20 14:14, Laszlo Ersek wrote: > > (+Liming and stewards; CC Nick) > > > > On 02/27/20 12:02, Maciej Rabeda wrote: > >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032 > >> > >> 'ping' command's receive flow utilizes a single Rx token which it > >> attempts to reuse before recycling the previously received packet. > >> This causes a situation where under ICMP traffic, > >> Ping6OnEchoReplyReceived() function will receive an already > >> recycled packet with EFI_SUCCESS token status and finally > >> dereference invalid pointers from RxData structure. > >> > >> Cc: Ray Ni <ray...@intel.com> > >> Cc: Zhichao Gao <zhichao....@intel.com> > >> Signed-off-by: Maciej Rabeda <maciej.rab...@linux.intel.com> > >> --- > >> ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> index 23567fa2c1bb..a3fa32515192 100644 > >> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> @@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived ( > >> > >> ON_EXIT: > >> > >> + // > >> + // Recycle the packet before reusing RxToken > >> + // > >> + gBS->SignalEvent (Private->IpChoice == > >> PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)- > >RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal); > >> + > >> if (Private->RxCount < Private->SendNum) { > >> // > >> // Continue to receive icmp echo reply packets. > >> @@ -632,10 +637,6 @@ ON_EXIT: > >> // > >> Private->Status = EFI_SUCCESS; > >> } > >> - // > >> - // Singal to recycle the each rxdata here, not at the end of process. > >> - // > >> - gBS->SignalEvent (Private->IpChoice == > >> PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)- > >RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal); > >> } > >> > >> /** > >> > > > > (1) This patch proposes to fix one of the BZs (2032) that fall under > > CVE-2019-14559 (joint tracker: 2550). > > > > Consequently: > > > > (1a) Do we want to include this in the upcoming stable tag? > > > > If so, we might want to extend the hard feature freeze by a few days. > > > > (1b) Please append the string " (CVE-2019-14559)" -- note the separating > > space! -- to the subject line. > > > > (2) However: I remember from an earlier Bugzilla entry (can't tell > > off-hand, which one, sorry) that ShellPkg issues are *never* considered > > CVE-worthy, because the shell is not considered a "production element" > > of the UEFI boot path. > > I misremembered -- there is indeed a comment like that, in the TianoCore > bugzilla, but it does not refer to ShellPkg. It refers to StdLib (which > has since been split off to the edk2-libc project): > > https://bugzilla.tianocore.org/show_bug.cgi?id=1510#c1 > > StdLib is supposed to be used only by applications in shell, all of > which are meant for debug, diagnosis and/or test purpose, not for > product UEFI BIOS. Any issue in it will not be taken as security > issue but just normal bug. > > Sorry about causing confusion. So, the ShellPkg maintainers should > decide what to do about this bug (keep it under the CVE scope vs. > exclude it from the CVE scope; and then, propose it for the stable tag > or merge it afterwards). > > One data point: the bug appears to go back to the inception of the Ping > command, in historical commit 68fb05272b45 ("Add Network1 profile.", > 2011-03-25). It's not a new bug, it seems. > > Thanks > Laszlo > > > > > TianoCore#2032 was originally filed for NetworkPkg, and indeed that > > seemed to justify the CVE assignment. However, now that Nick's and > > Maciej's analysis shows that NetworkPkg is unaffected (and we know, per > > above, that ShellPkg is not CVE-worthy), should we rather *remove* this > > BZ from the CVE-2019-14559 umbrella? > > > > Because, in that case, modifying the subject line on the patch is not > > necessary; and more importantly, we might not even want to put this into > > edk2-stable202002. (It's still a bugfix, but may not be important enough.) > > > > Thanks! > > Laszlo > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55044): https://edk2.groups.io/g/devel/message/55044 Mute This Topic: https://groups.io/mt/71584586/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
