[PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Michal Januszewski]

From: Michal Januszewski <spock@gentoo.org>

Currently, it is possible to set a VESA graphics mode at boot time via
the vga= parameter even when no framebuffer driver supporting this is
configured.  This could lead to the system booting with a black screen,
without a usable console.

Fix this problem by only allowing to set graphics modes at boot time
if a supporting framebuffer driver is configured.

Signed-off-by: Michal Januszewski <spock@gentoo.org>
---
diff --git a/arch/x86/boot/video-vesa.c b/arch/x86/boot/video-vesa.c
index 401ad99..32a6045 100644
--- a/arch/x86/boot/video-vesa.c
+++ b/arch/x86/boot/video-vesa.c
@@ -88,14 +88,11 @@ static int vesa_probe(void)
 			   (vminfo.memory_layout == 4 ||
 			    vminfo.memory_layout == 6) &&
 			   vminfo.memory_planes == 1) {
-#ifdef CONFIG_FB
+#if FB_SUPPORTS_BOOT_VESA
 			/* Graphics mode, color, linear frame buffer
 			   supported.  Only register the mode if
 			   if framebuffer is configured, however,
-			   otherwise the user will be left without a screen.
-			   We don't require CONFIG_FB_VESA, however, since
-			   some of the other framebuffer drivers can use
-			   this mode-setting, too. */
+			   otherwise the user will be left without a screen. */
 			mi = GET_HEAP(struct mode_info, 1);
 			mi->mode = mode + VIDEO_FIRST_VESA;
 			mi->depth = vminfo.bpp;
@@ -134,9 +131,13 @@ static int vesa_set_mode(struct mode_info *mode)
 		/* It's a supported text mode */
 		is_graphic = 0;
 	} else if ((vminfo.mode_attr & 0x99) == 0x99) {
+#if FB_SUPPORTS_BOOT_VESA
 		/* It's a graphics mode with linear frame buffer */
 		is_graphic = 1;
 		vesa_mode |= 0x4000; /* Request linear frame buffer */
+#else
+		return -1;
+#endif
 	} else {
 		return -1;	/* Invalid mode */
 	}
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 1ee2c05..20fdc2f 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -76,6 +76,10 @@ extern struct screen_info screen_info;
 #define ORIG_VIDEO_LINES	(screen_info.orig_video_lines)
 #define ORIG_VIDEO_ISVGA	(screen_info.orig_video_isVGA)
 #define ORIG_VIDEO_POINTS       (screen_info.orig_video_points)
+
+#define FB_SUPPORTS_BOOT_VESA (defined(CONFIG_FB_VESA) || \
+		defined(CONFIG_FB_SIS) || defined(CONFIG_FB_IMAC) || \
+		defined(CONFIG_FB_INTEL))
 #endif /* __KERNEL__ */
 
 #endif /* _SCREEN_INFO_H */

Re: [PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Krzysztof Helt]

On Sun, 21 Sep 2008 23:02:19 +0200
Michal Januszewski <spock@gentoo.org> wrote:

> From: Michal Januszewski <spock@gentoo.org>
> 
> Currently, it is possible to set a VESA graphics mode at boot time via
> the vga= parameter even when no framebuffer driver supporting this is
> configured.  This could lead to the system booting with a black screen,
> without a usable console.
> 

Right.

> ---
> diff --git a/arch/x86/boot/video-vesa.c b/arch/x86/boot/video-vesa.c
> index 401ad99..32a6045 100644
> --- a/arch/x86/boot/video-vesa.c
> +++ b/arch/x86/boot/video-vesa.c
> @@ -134,9 +131,13 @@ static int vesa_set_mode(struct mode_info *mode)
>  		/* It's a supported text mode */
>  		is_graphic = 0;
>  	} else if ((vminfo.mode_attr & 0x99) == 0x99) {
> +#if FB_SUPPORTS_BOOT_VESA
>  		/* It's a graphics mode with linear frame buffer */
>  		is_graphic = 1;
>  		vesa_mode |= 0x4000; /* Request linear frame buffer */
> +#else
> +		return -1;
> +#endif
>  	} else {
>  		return -1;	/* Invalid mode */
>  	}


This one could be simpler:

  		/* It's a supported text mode */
  		is_graphic = 0;
 +#if FB_SUPPORTS_BOOT_VESA
  	} else if ((vminfo.mode_attr & 0x99) == 0x99) {
  		/* It's a graphics mode with linear frame buffer */
  		is_graphic = 1;
  		vesa_mode |= 0x4000; /* Request linear frame buffer */
 +#endif
  	} else {
  		return -1;	/* Invalid mode */
  	}


Other than this, the patch is fine to me.

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

Regards,
Krzysztof

----------------------------------------------------------------------
Kredyt Hipoteczny w Banku Millennium - zdobywca Zlotego Lauru Klienta!
Sprawdz >> http://link.interia.pl/f1f15

Re: [Linux-fbdev-devel] [PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Bruno Prémont]

From: Michal Januszewski <spock@gentoo.org>
> Currently, it is possible to set a VESA graphics mode at boot time
> via the vga= parameter even when no framebuffer driver supporting
> this is configured.  This could lead to the system booting with a
> black screen, without a usable console.
> 
This looks pretty much the same as what I requested some time ago:

