From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web09.8818.1582858785978237389 for ; Thu, 27 Feb 2020 18:59:46 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: liming.gao@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Feb 2020 18:59:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,493,1574150400"; d="scan'208";a="232408649" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga008.fm.intel.com with ESMTP; 27 Feb 2020 18:59:44 -0800 Received: from shsmsx605.ccr.corp.intel.com (10.109.6.215) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 27 Feb 2020 18:59:40 -0800 Received: from shsmsx606.ccr.corp.intel.com (10.109.6.216) by SHSMSX605.ccr.corp.intel.com (10.109.6.215) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 28 Feb 2020 10:59:37 +0800 Received: from shsmsx606.ccr.corp.intel.com ([10.109.6.216]) by SHSMSX606.ccr.corp.intel.com ([10.109.6.216]) with mapi id 15.01.1713.004; Fri, 28 Feb 2020 10:59:37 +0800 From: "Liming Gao" To: Laszlo Ersek , "devel@edk2.groups.io" , "maciej.rabeda@linux.intel.com" , "Kinney, Michael D" , Andrew Fish , "Leif Lindholm (Linaro address)" CC: "Ni, Ray" , "Gao, Zhichao" , "Armour, Nicholas" , "Fu, Siyuan" , "Wu, Jiaxin" Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow. Thread-Topic: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow. Thread-Index: AQHV7V1lV4ffxVHDRU2SAtogmqLGr6gufvSAgABKLwCAASF4kA== Date: Fri, 28 Feb 2020 02:59:37 +0000 Message-ID: <3e3521ab1a17472a921f238db1f8dd9b@intel.com> References: <20200227110212.1070-1-maciej.rabeda@linux.intel.com> <4dcf2f0b-c86e-7533-3428-ad07e9129f2d@redhat.com> In-Reply-To: <4dcf2f0b-c86e-7533-3428-ad07e9129f2d@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.2.0.6 dlp-product: dlpe-windows dlp-reaction: no-action x-originating-ip: [10.239.127.36] MIME-Version: 1.0 Return-Path: liming.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Also include NetworkPkg reviewer to collect the feedback for this change.=20 Thanks Liming > -----Original Message----- > From: Laszlo Ersek > Sent: Friday, February 28, 2020 1:40 AM > To: devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Gao, Liming ; Kinney, Michael D > ; Andrew Fish ; Leif Lindhol= m (Linaro address) > Cc: Ni, Ray ; Gao, Zhichao ; Arm= our, Nicholas > Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 rec= eive flow. >=20 > 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=3D2032 > >> > >> '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 > >> Cc: Zhichao Gao > >> Signed-off-by: Maciej Rabeda > >> --- > >> ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c b/Sh= ellPkg/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 =3D=3D PING_IP_CHOICE_IP6?((EFI= _IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)- > >RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->R= ecycleSignal); > >> + > >> if (Private->RxCount < Private->SendNum) { > >> // > >> // Continue to receive icmp echo reply packets. > >> @@ -632,10 +637,6 @@ ON_EXIT: > >> // > >> Private->Status =3D EFI_SUCCESS; > >> } > >> - // > >> - // Singal to recycle the each rxdata here, not at the end of proces= s. > >> - // > >> - gBS->SignalEvent (Private->IpChoice =3D=3D PING_IP_CHOICE_IP6?((EFI= _IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)- > >RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->R= ecycleSignal); > >> } > >> > >> /** > >> > > > > (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 separatin= g > > 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. >=20 > 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): >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1510#c1 >=20 > 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. >=20 > 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). >=20 > 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. >=20 > Thanks > Laszlo >=20 > > > > 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 int= o > > edk2-stable202002. (It's still a bugfix, but may not be important enoug= h.) > > > > Thanks! > > Laszlo > >