[edk2] ArmPkg: fix booting with partial paths

Message ID 1398867079-4684-1-git-send-email-msalter@redhat.com
State New
Headers show

Commit Message

Mark Salter April 30, 2014, 2:11 p.m.
Boot entries created by efibootmgr may contain a partial device path
to the EFI application to boot. These entries begin with a partition
device path whereas entries created via ARM Boot Manager contain a
full path to the EFI application. The ARM BDS code will fill in the
missing parts of this partial device path as it does for removeable
device paths. This allows the application to be loaded and started.
However, the current code passes the original partial device path to
gBS->LoadImage() and thus LoadImage is unable to find a DeviceHandle
for the path. This means the application being booted cannot find the
boot device from the Loaded Image Protocol structure. In the case of
grub, this prevents the grub config file from being found. This patch
fixes this by making sure the full path is propagated back to the
caller of gBS->LoadImage() so that a proper DeviceHandle gets passed
to the application being booted.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Mark Salter <msalter@redhat.com>
---
 ArmPkg/Library/BdsLib/BdsFilePath.c | 77 ++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 27 deletions(-)

Comments

Olivier Martin May 8, 2014, 3:10 p.m. | #1
Thanks Mark for the contribution,
I have just committed your change in SVN rev 15518 after fixing the build
(your patch introduced a couple of build errors).

