Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow.

["Maciej Rabeda" <maciej.rabeda@linux.intel.com>]

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 <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
>> 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 <ray.ni@intel.com>
>>> Cc: Zhichao Gao <zhichao.gao@intel.com>
>>> Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>>> ---
>>>   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);
>>>   }
>>>
>>>   /**
>>>
>
> 
>