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

[Laszlo Ersek <lersek@redhat.com>]

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>