> -----Original Message-----
> From: Mark Salter [mailto:msalter@redhat.com]
> Sent: 30 April 2014 15:11
> To: edk2-devel@lists.sourceforge.net
> Subject: [edk2] [PATCH] ArmPkg: fix booting with partial paths
> 
> Boot entries created by efibootmgr may contain a partial device path
> to the EFI application to boot. These entries begin with a partition
> device path whereas entries created via ARM Boot Manager contain a
> full path to the EFI application. The ARM BDS code will fill in the
> missing parts of this partial device path as it does for removeable
> device paths. This allows the application to be loaded and started.
> However, the current code passes the original partial device path to
> gBS->LoadImage() and thus LoadImage is unable to find a DeviceHandle
> for the path. This means the application being booted cannot find the
> boot device from the Loaded Image Protocol structure. In the case of
> grub, this prevents the grub config file from being found. This patch
> fixes this by making sure the full path is propagated back to the
> caller of gBS->LoadImage() so that a proper DeviceHandle gets passed
> to the application being booted.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Mark Salter <msalter@redhat.com>
> ---
>  ArmPkg/Library/BdsLib/BdsFilePath.c | 77 ++++++++++++++++++++++++-----
> --------
>  1 file changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c
> b/ArmPkg/Library/BdsLib/BdsFilePath.c
> index 25f9272..2b65d01 100644
> --- a/ArmPkg/Library/BdsLib/BdsFilePath.c
> +++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
> @@ -300,35 +300,24 @@ TryRemovableDevice (
>    return Status;
>  }
> 
> -/**
> -  Connect a Device Path and return the handle of the driver that
> support this DevicePath
> -
> -  @param  DevicePath            Device Path of the File to connect
> -  @param  Handle                Handle of the driver that support this
> DevicePath
> -  @param  RemainingDevicePath   Remaining DevicePath nodes that do not
> match the driver DevicePath
> -
> -  @retval EFI_SUCCESS           A driver that matches the Device Path
> has been found
> -  @retval EFI_NOT_FOUND         No handles match the search.
> -  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
> -
> -**/
> +STATIC
>  EFI_STATUS
> -BdsConnectDevicePath (
> -  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
> -  OUT EFI_HANDLE                *Handle,
> -  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
> +BdsConnectAndUpdateDevicePath (
> +  IN OUT EFI_DEVICE_PATH_PROTOCOL  **DevicePath,
> +  OUT    EFI_HANDLE                *Handle,
> +  OUT    EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
>    )
>  {
>    EFI_DEVICE_PATH*            Remaining;
>    EFI_DEVICE_PATH*            NewDevicePath;
>    EFI_STATUS                  Status;
> 
> -  if ((DevicePath == NULL) || (Handle == NULL)) {
> +  if ((DevicePath == NULL) || (*DevicePath == NULL) || (Handle ==
> NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
>    do {
> -    Remaining = DevicePath;
> +    Remaining = *DevicePath;
>      // The LocateDevicePath() function locates all devices on
> DevicePath that support Protocol and returns
>      // the handle to the device that is closest to DevicePath. On
> output, the device path pointer is modified
>      // to point to the remaining part of the device path
> @@ -348,7 +337,7 @@ BdsConnectDevicePath (
>    if (!EFI_ERROR (Status)) {
>      // Now, we have got the whole Device Path connected, call again
> ConnectController to ensure all the supported Driver
>      // Binding Protocol are connected (such as DiskIo and
> SimpleFileSystem)
> -    Remaining = DevicePath;
> +    Remaining = *DevicePath;
>      Status = gBS->LocateDevicePath (&gEfiDevicePathProtocolGuid,
> &Remaining, Handle);
>      if (!EFI_ERROR (Status)) {
>        Status = gBS->ConnectController (*Handle, NULL, Remaining,
> FALSE);
> @@ -371,9 +360,11 @@ BdsConnectDevicePath (
>      //TODO: Should we just return success and leave the caller decide
> if it is the expected RemainingPath
>      Status = EFI_SUCCESS;
>    } else {
> -    Status = TryRemovableDevice (DevicePath, Handle, &NewDevicePath);
> +    Status = TryRemovableDevice (*DevicePath, Handle, &NewDevicePath);
>      if (!EFI_ERROR (Status)) {
> -      return BdsConnectDevicePath (NewDevicePath, Handle,
> RemainingDevicePath);
> +      Status = BdsConnectAndUpdateDevicePath (&NewDevicePath, Handle,
> RemainingDevicePath);
> +      *DevicePath = NewDevicePath;
> +      return Status;
>      }
>    }
> 
> @@ -384,6 +375,27 @@ BdsConnectDevicePath (
>    return Status;
>  }
> 
> +/**
> +  Connect a Device Path and return the handle of the driver that
> support this DevicePath
> +
> +  @param  DevicePath            Device Path of the File to connect
> +  @param  Handle                Handle of the driver that support this
> DevicePath
> +  @param  RemainingDevicePath   Remaining DevicePath nodes that do not
> match the driver DevicePath
> +
> +  @retval EFI_SUCCESS           A driver that matches the Device Path
> has been found
> +  @retval EFI_NOT_FOUND         No handles match the search.
> +  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
> +
> +**/
> +BdsConnectDevicePath (
> +  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
> +  OUT EFI_HANDLE                *Handle,
> +  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
> +  )
> +{
> +  return BdsConnectAndUpdateDevicePath(&DevicePath, Handle,
> RemainingDevicePath);
> +}
> +
>  BOOLEAN
>  BdsFileSystemSupport (
>    IN EFI_DEVICE_PATH *DevicePath,
> @@ -866,8 +878,8 @@ BDS_FILE_LOADER FileLoaders[] = {
>  };
> 
>  EFI_STATUS
> -BdsLoadImage (
> -  IN     EFI_DEVICE_PATH       *DevicePath,
> +BdsLoadImageAndUpdateDevicePath (
> +  IN OUT EFI_DEVICE_PATH       **DevicePath,
>    IN     EFI_ALLOCATE_TYPE     Type,
>    IN OUT EFI_PHYSICAL_ADDRESS* Image,
>    OUT    UINTN                 *FileSize
> @@ -878,15 +890,15 @@ BdsLoadImage (
>    EFI_DEVICE_PATH *RemainingDevicePath;
>    BDS_FILE_LOADER*  FileLoader;
> 
> -  Status = BdsConnectDevicePath (DevicePath, &Handle,
> &RemainingDevicePath);
> +  Status = BdsConnectAndUpdateDevicePath (DevicePath, &Handle,
> &RemainingDevicePath);
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> 
>    FileLoader = FileLoaders;
>    while (FileLoader->Support != NULL) {
> -    if (FileLoader->Support (DevicePath, Handle, RemainingDevicePath))
> {
> -      return FileLoader->LoadImage (DevicePath, Handle,
> RemainingDevicePath, Type, Image, FileSize);
> +    if (FileLoader->Support (*DevicePath, Handle,
> RemainingDevicePath)) {
> +      return FileLoader->LoadImage (*DevicePath, Handle,
> RemainingDevicePath, Type, Image, FileSize);
>      }
>      FileLoader++;
>    }
> @@ -894,6 +906,17 @@ BdsLoadImage (
>    return EFI_UNSUPPORTED;
>  }
> 
> +BdsLoadImage (
> +  IN     EFI_DEVICE_PATH       *DevicePath,
> +  IN     EFI_DEVICE_PATH       *DevicePath,
> +  IN     EFI_ALLOCATE_TYPE     Type,
> +  IN OUT EFI_PHYSICAL_ADDRESS* Image,
> +  OUT    UINTN                 *FileSize
> +  )
> +{
> +  return BdsLoadImageAndUpdateDevicePath (&DevicePath, Type, Image,
> FileSize);
> +}
> +
>  /**
>    Start an EFI Application from a Device Path
> 
> @@ -920,7 +943,7 @@ BdsStartEfiApplication (
>    EFI_LOADED_IMAGE_PROTOCOL*   LoadedImage;
> 
>    // Find the nearest supported file loader
> -  Status = BdsLoadImage (DevicePath, AllocateAnyPages, &BinaryBuffer,
> &BinarySize);
> +  Status = BdsLoadImageAndUpdateDevicePath (&DevicePath,
> AllocateAnyPages, &BinaryBuffer, &BinarySize);
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> --
> 1.9.0
> 
> 
> -----------------------------------------------------------------------
> -------
> "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
> Instantly run your Selenium tests across 300+ browser/OS combos.  Get
> unparalleled scalability from the best Selenium testing platform
> available.
> Simple to use. Nothing to install. Get started now for free."
> http://p.sf.net/sfu/SauceLabs
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel





------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
&#149; 3 signs your SCM is hindering your productivity
&#149; Requirements for releasing software faster
&#149; Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce

Patch

diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c b/ArmPkg/Library/BdsLib/BdsFilePath.c
index 25f9272..2b65d01 100644
--- a/ArmPkg/Library/BdsLib/BdsFilePath.c
+++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
@@ -300,35 +300,24 @@  TryRemovableDevice (
   return Status;
 }
 
-/**
-  Connect a Device Path and return the handle of the driver that support this DevicePath
-
-  @param  DevicePath            Device Path of the File to connect
-  @param  Handle                Handle of the driver that support this DevicePath
-  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match the driver DevicePath
-
-  @retval EFI_SUCCESS           A driver that matches the Device Path has been found
-  @retval EFI_NOT_FOUND         No handles match the search.
-  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
-
-**/
+STATIC
 EFI_STATUS
-BdsConnectDevicePath (
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  OUT EFI_HANDLE                *Handle,
-  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
+BdsConnectAndUpdateDevicePath (
+  IN OUT EFI_DEVICE_PATH_PROTOCOL  **DevicePath,
+  OUT    EFI_HANDLE                *Handle,
+  OUT    EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
   )
 {
   EFI_DEVICE_PATH*            Remaining;
   EFI_DEVICE_PATH*            NewDevicePath;
   EFI_STATUS                  Status;
 
-  if ((DevicePath == NULL) || (Handle == NULL)) {
+  if ((DevicePath == NULL) || (*DevicePath == NULL) || (Handle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
   do {
-    Remaining = DevicePath;
+    Remaining = *DevicePath;
     // The LocateDevicePath() function locates all devices on DevicePath that support Protocol and returns
     // the handle to the device that is closest to DevicePath. On output, the device path pointer is modified
     // to point to the remaining part of the device path
@@ -348,7 +337,7 @@  BdsConnectDevicePath (
   if (!EFI_ERROR (Status)) {
     // Now, we have got the whole Device Path connected, call again ConnectController to ensure all the supported Driver
     // Binding Protocol are connected (such as DiskIo and SimpleFileSystem)
-    Remaining = DevicePath;
+    Remaining = *DevicePath;
     Status = gBS->LocateDevicePath (&gEfiDevicePathProtocolGuid, &Remaining, Handle);
     if (!EFI_ERROR (Status)) {
       Status = gBS->ConnectController (*Handle, NULL, Remaining, FALSE);
@@ -371,9 +360,11 @@  BdsConnectDevicePath (
     //TODO: Should we just return success and leave the caller decide if it is the expected RemainingPath
     Status = EFI_SUCCESS;
   } else {
-    Status = TryRemovableDevice (DevicePath, Handle, &NewDevicePath);
+    Status = TryRemovableDevice (*DevicePath, Handle, &NewDevicePath);
     if (!EFI_ERROR (Status)) {
-      return BdsConnectDevicePath (NewDevicePath, Handle, RemainingDevicePath);
+      Status = BdsConnectAndUpdateDevicePath (&NewDevicePath, Handle, RemainingDevicePath);
+      *DevicePath = NewDevicePath;
+      return Status;
     }
   }
 
@@ -384,6 +375,27 @@  BdsConnectDevicePath (
   return Status;
 }
 
+/**
+  Connect a Device Path and return the handle of the driver that support this DevicePath
+
+  @param  DevicePath            Device Path of the File to connect
+  @param  Handle                Handle of the driver that support this DevicePath
+  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match the driver DevicePath
+
+  @retval EFI_SUCCESS           A driver that matches the Device Path has been found
+  @retval EFI_NOT_FOUND         No handles match the search.
+  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
+
+**/
+BdsConnectDevicePath (
+  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
+  OUT EFI_HANDLE                *Handle,
+  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
+  )
+{
+  return BdsConnectAndUpdateDevicePath(&DevicePath, Handle, RemainingDevicePath);
+}
+
 BOOLEAN
 BdsFileSystemSupport (
   IN EFI_DEVICE_PATH *DevicePath,
@@ -866,8 +878,8 @@  BDS_FILE_LOADER FileLoaders[] = {
 };
 
 EFI_STATUS
-BdsLoadImage (
-  IN     EFI_DEVICE_PATH       *DevicePath,
+BdsLoadImageAndUpdateDevicePath (
+  IN OUT EFI_DEVICE_PATH       **DevicePath,
   IN     EFI_ALLOCATE_TYPE     Type,
   IN OUT EFI_PHYSICAL_ADDRESS* Image,
   OUT    UINTN                 *FileSize
@@ -878,15 +890,15 @@  BdsLoadImage (
   EFI_DEVICE_PATH *RemainingDevicePath;
   BDS_FILE_LOADER*  FileLoader;
 
-  Status = BdsConnectDevicePath (DevicePath, &Handle, &RemainingDevicePath);
+  Status = BdsConnectAndUpdateDevicePath (DevicePath, &Handle, &RemainingDevicePath);
   if (EFI_ERROR (Status)) {
     return Status;
   }
 
   FileLoader = FileLoaders;
   while (FileLoader->Support != NULL) {
-    if (FileLoader->Support (DevicePath, Handle, RemainingDevicePath)) {
-      return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, Type, Image, FileSize);
+    if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
+      return FileLoader->LoadImage (*DevicePath, Handle, RemainingDevicePath, Type, Image, FileSize);
     }
     FileLoader++;
   }
@@ -894,6 +906,17 @@  BdsLoadImage (
   return EFI_UNSUPPORTED;
 }
 
+BdsLoadImage (
+  IN     EFI_DEVICE_PATH       *DevicePath,
+  IN     EFI_DEVICE_PATH       *DevicePath,
+  IN     EFI_ALLOCATE_TYPE     Type,
+  IN OUT EFI_PHYSICAL_ADDRESS* Image,
+  OUT    UINTN                 *FileSize
+  )
+{
+  return BdsLoadImageAndUpdateDevicePath (&DevicePath, Type, Image, FileSize);
+}
+
 /**
   Start an EFI Application from a Device Path
 
@@ -920,7 +943,7 @@  BdsStartEfiApplication (
   EFI_LOADED_IMAGE_PROTOCOL*   LoadedImage;
 
   // Find the nearest supported file loader
-  Status = BdsLoadImage (DevicePath, AllocateAnyPages, &BinaryBuffer, &BinarySize);
+  Status = BdsLoadImageAndUpdateDevicePath (&DevicePath, AllocateAnyPages, &BinaryBuffer, &BinarySize);
   if (EFI_ERROR (Status)) {
     return Status;
   }