Re: [edk2-devel] [PATCH] ShellPkg: Avoid corrupting installed device path protocols

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Ard,

Are you saying that ConvertEfiFileProtocolToShellHandle() modifies the device path passed in?

Is that a bug in this API?

Mike


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> Sent: Wednesday, December 7, 2022 8:13 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Ard Biesheuvel <ardb@kernel.org>
> Subject: [edk2-devel] [PATCH] ShellPkg: Avoid corrupting installed device path protocols
> 
> The Shell locates device path protocol instances from the database and
> happily passes them to destructive device path operations, resulting in
> the original protocol to get corrupted as well. So take a copy instead,
> and discard it once we no longer need it.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  ShellPkg/Application/Shell/ShellProtocol.c    | 10 +++-
>  .../Library/UefiShellLevel2CommandsLib/Map.c  | 47 +++++++++++--------
>  2 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
> index 509eb60e40f4..6dbf344520d0 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -838,7 +838,9 @@ EfiShellOpenRootByHandle (
>    EFI_STATUS                       Status;
> 
>    EFI_SIMPLE_FILE_SYSTEM_PROTOCOL  *SimpleFileSystem;
> 
>    EFI_FILE_PROTOCOL                *RealFileHandle;
> 
> -  EFI_DEVICE_PATH_PROTOCOL         *DevPath;
> 
> +  CONST EFI_DEVICE_PATH_PROTOCOL   *DevPath;
> 
> +  VOID                             *DevPathBuf;
> 
> +  EFI_DEVICE_PATH_PROTOCOL         *DevPathCopy;
> 
> 
> 
>    //
> 
>    // get the simple file system interface
> 
> @@ -875,7 +877,11 @@ EfiShellOpenRootByHandle (
>      return Status;
> 
>    }
> 
> 
> 
> -  *FileHandle = ConvertEfiFileProtocolToShellHandle (RealFileHandle, EfiShellGetMapFromDevicePath (&DevPath));
> 
> +  DevPathCopy = DevPathBuf = DuplicateDevicePath (DevPath);
> 
> +  *FileHandle = ConvertEfiFileProtocolToShellHandle (RealFileHandle,
> 
> +                                                     EfiShellGetMapFromDevicePath (&DevPathCopy)
> 
> +                                                     );
> 
> +  SHELL_FREE_NON_NULL (DevPathBuf);
> 
>    return (EFI_SUCCESS);
> 
>  }
> 
> 
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c b/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c
> index f3c888edd48c..094e08eab4a5 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c
> @@ -134,7 +134,7 @@ SearchList (
>  **/
> 
>  CHAR16 *
> 
>  GetDeviceMediaType (
> 
> -  IN  EFI_DEVICE_PATH_PROTOCOL  *DevicePath
> 
> +  IN  CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath
> 
>    )
> 
>  {
> 
>    ACPI_HID_DEVICE_PATH  *Acpi;
> 
> @@ -179,7 +179,7 @@ GetDeviceMediaType (
>  **/
> 
>  BOOLEAN
> 
>  IsRemoveableDevice (
> 
> -  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath
> 
> +  IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath
> 
>    )
> 
>  {
> 
>    if (NULL == DevicePath) {
> 
> @@ -307,24 +307,29 @@ PerformSingleMappingDisplay (
>    IN CONST EFI_HANDLE  Handle
> 
>    )
> 
>  {
> 
> -  EFI_DEVICE_PATH_PROTOCOL  *DevPath;
> 
> -  EFI_DEVICE_PATH_PROTOCOL  *DevPathCopy;
> 
> -  CONST CHAR16              *MapList;
> 
> -  CHAR16                    *CurrentName;
> 
> -  CHAR16                    *MediaType;
> 
> -  CHAR16                    *DevPathString;
> 
> -  CHAR16                    *TempSpot;
> 
> -  CHAR16                    *Alias;
> 
> -  UINTN                     TempLen;
> 
> -  BOOLEAN                   Removable;
> 
> -  CONST CHAR16              *TempSpot2;
> 
> +  CONST EFI_DEVICE_PATH_PROTOCOL  *DevPath;
> 
> +  VOID                            *DevPathBuf;
> 
> +  EFI_DEVICE_PATH_PROTOCOL        *DevPathCopy;
> 
> +  CONST CHAR16                    *MapList;
> 
> +  CHAR16                          *CurrentName;
> 
> +  CHAR16                          *MediaType;
> 
> +  CHAR16                          *DevPathString;
> 
> +  CHAR16                          *TempSpot;
> 
> +  CHAR16                          *Alias;
> 
> +  UINTN                           TempLen;
> 
> +  BOOLEAN                         Removable;
> 
> +  CONST CHAR16                    *TempSpot2;
> 
> 
> 
>    Alias       = NULL;
> 
>    TempSpot2   = NULL;
> 
>    CurrentName = NULL;
> 
>    DevPath     = DevicePathFromHandle (Handle);
> 
> -  DevPathCopy = DevPath;
> 
> +  DevPathBuf  = DuplicateDevicePath (DevPath);
> 
> +  DevPathCopy = DevPathBuf;
> 
>    MapList     = gEfiShellProtocol->GetMapFromDevicePath (&DevPathCopy);
> 
> +
> 
> +  SHELL_FREE_NON_NULL (DevPathBuf);
> 
> +
> 
>    if (MapList == NULL) {
> 
>      return EFI_NOT_FOUND;
> 
>    }
> 
> @@ -485,16 +490,20 @@ PerformSingleMappingDelete (
>    IN CONST EFI_HANDLE  Handle
> 
>    )
> 
>  {
> 
> -  EFI_DEVICE_PATH_PROTOCOL  *DevPath;
> 
> -  EFI_DEVICE_PATH_PROTOCOL  *DevPathCopy;
> 
> -  CONST CHAR16              *MapList;
> 
> -  CHAR16                    *CurrentName;
> 
> +  CONST EFI_DEVICE_PATH_PROTOCOL  *DevPath;
> 
> +  VOID                            *DevPathBuf;
> 
> +  EFI_DEVICE_PATH_PROTOCOL        *DevPathCopy;
> 
> +  CONST CHAR16                    *MapList;
> 
> +  CHAR16                          *CurrentName;
> 
> 
> 
>    DevPath     = DevicePathFromHandle (Handle);
> 
> -  DevPathCopy = DevPath;
> 
> +  DevPathBuf  = DuplicateDevicePath (DevPath);
> 
> +  DevPathCopy = DevPathBuf;
> 
>    MapList     = gEfiShellProtocol->GetMapFromDevicePath (&DevPathCopy);
> 
>    CurrentName = NULL;
> 
> 
> 
> +  SHELL_FREE_NON_NULL (DevPathBuf);
> 
> +
> 
>    if (MapList == NULL) {
> 
>      return (EFI_NOT_FOUND);
> 
>    }
> 
> --
> 2.35.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#97090): https://edk2.groups.io/g/devel/message/97090
> Mute This Topic: https://groups.io/mt/95518373/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>