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 <[email protected]>; [email protected] > Cc: Carsey, Jaben <[email protected]> > 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: [email protected] > >Cc: Carsey, Jaben <[email protected]>; Ni, Ruiyu <[email protected]> > >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 <[email protected]> > >Cc: Ruiyu Ni <[email protected]> > >Contributed-under: TianoCore Contribution Agreement 1.0 > >Signed-off-by: Michael Kinney <[email protected]> > >--- > > 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
