Re: [Patch] ShellPkg/Hexedit: Fix FreePool() ASSERT() when writing disk

["Carsey, Jaben" <jaben.carsey@intel.com>]

Reviewed-by: Jaben Carsey <Jaben.carsey@intel.com>


> -----Original Message-----
> From: Ni, Ruiyu 
> Sent: Wednesday, September 28, 2016 7:52 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: RE: [Patch] ShellPkg/Hexedit: Fix FreePool() ASSERT() when writing disk
> 
> I agree to use the same style of code as what the other functions do.
> 
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> -----Original Message-----
>> From: Kinney, Michael D
>> Sent: Thursday, September 29, 2016 8:38 AM
>> To: edk2-devel@lists.01.org
>> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
>> Subject: [Patch] ShellPkg/Hexedit: Fix FreePool() ASSERT() when writing disk
>> 
>> The HDiskImageSave() function copies a device path using
>> DuplicateDevicePath() and passes that device path to
>> gBS->LocateDevicePath() that changes the value of the
>> device path pointer.  When FreePool() is called with the
>> modified device path pointer, the FreePool() service
>> generates an ASSERT() because the signature for the pool
>> head can not be found.
>> 
>> The function HDiskImageRead() immediately above
>> HDiskImageSave() has the correct algorithm that uses an
>> additional local variable called DupDevicePathForFree to
>> preserve the pointer to the allocated buffer so it can
>> be used in the call to FreePool().
>> 
>> Bug: <https://bugzilla.tianocore.org/show_bug.cgi?id=131>
>> 
>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
>> ---
>> ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
>> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
>> index a50b52f..bc74a4f 100644
>> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
>> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
>> @@ -343,6 +343,7 @@ HDiskImageSave (
>> 
>>  CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
>>  EFI_DEVICE_PATH_PROTOCOL        *DupDevicePath;
>> +  EFI_DEVICE_PATH_PROTOCOL        *DupDevicePathForFree;
>>  EFI_BLOCK_IO_PROTOCOL           *BlkIo;
>>  EFI_STATUS                      Status;
>>  EFI_HANDLE                      Handle;
>> @@ -364,12 +365,13 @@ HDiskImageSave (
>>    return EFI_INVALID_PARAMETER;
>>  }
>>  DupDevicePath = DuplicateDevicePath(DevicePath);
>> +  DupDevicePathForFree = DupDevicePath;
>> 
>>  //
>>  // get blkio interface
>>  //
>>  Status = gBS->LocateDevicePath(&gEfiBlockIoProtocolGuid,&DupDevicePath,&Handle);
>> -  FreePool(DupDevicePath);
>> +  FreePool(DupDevicePathForFree);
>>  if (EFI_ERROR (Status)) {
>> //    StatusBarSetStatusString (L"Read Disk Failed");
>>    return Status;
>> --
>> 2.6.3.windows.1
>