efifb: ignore frame buffer with invalid configuration

[PATCH v2] efifb: ignore frame buffer with invalid configuration

Posted by Roger Pau Monne 1 year, 5 months ago

On one of my boxes when the HDMI cable is not plugged in the
FrameBufferBase of the EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE structure is
set to 0 by the firmware (while some of the other fields looking
plausible).

Such (bogus address) ends up mapped in vesa_init(), and since it
overlaps with a RAM region the whole system goes down pretty badly,
see:

(XEN) vesafb: framebuffer at 0x0000000000000000, mapped to 0xffff82c000201000, using 35209k, total 35209k
(XEN) vesafb: mode is 0x37557x32, linelength=960, font 8x16
(XEN) vesafb: Truecolor: size=8:8:8:8, shift=24:0:8:16
(XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) �ERROR: Class:0; Subclass:0; Operation: 0
ERROR: No ConOut
ERROR: No ConIn

Do like Linux and prevent using the EFI Frame Buffer if the base
address is 0.  This is inline with the logic in Linuxes
fb_base_is_valid() function at drivers/video/fbdev/efifb.c v6.0.9.

See also Linux commit 133bb070e94ab41d750c6f2160c8843e46f11b78 for
further reference.

Also prevent using Frame Buffers that have a 0 height or width, as
those are also invalid.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Print a message.
 - Also refuse configurations with height or width == 0.
---
 xen/arch/x86/efi/efi-boot.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index e82ac9daa7..27f928ed3c 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -506,6 +506,13 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
 #ifdef CONFIG_VIDEO
     int bpp = 0;
 
+    if ( !gop->Mode->FrameBufferBase || !mode_info->HorizontalResolution ||
+         !mode_info->VerticalResolution )
+    {
+        PrintErr(L"Invalid Frame Buffer configuration found\r\n");
+        return;
+    }
+
     switch ( mode_info->PixelFormat )
     {
     case PixelRedGreenBlueReserved8BitPerColor:
-- 
2.37.3

Re: [PATCH v2] efifb: ignore frame buffer with invalid configuration

Posted by Jan Beulich 1 year, 5 months ago

On 18.11.2022 15:11, Roger Pau Monne wrote:
> On one of my boxes when the HDMI cable is not plugged in the
> FrameBufferBase of the EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE structure is
> set to 0 by the firmware (while some of the other fields looking
> plausible).
> 
> Such (bogus address) ends up mapped in vesa_init(), and since it
> overlaps with a RAM region the whole system goes down pretty badly,
> see:
> 
> (XEN) vesafb: framebuffer at 0x0000000000000000, mapped to 0xffff82c000201000, using 35209k, total 35209k
> (XEN) vesafb: mode is 0x37557x32, linelength=960, font 8x16
> (XEN) vesafb: Truecolor: size=8:8:8:8, shift=24:0:8:16
> (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) �ERROR: Class:0; Subclass:0; Operation: 0
> ERROR: No ConOut
> ERROR: No ConIn
> 
> Do like Linux and prevent using the EFI Frame Buffer if the base
> address is 0.  This is inline with the logic in Linuxes
> fb_base_is_valid() function at drivers/video/fbdev/efifb.c v6.0.9.
> 
> See also Linux commit 133bb070e94ab41d750c6f2160c8843e46f11b78 for
> further reference.
> 
> Also prevent using Frame Buffers that have a 0 height or width, as
> those are also invalid.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

RE: [PATCH v2] efifb: ignore frame buffer with invalid configuration

Posted by Henry Wang 1 year, 5 months ago

Hi Roger and Jan,

> -----Original Message-----
> Subject: Re: [PATCH v2] efifb: ignore frame buffer with invalid configuration
> 
> On 18.11.2022 15:11, Roger Pau Monne wrote:
> > On one of my boxes when the HDMI cable is not plugged in the
> > FrameBufferBase of the EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE
> structure is
> > set to 0 by the firmware (while some of the other fields looking
> > plausible).
> >
> > Such (bogus address) ends up mapped in vesa_init(), and since it
> > overlaps with a RAM region the whole system goes down pretty badly,
> > see:
> >
> > (XEN) vesafb: framebuffer at 0x0000000000000000, mapped to
> 0xffff82c000201000, using 35209k, total 35209k
> > (XEN) vesafb: mode is 0x37557x32, linelength=960, font 8x16
> > (XEN) vesafb: Truecolor: size=8:8:8:8, shift=24:0:8:16
> > (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) �ERROR: Class:0;
> Subclass:0; Operation: 0
> > ERROR: No ConOut
> > ERROR: No ConIn
> >
> > Do like Linux and prevent using the EFI Frame Buffer if the base
> > address is 0.  This is inline with the logic in Linuxes
> > fb_base_is_valid() function at drivers/video/fbdev/efifb.c v6.0.9.
> >
> > See also Linux commit 133bb070e94ab41d750c6f2160c8843e46f11b78 for
> > further reference.
> >
> > Also prevent using Frame Buffers that have a 0 height or width, as
> > those are also invalid.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

I saw the IRC/Matrix message from both of you. Since Jan mentioned
that this patch will be backported anyway and this is a bugfix patch
for safety of the code. I am giving my release-ack to reduce the
maintenance/backport burden for committers.

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry