Re: [PATCH 2/2] video: vga16fb: Only probe for EGA and VGA 16 color graphic cards

[Geert Uytterhoeven]

Hi Javier,

On Fri, Jan 7, 2022 at 9:00 PM Javier Martinez Canillas
<javierm@xxxxxxxxxx> wrote:
> The vga16fb framebuffer driver only supports Enhanced Graphics Adapter
> (EGA) and Video Graphics Array (VGA) 16 color graphic cards.
>
> But it doesn't check if the adapter is one of those or if a VGA16 mode
> is used. This means that the driver will be probed even if a VESA BIOS
> Extensions (VBE) or Graphics Output Protocol (GOP) interface is used.
>
> This issue has been present for a long time but it was only exposed by
> commit d391c5827107 ("drivers/firmware: move x86 Generic System
> Framebuffers support") since the platform device registration to match
> the {vesa,efi}fb drivers is done later as a consequence of that change.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215001
> Fixes: d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support")
> Reported-by: Kris Karas <bugs-a21@xxxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # 5.15.x
> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> Tested-by: Kris Karas <bugs-a21@xxxxxxxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/video/fbdev/vga16fb.c
> +++ b/drivers/video/fbdev/vga16fb.c
> @@ -1422,6 +1422,18 @@ static int __init vga16fb_init(void)
>
>         vga16fb_setup(option);
>  #endif
> +
> +       /* only EGA and VGA in 16 color graphic mode are supported */
> +       if (screen_info.orig_video_isVGA != VIDEO_TYPE_EGAC &&
> +           screen_info.orig_video_isVGA != VIDEO_TYPE_VGAC)
> +               return -ENODEV;

Probably these checks should be wrapped inside a check for CONFIG_X86?

All non-x86 architectures (except for 2 MIPS platforms) treat
orig_video_isVGA as a boolean flag, and just assign 1 to it.

> +
> +       if (screen_info.orig_video_mode != 0x0D &&      /* 320x200/4 (EGA) */
> +           screen_info.orig_video_mode != 0x0E &&      /* 640x200/4 (EGA) */
> +           screen_info.orig_video_mode != 0x10 &&      /* 640x350/4 (EGA) */
> +           screen_info.orig_video_mode != 0x12)        /* 640x480/4 (VGA) */
> +               return -ENODEV;
> +

Likewise.

A long time ago, I used vga16fb on a PPC box to use a standard PC
graphics card (initialized using an emulator for the card's BIOS ROM),
as a second display. The above changes would break such a use case.

>         ret = platform_driver_register(&vga16fb_driver);
>
>         if (!ret) {

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds