[edk2-devel] [PATCH v1] OvmfPkg/X86QemuLoadImageLib: Handle allocation failure for CommandLine

[Lendacky, Thomas]

On 3/18/21 4:44 PM, Martin Radev wrote:
> The CommandLine and InitrdData may be set to NULL if the provided
> size is too large. Because the zero page is mapped, this would not
> cause an immediate crash but can lead to memory corruption instead.
> This patch just adds validation and returns error if either allocation
> has failed.
> 
> Ref: https://github.com/martinradev/edk2/commit/6c0ce748b97393240c006e24b73652f30e597a05
> 
> Signed-off-by: Martin Radev <martin.b.radev at gmail.com>

Looks good to me. The two other LoadLinuxAllocate...() calls check for
NULL, so it's reasonable that these should as well.

Acked-by: Tom Lendacky <thomas.lendacky at amd.com>

> ---
>  OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> index 931553c0c1..b983c4d7d0 100644
> --- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> +++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> @@ -161,6 +161,12 @@ QemuLoadLegacyImage (
>      LoadedImage->CommandLine = LoadLinuxAllocateCommandLinePages (
>                                   EFI_SIZE_TO_PAGES (
>                                     LoadedImage->CommandLineSize));
> +
> +    if (LoadedImage->CommandLine == NULL) {
> +      DEBUG ((DEBUG_ERROR, "Unable to allocate memory for kernel command line!\n"));
> +      Status = EFI_OUT_OF_RESOURCES;
> +      goto FreeImage;
> +    }
>      QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData);
>      QemuFwCfgReadBytes (LoadedImage->CommandLineSize, LoadedImage->CommandLine);
>    }
> @@ -178,6 +184,11 @@ QemuLoadLegacyImage (
>      LoadedImage->InitrdData = LoadLinuxAllocateInitrdPages (
>                                  LoadedImage->SetupBuf,
>                                  EFI_SIZE_TO_PAGES (LoadedImage->InitrdSize));
> +    if (LoadedImage->InitrdData == NULL) {
> +      DEBUG ((DEBUG_ERROR, "Unable to allocate memory for initrd!\n"));
> +      Status = EFI_OUT_OF_RESOURCES;
> +      goto FreeImage;
> +    }
>      DEBUG ((DEBUG_INFO, "Initrd size: 0x%x\n",
>        (UINT32)LoadedImage->InitrdSize));
>      DEBUG ((DEBUG_INFO, "Reading initrd image ..."));
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73046): https://edk2.groups.io/g/devel/message/73046
Mute This Topic: https://groups.io/mt/81445621/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-