Mike,
How about remove the DuplicateDevicePath and FreePool?
------
.../Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
index a50b52f..bfdbb40 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/DiskImage.c
@@ -340,9 +340,7 @@ HDiskImageSave (
IN UINTN Size
)
{
-
- CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath;
- EFI_DEVICE_PATH_PROTOCOL *DupDevicePath;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
EFI_BLOCK_IO_PROTOCOL *BlkIo;
EFI_STATUS Status;
EFI_HANDLE Handle;
@@ -358,18 +356,15 @@ HDiskImageSave (
HBufferImage.BufferType = FileTypeDiskBuffer;
- DevicePath =
gEfiShellProtocol->GetDevicePathFromMap(DeviceName);
+ DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)
gEfiShellProtocol->GetDevicePathFromMap(DeviceName);
if (DevicePath == NULL) {
// StatusBarSetStatusString (L"Cannot Find Device");
return EFI_INVALID_PARAMETER;
}
- DupDevicePath = DuplicateDevicePath(DevicePath);
-
//
// get blkio interface
//
- Status =
gBS->LocateDevicePath(&gEfiBlockIoProtocolGuid,&DupDevicePath,&Handle);
- FreePool(DupDevicePath);
+ Status = gBS->LocateDevicePath(&gEfiBlockIoProtocolGuid,&DevicePath,&Handle);
if (EFI_ERROR (Status)) {
// StatusBarSetStatusString (L"Read Disk Failed");
return Status;
Regards,
Ray
From: Kinney, Michael D
Sent: Thursday, September 29, 2016 10:12 AM
To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D
<michael.d.kin...@intel.com>
Cc: Carsey, Jaben <jaben.car...@intel.com>
Subject: RE: [Patch] ShellPkg/Hexedit: Fix FreePool() ASSERT() when writing disk
Ray,
I agree that the contents of the device path buffer is not changed.
&DupDevicePath is passed into LocateDevicePath() and the
value pointed to by &DupDevicePath is modified.
Then the call to FreePool(DupDevicePath) uses a different
DupDevicePath value than was returned by
DuplicateDevicePath(DevicePath). That generated CR macro
ASSERT().
Mike
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, September 28, 2016 7:00 PM
> To: Kinney, Michael D
> <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>;
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Carsey, Jaben <jaben.car...@intel.com<mailto:jaben.car...@intel.com>>
> Subject: RE: [Patch] ShellPkg/Hexedit: Fix FreePool() ASSERT() when writing
> disk
>
> Mike,
> BS.LocateDevicePath() doesn't change the device path content.
> So I think the Duplication/Free of device path is not needed.
>
> Regards,
> Ray
>
> >-----Original Message-----
> >From: Kinney, Michael D
> >Sent: Thursday, September 29, 2016 8:38 AM
> >To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> >Cc: Carsey, Jaben <jaben.car...@intel.com<mailto:jaben.car...@intel.com>>;
> >Ni, Ruiyu <ruiyu...@intel.com<mailto:ruiyu...@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.car...@intel.com<mailto:jaben.car...@intel.com>>
> >Cc: Ruiyu Ni <ruiyu...@intel.com<mailto:ruiyu...@intel.com>>
> >Contributed-under: TianoCore Contribution Agreement 1.0
> >Signed-off-by: Michael Kinney
> ><michael.d.kin...@intel.com<mailto:michael.d.kin...@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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
