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.web10.5350.1585645419249207938 for ; Tue, 31 Mar 2020 02:03:39 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 134.134.136.126, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: JQBp4IHnCnvcKIOPqzJDR529EFNomgqFr0LNFZ9uhORhXnKEyBdmO0dlHJr3pDpowE08S4wlEe oInHckCT9A8A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Mar 2020 02:03:38 -0700 IronPort-SDR: yNoCUyFgv5Cba/R91x05JB0UUDdYXMW7uTg9vDpmdpk03xiDs/R7+a66n/2EiVZ8hA0y/0daog hmHAd1H8qf3Q== X-IronPort-AV: E=Sophos;i="5.72,327,1580803200"; d="scan'208";a="395425038" Received: from tossows-mobl2.ger.corp.intel.com (HELO [10.213.8.245]) ([10.213.8.245]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Mar 2020 02:03:36 -0700 Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow. To: devel@edk2.groups.io, zhichao.gao@intel.com, Laszlo Ersek , "Ni, Ray" , "Wu, Jiaxin" , "Fu, Siyuan" References: <20200227110212.1070-1-maciej.rabeda@linux.intel.com> <526dba76ac004389873ca89c053407af@intel.com> From: "Maciej Rabeda" Message-ID: Date: Tue, 31 Mar 2020 11:03:34 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <526dba76ac004389873ca89c053407af@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: pl Siyuan/Jiaxin, Can we get this patch reviewed? I cannot give myself green light on this patch :) On 26-Mar-20 04:16, Gao, Zhichao wrote: > The ping command implementation would go into the ip4/ip6 protocol. But I am not familiar with the network part. > > Jiaxin/Siyuan, > > Can you help to review this patch? > > Thanks, > Zhichao > >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Wednesday, March 25, 2020 7:34 PM >> To: Ni, Ray ; Gao, Zhichao >> Cc: devel@edk2.groups.io; maciej.rabeda@linux.intel.com >> Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 >> receive flow. >> >> Ray, Zhichao, >> >> 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 >>> Cc: Zhichao Gao >>> Signed-off-by: Maciej Rabeda >>> --- >>> ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >> can you please review this ShellPkg patch? It's been on the list for almost a >> month now. >> >> Thanks >> Laszlo >> >>> 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.R >>> + xData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private- >>> RxToken.Packe >>> + t.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.RxD >>> ata)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private- >>> RxToken.Packet.Rx >>> Data)->RecycleSignal); >>> } >>> >>> /** >>> > > >