[edk2-devel] [PATCH] ShellPkg: Avoid corrupting installed device path protocols
Michael D Kinney
michael.d.kinney at intel.com
Thu Dec 8 19:19:49 UTC 2022
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 <ardb at kernel.org>
> Sent: Thursday, December 8, 2022 10:45 AM
> To: Kinney, Michael D <michael.d.kinney at intel.com>
> Cc: devel at edk2.groups.io; Ni, Ray <ray.ni at intel.com>; Gao, Zhichao <zhichao.gao at 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.kinney at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list