http://bugzilla.kernel.org/show_bug.cgi?id=8062

Would need to check -mm archives for the actual patch.

Don't remember why it did not make it to mainline...

Bruno

Re: [Linux-fbdev-devel] [PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Andrew Morton]

On Mon, 22 Sep 2008 22:00:54 +0200
Bruno Prémont <bonbons@linux-vserver.org> wrote:

> From: Michal Januszewski <spock@gentoo.org>
> > Currently, it is possible to set a VESA graphics mode at boot time
> > via the vga= parameter even when no framebuffer driver supporting
> > this is configured.  This could lead to the system booting with a
> > black screen, without a usable console.
> > 
> This looks pretty much the same as what I requested some time ago:
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=8062
> 
> Would need to check -mm archives for the actual patch.
> 
> Don't remember why it did not make it to mainline...

If someone can give me some idea of what that patch looked
like then I can go hunt it down, find out why it didn't make it.

Re: [Linux-fbdev-devel] [PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Michal Januszewski]

On Mon, Sep 22, 2008 at 22:00, Bruno Prémont <bonbons@linux-vserver.org> wrote:

> This looks pretty much the same as what I requested some time ago:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=8062
>
> Would need to check -mm archives for the actual patch.
>
> Don't remember why it did not make it to mainline...

Well, the bug and the mm fix [1] are about a similar, yet slightly different
issue than the one I'm trying to fix with my patch (note that the issue
reported in the bug is currently fixed in the kernel, but the actual fix
is differrent than the one in -mm due to x86 boot code refactoring
that took place since then).

My patch fixes the case where the framebuffer is enabled, but none of
the drivers support boot-time video mode setting, whereas the old
patch fixed the case where it was possible to set a video mode even
though the kernel didn't contain the framebuffer layer.

[1] http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc2/2.6.21-rc2-mm1/broken-out/fbdev-ignore-vesa-modes-if-framebuffer-is-disabled.patch

Best regards,
-- 
Michal Januszewski, Gentoo Linux Developer
http://people.gentoo.org/spock

Re: [Linux-fbdev-devel] [PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Andrew Morton]

On Mon, 22 Sep 2008 22:32:40 +0200
"Michal Januszewski" <michalj@gmail.com> wrote:

> On Mon, Sep 22, 2008 at 22:00, Bruno Pr__mont <bonbons@linux-vserver.org> wrote:
> 
> > This looks pretty much the same as what I requested some time ago:
> >
> > http://bugzilla.kernel.org/show_bug.cgi?id=8062
> >
> > Would need to check -mm archives for the actual patch.
> >
> > Don't remember why it did not make it to mainline...
> 
> Well, the bug and the mm fix [1] are about a similar, yet slightly different
> issue than the one I'm trying to fix with my patch (note that the issue
> reported in the bug is currently fixed in the kernel, but the actual fix
> is differrent than the one in -mm due to x86 boot code refactoring
> that took place since then).
> 
> My patch fixes the case where the framebuffer is enabled, but none of
> the drivers support boot-time video mode setting, whereas the old
> patch fixed the case where it was possible to set a video mode even
> though the kernel didn't contain the framebuffer layer.
> 
> [1] http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc2/2.6.21-rc2-mm1/broken-out/fbdev-ignore-vesa-modes-if-framebuffer-is-disabled.patch
> 

OK.  It was merged into mainline but was later reverted by Linus:


commit 01e73be3c8f254ef19d787f9b6757468175267eb
Author: Linus Torvalds <torvalds@woody.linux-foundation.org>
Date:   Tue May 8 20:12:30 2007 -0700

    Revert "fbdev: ignore VESA modes if framebuffer is disabled"
    
    This reverts commit 464bdd33e9baad9806c7adbd8dfc37081a55f27e.
    
    Peter Anvin correctly points out that VESA modes have nothing to do with
    frame buffers per se - they are often just regular extended text modes.
    Disabling them just because we don't have frame buffer support is very
    wrong.
    
    Cc: H. Peter Anvin <hpa@zytor.com>
    Cc: Antonino A. Daplas <adaplas@gmail.com>,
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Re: [PATCH] fbdev: don't allow to set a video mode via vga= if FB doesn't support it

[Michal Januszewski]

On Mon, Sep 22, 2008 at 21:47, Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:

> This one could be simpler:
>
>                /* It's a supported text mode */
>                is_graphic = 0;
>  +#if FB_SUPPORTS_BOOT_VESA
>        } else if ((vminfo.mode_attr & 0x99) == 0x99) {
>                /* It's a graphics mode with linear frame buffer */
>                is_graphic = 1;
>                vesa_mode |= 0x4000; /* Request linear frame buffer */
>  +#endif
>        } else {
>                return -1;      /* Invalid mode */
>        }

Agreed.

I'll wait at least one more day for any additional comments and repost
a fixed version.

Thanks for doing the code review.
-- 
Michal Januszewski, Gentoo Linux Developer
http://people.gentoo.org/spock