Hi Ard, Much of this code has not been updated since initially added in 2010.
Looks like a bug to me that has been there the whole time. I agree it is a behavior change in the implementation. But unless new code use of this API looks at the implementation, they would not know it is destructive and they need to make a copy. This API is available to external shell apps that use the shell protocol. We should get the shell owners to evaluate removing the destructive behavior. Mike > -----Original Message----- > From: Ard Biesheuvel <a...@kernel.org> > Sent: Thursday, December 8, 2022 10:45 AM > To: Kinney, Michael D <michael.d.kin...@intel.com> > Cc: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > Subject: Re: [edk2-devel] [PATCH] ShellPkg: Avoid corrupting installed device > path protocols > > On Thu, 8 Dec 2022 at 19:28, Kinney, Michael D > <michael.d.kin...@intel.com> wrote: > > > > Hi Ard, > > > > From this description, it does not look like it should be modifying the > > contents of the device path. Just point to the device path end node that > > follows the match found. > > > > /** > > Gets the mapping that most closely matches the device path. > > > > This function gets the mapping which corresponds to the device path > > *DevicePath. If > > there is no exact match, then the mapping which most closely matches > > *DevicePath > > is returned, and *DevicePath is updated to point to the remaining portion > > of the > > device path. If there is an exact match, the mapping is returned and > > *DevicePath > > points to the end-of-device-path node. > > > > @param DevicePath On entry, points to a device path pointer. > > On > > exit, updates the pointer to point to the > > portion of the device path after the > > mapping. > > > > @retval NULL No mapping was found. > > @return !=NULL Pointer to NULL-terminated mapping. The > > buffer > > is callee allocated and should be freed by > > the caller. > > **/ > > CONST CHAR16 * > > EFIAPI > > EfiShellGetMapFromDevicePath ( > > IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath > > ); > > > > I see this API used in many places, and it looks like it would be > > destructive to any multi-instance device path. Multi-instance > > device paths are typically used for consoles, so we may not have > > noticed this destructive behavior with file system mapping paths. > > > > Did you try removing the call to SetDevicePathEndNode (*DevicePath); ? > > > > No, but that would be a functional change visible to all users of the > current API. > > And note that the calling code already has 'DevicePathCopy' variables, > it just doesn't bother using them, so the intent is clearly to pass a > copy, not the actual device path. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97146): https://edk2.groups.io/g/devel/message/97146 Mute This Topic: https://groups.io/mt/95518373/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
