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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWvFL9GYM-dMdZeyZrQjr13Sgt-XXN19bc6jA3emn2Dcw@mail.gmail.com>
Date:   Sun, 9 Jan 2022 11:01:32 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Javier Martinez Canillas <javierm@...hat.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Kris Karas <bugs-a21@...nlit-rail.com>,
        stable <stable@...r.kernel.org>, Borislav Petkov <bp@...e.de>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>
Subject: Re: [PATCH 2/2] video: vga16fb: Only probe for EGA and VGA 16 color
 graphic cards

Hi Javier,

On Fri, Jan 7, 2022 at 9:00 PM Javier Martinez Canillas
<javierm@...hat.com> 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@...nlit-rail.com>
> Cc: <stable@...r.kernel.org> # 5.15.x
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> Tested-by: Kris Karas <bugs-a21@...nlit-rail.com>

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@...ux-m68k.org

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