[edk2,v2,edk2-platforms,3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths

Message ID 20181123084406.27192-4-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • Platform/ARM: fix DevicePath mishandling in BdsLib
Related show

Commit Message

Ard Biesheuvel Nov. 23, 2018, 8:44 a.m.
DevicePath node types may have any size, and so it is up to the
code that manipulates them to ensure that dereferencing them only
occurs when the pointer is aligned explicitly.

Since BdsConnectAndUpdateDevicePath() has only two callers, one of
which itself, we can simply duplicate the device path (similar to
how DxeCore's CoreConnectController () does it), and free the pool
allocation again on the way out. (Note that the allocation only
occurs when the non-recursive path is taken and the function
returns EFI_SUCCESS)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 Platform/ARM/Library/BdsLib/BdsFilePath.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.17.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Nov. 23, 2018, 12:46 p.m. | #1
On 11/23/18 09:44, Ard Biesheuvel wrote:
> DevicePath node types may have any size, and so it is up to the

> code that manipulates them to ensure that dereferencing them only

> occurs when the pointer is aligned explicitly.

> 

> Since BdsConnectAndUpdateDevicePath() has only two callers, one of

> which itself, we can simply duplicate the device path (similar to

> how DxeCore's CoreConnectController () does it), and free the pool

> allocation again on the way out. (Note that the allocation only

> occurs when the non-recursive path is taken and the function

> returns EFI_SUCCESS)

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  Platform/ARM/Library/BdsLib/BdsFilePath.c | 12 ++++++++----

>  1 file changed, 8 insertions(+), 4 deletions(-)

> 

> diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c b/Platform/ARM/Library/BdsLib/BdsFilePath.c

> index 62f796e5526d..ad66b2f82718 100644

> --- a/Platform/ARM/Library/BdsLib/BdsFilePath.c

> +++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c

> @@ -423,8 +423,8 @@ BdsConnectAndUpdateDevicePath (

>      }

>    }

>  

> -  if (RemainingDevicePath) {

> -    *RemainingDevicePath = Remaining;

> +  if (!EFI_ERROR (Status) && RemainingDevicePath != NULL) {

> +    *RemainingDevicePath = DuplicateDevicePath (Remaining);

>    }

>  

>    return Status;

> @@ -1314,14 +1314,18 @@ BdsLoadImageAndUpdateDevicePath (

>    }

>  

>    FileLoader = FileLoaders;

> +  Status = EFI_UNSUPPORTED;

>    while (FileLoader->Support != NULL) {

>      if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {

> -      return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, Type, Image, FileSize);

> +      Status = FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath,

> +                             Type, Image, FileSize);

> +      break;

>      }

>      FileLoader++;

>    }

>  

> -  return EFI_UNSUPPORTED;

> +  FreePool (RemainingDevicePath);

> +  return Status;

>  }

>  

>  EFI_STATUS

> 


Reviewed-by: Laszlo Ersek <lersek@redhat.com>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c b/Platform/ARM/Library/BdsLib/BdsFilePath.c
index 62f796e5526d..ad66b2f82718 100644
--- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
+++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
@@ -423,8 +423,8 @@  BdsConnectAndUpdateDevicePath (
     }
   }
 
-  if (RemainingDevicePath) {
-    *RemainingDevicePath = Remaining;
+  if (!EFI_ERROR (Status) && RemainingDevicePath != NULL) {
+    *RemainingDevicePath = DuplicateDevicePath (Remaining);
   }
 
   return Status;
@@ -1314,14 +1314,18 @@  BdsLoadImageAndUpdateDevicePath (
   }
 
   FileLoader = FileLoaders;
+  Status = EFI_UNSUPPORTED;
   while (FileLoader->Support != NULL) {
     if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
-      return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, Type, Image, FileSize);
+      Status = FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath,
+                             Type, Image, FileSize);
+      break;
     }
     FileLoader++;
   }
 
-  return EFI_UNSUPPORTED;
+  FreePool (RemainingDevicePath);
+  return Status;
 }
 
 EFI_STATUS