[edk2-devel] [PATCH v1 1/1] OvmfPkg/QemuVideoDxe: Zero out PixelInformation in QueryMode

[Samer El-Haj-Mahmoud]

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>


Any chance we can get this pushed?

Thanks,
--Samer


> -----Original Message-----
> From: Dimitrije Pavlov <Dimitrije.Pavlov at arm.com>
> Sent: Tuesday, June 28, 2022 2:48 PM
> To: devel at edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>; Jiewen Yao
> <jiewen.yao at intel.com>; Jordan Justen <jordan.l.justen at intel.com>;
> Gerd Hoffmann <kraxel at redhat.com>; Jeff Booher-Kaeding <Jeff.Booher-
> Kaeding at arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud at arm.com>; Sunny Wang <Sunny.Wang at arm.com>; Jeremy
> Linton <Jeremy.Linton at arm.com>
> Subject: [PATCH v1 1/1] OvmfPkg/QemuVideoDxe: Zero out
> PixelInformation in QueryMode
> 
> Ensure that the PixelInformation field of the
> EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is zeroed out in
> EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode() and
> EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() when PixelFormat is
> PixelBlueGreenRedReserved8BitPerColor.
> 
> According to UEFI 2.9 Section 12.9, PixelInformation field of the
> EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is valid only if
> PixelFormat is PixelBitMask. This means that firmware is not required
> to fill out the PixelInformation field for other PixelFormat types,
> which implies that the QemuVideoDxe implementation is technically
> correct.
> 
> However, not zeroing out those fields will leak the contents of the
> memory returned by the memory allocator, so it is better to explicitly
> set them to zero.
> 
> In addition, the SCT test suite relies on PixelInformation always
> having a consistent value, which causes failures.
> 
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Gerd Hoffmann <kraxel at redhat.com>
> Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding at arm.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>
> Cc: Sunny Wang <Sunny.Wang at arm.com>
> Cc: Jeremy Linton <Jeremy.Linton at arm.com>
> 
> Signed-off-by: Dimitrije Pavlov <Dimitrije.Pavlov at arm.com>
> ---
>  OvmfPkg/QemuVideoDxe/Gop.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c
> b/OvmfPkg/QemuVideoDxe/Gop.c
> index 0c4dea7fb6f2..7a9fe208c99c 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -31,7 +31,14 @@ QemuVideoCompleteModeInfo (
>      Info->PixelInformation.ReservedMask = 0;
>    } else if (ModeData->ColorDepth == 32) {
>      DEBUG ((DEBUG_INFO, "PixelBlueGreenRedReserved8BitPerColor\n"));
> -    Info->PixelFormat = PixelBlueGreenRedReserved8BitPerColor;
> +    Info->PixelFormat                   =
> PixelBlueGreenRedReserved8BitPerColor;
> +    Info->PixelInformation.RedMask      = 0;
> +    Info->PixelInformation.GreenMask    = 0;
> +    Info->PixelInformation.BlueMask     = 0;
> +    Info->PixelInformation.ReservedMask = 0;
> +  } else {
> +    DEBUG ((DEBUG_ERROR, "%a: Invalid ColorDepth %u", __FUNCTION__,
> ModeData->ColorDepth));
> +    ASSERT (FALSE);
>    }
> 
>    Info->PixelsPerScanLine = Info->HorizontalResolution;
> --
> 2.34.1



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