On 11/22/18 18:26, Ard Biesheuvel wrote:
> BdsLoadImage () is part of the BdsLib library API and is not documented
> as modifying its DevicePath argument, but does so nonetheless. So take
> a copy instead, and free it after use.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> Platform/ARM/Library/BdsLib/BdsFilePath.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c
> b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> index 67dafa4f3651..74fdbbee773d 100644
> --- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
> +++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> @@ -1351,5 +1351,16 @@ BdsLoadImage (
> OUT UINTN *FileSize
> )
> {
> - return BdsLoadImageAndUpdateDevicePath (&DevicePath, Type, Image,
> FileSize);
> + EFI_DEVICE_PATH *Path;
> + EFI_STATUS Status;
> +
> + Path = DuplicateDevicePath (DevicePath);
> + if (Path == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
This introduces a minor change in behavior.
Previously, if BdsLoadImage() got DevicePath==NULL, then
BdsLoadImageAndUpdateDevicePath() -> BdsConnectAndUpdateDevicePath()
would hit (*DevicePath == NULL), and return EFI_INVALID_PARAMETER.
Now, (DevicePath==NULL) causes DuplicateDevicePath() to return NULL, and
we translate that to EFI_OUT_OF_RESOURCES.
Can you check for (DevicePath==NULL) first, and preserve
EFI_INVALID_PARAMETER?
> +
> + Status = BdsLoadImageAndUpdateDevicePath (&Path, Type, Image, FileSize);
> + FreePool (Path);
This is not safe; BdsLoadImageAndUpdateDevicePath() may change Path.
Namely, in BdsConnectAndUpdateDevicePath(), we have at one location,
*DevicePath = NewDevicePath;
... Which, in fact, makes me wonder whether we need this patch at all. I
believe BdsLoadImageAndUpdateDevicePath() -- and
BdsConnectAndUpdateDevicePath() -- are supposed to update the caller's
*pointer* to the device path, and not the pointed-to device path itself.
Do you agree?
Thanks,
Laszlo
> +
> + return Status;
> }
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
