2.6.7 fbcon: set_con2fb on current console = crash

2.6.7 fbcon: set_con2fb on current console = crash

[Jakub Bogusz]

[-- Attachment #1: Type: text/plain, Size: 980 bytes --]

After upgrade from 2.6.4 to 2.6.7 I noticed that calling set_con2fb
(through FBIOPUT_CON2FBMAP ioctl) on current console (already attached
to fb using this ioctl) causes crash (oops and then recursive oops when
trying to printk on console) and makes console unusable.

That's because take_over_console() calls fbcon_deinit(vc_num)
(which calls fbcon_free_font() on that console display) and then
fbcon_init(vc_num, ...), which copies font data from current fb console.
If current console was just deinit()ed, its fontdata is NULL - and this
pointer is "copied" to the same place, leaving current console with
fontdata==NULL (which leads to oops on nearest putc/putcs).

Attached patch restores 2.6.4 behaviour on set_con2fb (to set font if
it's not set already) - but it's not perfect solution as user font is
still lost (unline on 2.4.x kernels).
Any idea how to preserve user font on set_con2fb() called on current
console?


-- 
Jakub Bogusz    http://cyber.cs.net.pl/~qboosh/

[-- Attachment #2: linux-fbcon-con2fb-crash-workaround.patch --]
[-- Type: text/plain, Size: 707 bytes --]

--- linux-2.6.7/drivers/video/console/fbcon.c.orig	2004-06-16 18:24:04.000000000 +0200
+++ linux-2.6.7/drivers/video/console/fbcon.c	2004-06-18 23:12:35.000000000 +0200
@@ -617,6 +617,15 @@
 	if (p->userfont) {
 		REFCOUNT(p->fontdata)++;
 		charcnt = FNTCHARCNT(p->fontdata);
+	} else if(!p->fontdata) {
+		struct font_desc *font = NULL;
+		if (!fontname[0] || !(font = find_font(fontname)))
+			font = get_default_font(info->var.xres,
+						   info->var.yres);
+		vc->vc_font.width = font->width;
+		vc->vc_font.height = font->height;
+		vc->vc_font.data = p->fontdata = font->data;
+		vc->vc_font.charcount = 256; /* FIXME  Need to support more fonts */
 	}
 	con_copy_unimap(vc->vc_num, display_fg);
 

Re: 2.6.7 fbcon: set_con2fb on current console = crash

[Antonino A. Daplas]

On Saturday 19 June 2004 05:50, Jakub Bogusz wrote:
> After upgrade from 2.6.4 to 2.6.7 I noticed that calling set_con2fb
> (through FBIOPUT_CON2FBMAP ioctl) on current console (already attached
> to fb using this ioctl) causes crash (oops and then recursive oops when
> trying to printk on console) and makes console unusable.
>
> That's because take_over_console() calls fbcon_deinit(vc_num)
> (which calls fbcon_free_font() on that console display) and then
> fbcon_init(vc_num, ...), which copies font data from current fb console.
> If current console was just deinit()ed, its fontdata is NULL - and this
> pointer is "copied" to the same place, leaving current console with
> fontdata==NULL (which leads to oops on nearest putc/putcs).
>
> Attached patch restores 2.6.4 behaviour on set_con2fb (to set font if
> it's not set already) - but it's not perfect solution as user font is
> still lost (unline on 2.4.x kernels).
> Any idea how to preserve user font on set_con2fb() called on current
> console?

Thanks.  Actually there's still a critical flaw in the set_con2fbmap code.  
For one, con2fb_map is never initialized.  It's just fortunate that this 
array happens to be  filled with zeroes so con2fb_map[n] will always return 
zero and registered_fb[0] happens to contain a valid info.  So it works, by 
accident. 

Secondly, if you load fbdev1, load fbdev2, unload fbdev1, load fbcon, the 
console will freeze. This is because fbdev1, which is originally in 
registered_fb[0], is now unloaded, and fbdev2, which is in registered_fb[1] 
is still loaded.  However, fbcon looks at registered_fb[0] during init.

Also, I really don't like the take_over_console part in set_con2fbmap.  Too 
many unknowns. 

There are still lot more problems which I won't mention. I'll try to fix some 
of them over this weekend.

Tony




-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: 2.6.7 fbcon: set_con2fb on current console = crash

[Geert Uytterhoeven]

On Sat, 19 Jun 2004, Antonino A. Daplas wrote:
> Thanks.  Actually there's still a critical flaw in the set_con2fbmap code.
> For one, con2fb_map is never initialized.  It's just fortunate that this
> array happens to be  filled with zeroes so con2fb_map[n] will always return
> zero and registered_fb[0] happens to contain a valid info.  So it works, by
> accident.

According to the C standard, global variables are initialized to zero, unless
specified otherwise.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: 2.6.7 fbcon: set_con2fb on current console = crash

[Antonino A. Daplas]

On Saturday 19 June 2004 17:28, Geert Uytterhoeven wrote:
> On Sat, 19 Jun 2004, Antonino A. Daplas wrote:
> > Thanks.  Actually there's still a critical flaw in the set_con2fbmap
> > code. For one, con2fb_map is never initialized.  It's just fortunate that
> > this array happens to be  filled with zeroes so con2fb_map[n] will always
> > return zero and registered_fb[0] happens to contain a valid info.  So it
> > works, by accident.
>
> According to the C standard, global variables are initialized to zero,
> unless specified otherwise.
>

I know, but what I meant was con2fb_map[] is never initialized by fbcon.  So 
if the first valid fbdev is in registered_fb[1], then con2fbmap must be 
initialized to 1's by fbcon.  It doesn't.  Note, fbdev-2.4 does the 
initialization correctly.

So 2 critical flaws in the code:

1. con2fb_map[] is always zero-set
2. fbcon assumes that registered_fb[0] is always valid

Both flaws will manifest by doing this:

modprobe fbdev1 - in registered_fb[0]
modprobe fbdev2 - in registered_fb[1]
rmmod fbdev1      - registered_fb[0] becomes invalid
modprobe fbcon  
	 - con2fb_map[] with zeroes instead of 1's and fbcon_startup looks for 
fb_info in registered_fb[0] instead of registered_fb[1]

Tony




-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND