fbdev: Fix crash if fb_set_var() called before register_framebuffer()
PATCH 1/1 fbdev: Fix crash if fb_set_var() called before register_framebuffer()
The field info->modelist is initialized during register_framebuffer. This field is also referred to in fb_set_var(). Thus a call to fb_set_var() before register_framebuffer() will cause a crash. A few drivers do this, notably controlfb. (This might fix reports of controlfb crashing in powermacs).
---
fbmem.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletion(-)
drivers/video/fbmem.c +4 -1
@@ -725,7 +725,10 @@fb_set_cmap (&info->cmap, info);fb_var_to_videomode (&mode, &info->var); -fb_add_videomode (&mode, &info->modelist); + +if (info->modelist.prev && info->modelist.next && + !list_empty (&info->modelist)) +fb_add_videomode (&mode, &info->modelist);if (info->flags &FBINFO_MISC_MODECHANGEUSER ) {struct fb_event event;
Quoted 6 lines
The field
info->modelistis initialized duringregister_framebuffer. This field is also referred to infb_set_var(). Thus a call tofb_set_var()beforeregister_framebuffer()will cause a crash. A few drivers do this, notably controlfb. (This might fix reports of controlfb crashing in powermacs).
this patch works well... I can now boot my PowerMac 7300 using 2.6.10-rc2-bk8 (that I got on kernel.org) with this patch...
but I still has some problem with my Matrox Millenium PCI card using matroxfb driver... the kernel boot... but I get corrupted characters on the console... X load ok and display ok...
It works really well with the onboard video (controlfb driver)
matroxfb driver works fine on 2.6.8
Quoted 8 lines · Mario Gaucher
Quoted 5 lines
The field
info->modelistis initialized duringregister_framebuffer. This field is also referred to infb_set_var(). Thus a call tofb_set_var()beforeregister_framebuffer()will cause a crash. A few drivers do this, notably controlfb. (This might fix reports of controlfb crashing in powermacs).this patch works well... I can now boot my PowerMac 7300 using 2.6.10-rc2-bk8 (that I got on kernel.org) with this patch...
That's good.
Quoted 4 lines
but I still has some problem with my Matrox Millenium PCI card using matroxfb driver... the kernel boot... but I get corrupted characters on the console... X load ok and display ok...
Try this first.
1. Open drivers/video/matrox/matrofb_accel.c
2. At the end of the file is this function:
static void matroxfb_imageblit(struct fb_info* info, const struct fb_image* image) {
MINFO_FROM_INFO(info);
DBG_HEAVY(__FUNCTION__);
if (image->depth == 1) {
u_int32_t fgx, bgx;
fgx = ((u_int32_t*)info->pseudo_palette)[image->fg_color];
bgx = ((u_int32_t*)info->pseudo_palette)[image->bg_color];
matroxfb_1bpp_imageblit(PMINFO fgx, bgx, image->data, image->width, image->height, image->dy, image->dx);
} else {
/* Danger! image->depth is useless: logo painting code always
passes framebuffer color depth here, although logo data are
always 8bpp and info->pseudo_palette is changed to contain
logo palette to be used (but only for true/direct-color... sic...).
So do it completely in software... */
cfb_imageblit(info, image);
}
}3. Replace the above function to use software drawing so it becomes like this:
static void matroxfb_imageblit(struct fb_info* info, const struct fb_image* image) {
cfb_imageblit(info, image); }
Tony
From: "Antonino A. Daplas" <[email protected]>
Quoted 10 lines
Quoted 3 lines
but I still has some problem with my Matrox Millenium PCI card using matroxfb driver... the kernel boot... but I get corrupted characters on the console... X load ok and display ok...
Try this first.
1. Open
drivers/video/matrox/matrofb_accel.c
2. At the end of the file is this function:static void matroxfb_imageblit(struct fb_info* info, const struct
fb_image* image) {
Quoted 10 lines
MINFO_FROM_INFO(info);
DBG_HEAVY(__FUNCTION__);
if (image->depth == 1) {
u_int32_t fgx, bgx;
fgx = ((u_int32_t*)info->pseudo_palette)[image->fg_color];
bgx = ((u_int32_t*)info->pseudo_palette)[image->bg_color];
matroxfb_1bpp_imageblit(PMINFO fgx, bgx, image->data, image->width,image->height, image->dy, image->dx);
Quoted 11 lines
} else { /* Danger! image->depth is useless: logo painting code always passes framebuffer color depth here, although logo data are always 8bpp and info->pseudo_palette is changed to contain logo palette to be used (but only for true/direct-color... sic...). So do it completely in software... */ cfb_imageblit(info, image); } }3. Replace the above function to use software drawing so it becomes like
this:
Quoted 2 lines
static void matroxfb_imageblit(struct fb_info* info, const struct
fb_image* image) {
Quoted 2 lines
cfb_imageblit(info, image); }
this patch works fine on my PowerMac 7300 with my Matrox Millenium PCI card...
so I think that both patch you sent me should be included in the kernel...
thank you
Quoted 45 lines · Mario Gaucher
From: "Antonino A. Daplas" <[email protected]>
Quoted 10 lines
Quoted 3 lines
but I still has some problem with my Matrox Millenium PCI card using matroxfb driver... the kernel boot... but I get corrupted characters on the console... X load ok and display ok...
Try this first.
1. Open
drivers/video/matrox/matrofb_accel.c
2. At the end of the file is this function:static void matroxfb_imageblit(struct fb_info* info, const struct
fb_image* image) {
Quoted 10 lines
MINFO_FROM_INFO(info); DBG_HEAVY(__FUNCTION__); if (image->depth == 1) { u_int32_t fgx, bgx; fgx = ((u_int32_t*)info->pseudo_palette)[image->fg_color]; bgx = ((u_int32_t*)info->pseudo_palette)[image->bg_color]; matroxfb_1bpp_imageblit(PMINFO fgx, bgx, image->data, image->width,
image->height,image->dy,image->dx);Quoted 11 lines
} else { /* Danger! image->depth is useless: logo painting code always passes framebuffer color depth here, although logo data are always 8bpp and info->pseudo_palette is changed to contain logo palette to be used (but only for true/direct-color... sic...). So do it completely in software... */ cfb_imageblit(info, image); } }3. Replace the above function to use software drawing so it becomes like
this:
Quoted 1 line
static void matroxfb_imageblit(struct fb_info* info, const struct
fb_image* image) {
Quoted 7 lines
Quoted 2 lines
cfb_imageblit(info, image); }this patch works fine on my PowerMac 7300 with my Matrox Millenium PCI card...
so I think that both patch you sent me should be included in the kernel...
No, this modification is for testing purposes only. It means that hardware color expansion in matroxfb for powerpc's got broken. CC'ed Petr.
Tony
Quoted 7 lines · Antonino A. Daplas
The field
info->modelistis initialized duringregister_framebuffer. This field is also referred to infb_set_var(). Thus a call tofb_set_var()beforeregister_framebuffer()will cause a crash. A few drivers do this, notably controlfb. (This might fix reports of controlfb crashing in powermacs)..../...
Hi Antonio, I "missed" this modelist thingy. Can you explain what is the logic ? There is now a modelist attached to fb_info used for mode matching ? In this case, it makes sense to be able to add things to it before register_framebuffer(). I mean, I would expect drivers to probe at least their default head modes before registering the fb, since the later will cause fbcon to try to setup a mode ...
Ben.
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 6 lines · Antonino A. Daplas
The field
info->modelistis initialized duringregister_framebuffer. This field is also referred to infb_set_var(). Thus a call tofb_set_var()beforeregister_framebuffer()will cause a crash. A few drivers do this, notably controlfb. (This might fix reports of controlfb crashing in powermacs).
Quoted 8 lines
Quoted 1 line
.../...
Hi Antonio, I "missed" this modelist thingy. Can you explain what is the logic ? There is now a modelist attached to
fb_infoused for mode matching ? In this case, it makes sense to be able to add things to it beforeregister_framebuffer(). I mean, I would expect drivers to probe at least their default head modes before registering the fb, since the later will cause fbcon to try to setup a mode ...
Yep, this came about because there were two problems in terms of mode setting before I added the modelist:
1. Lack of per-display var which prevents full restore of the console on mode switch
2. the stty utility does not work correctly for drivers that do not have mode validation
The modelist functions as a private mode database, meaning the entries are always correct for the driver/graphics/display combination in question.
The modelist can be filled up in a number of ways:
1. By default, upon register_framebuffer(), if info->modelist is empty or not initialized, it will be filled up with mode timings derived from info->var. In effect, most drivers will have at least one correct entry in info->modelist.
2. Drivers can choose to fill up info->modelist, perhaps from timings derived from EDID. There is one function in fbmem.c, fb_videomode_to_modelist() which converts a modedb array to a modelist.
3. For each correct fbset, (accepted by both driver and user), if the timings are unique, this will be also be added to the modelist. Also, in the unfortunate case that an illegal mode timing is accidentally entered into the modelist, there is a mechanism to remove that entry.
Therefore, when doing an stty, fbcon will only look at info->modelist, finds the best matching mode, builds a var from that particular entry and passes the var to the driver. There is no guesswork involved, and drivers will always operate on known working mode timings.
Also, the graphics states per console are also preserved. However, instead of saving a var per console, we only save a part of the var, the graphics state. The mode timings are saved as pointers to entries in info->modelist. In effect, we have the full functionality of a per-display var, but we save half the memory, that's around 5K.
You'll probably notice that with recent kernels, if you switch consoles, your previous console settings are restored. Also, doing an stty should, in theory, never produce a corrupt console, even if you enter insane numbers. Of course, the effectivity of stty depends on the number of entries in info->modelist.
Anyway, the goal here is to fix as many regressions as possible. And I think the 2.6 framebuffer is now behaving in almost the same manner as 2.4, with a few minor exceptions.
BTW, a few drivers do fill up info->modelist before register_framebuffer(), rivafb and savagefb.
Tony
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
Quoted 4 lines · Antonino A. Daplas
1. Lack of per-display var which prevents full restore of the console on mode switch
2. the stty utility does not work correctly for drivers that do not have mode validation
Ok, good.
.../...
Quoted 4 lines
3. For each correct fbset, (accepted by both driver and user), if the timings are unique, this will be also be added to the modelist. Also, in the unfortunate case that an illegal mode timing is accidentally entered into the modelist, there is a mechanism to remove that entry.
Ok, though I'm not too fan of this one, I'd rather have an explicit flag in the var passed by fbset trigger the "remember" thing... oh well, we'll see how things goes.
Quoted 4 lines
Therefore, when doing an stty, fbcon will only look at
info->modelist, finds the best matching mode, builds a var from that particular entry and passes the var to the driver. There is no guesswork involved, and drivers will always operate on known working mode timings.
Yah, I know that problem, this looks like a good solution.
Quoted 5 lines
Also, the graphics states per console are also preserved. However, instead of saving a var per console, we only save a part of the var, the graphics state. The mode timings are saved as pointers to entries in
info->modelist. In effect, we have the full functionality of a per-display var, but we save half the memory, that's around 5K.
I dislike the pointers tho. Remember that one day, we'll do monitor hotswap, which means that at one point, the driver will remove/add/rebuild modelist...
Quoted 5 lines
You'll probably notice that with recent kernels, if you switch consoles, your previous console settings are restored. Also, doing an stty should, in theory, never produce a corrupt console, even if you enter insane numbers. Of course, the effectivity of stty depends on the number of entries in
info->modelist.
Yah, sounds good.
Quoted 6 lines
Anyway, the goal here is to fix as many regressions as possible. And I think the 2.6 framebuffer is now behaving in almost the same manner as 2.4, with a few minor exceptions.
BTW, a few drivers do fill up
info->modelistbeforeregister_framebuffer(), rivafb and savagefb.
I'll add that to radeonfb too, I'm working on a big update of this driver adding power management support for newer apple laptops plus a few other things, I'll include that as well.
Ben.
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 4 lines · Antonino A. Daplas
1. Lack of per-display var which prevents full restore of the console on mode switch
2. the stty utility does not work correctly for drivers that do not have mode validationOk, good.
.../...
Quoted 8 lines
Quoted 4 lines
3. For each correct fbset, (accepted by both driver and user), if the timings are unique, this will be also be added to the modelist. Also, in the unfortunate case that an illegal mode timing is accidentally entered into the modelist, there is a mechanism to remove that entry.
Ok, though I'm not too fan of this one, I'd rather have an explicit flag in the var passed by fbset trigger the "remember" thing... oh well, we'll see how things goes.
You mean the mode entry deletion? That's okay, the code is minor, still unused, and was submitted as optional, but still got merged. It can be easily removed.
Quoted 14 lines
Quoted 5 lines
Therefore, when doing an stty, fbcon will only look at
info->modelist, finds the best matching mode, builds a var from that particular entry and passes the var to the driver. There is no guesswork involved, and drivers will always operate on known working mode timings.Yah, I know that problem, this looks like a good solution.
Quoted 5 lines
Also, the graphics states per console are also preserved. However, instead of saving a var per console, we only save a part of the var, the graphics state. The mode timings are saved as pointers to entries in
info->modelist. In effect, we have the full functionality of a per-display var, but we save half the memory, that's around 5K.
Quoted 3 lines
I dislike the pointers tho. Remember that one day, we'll do monitor hotswap, which means that at one point, the driver will remove/add/rebuild modelist...
Removing/adding/rebuilding the modelist should be doable with pointers. But in the case people don't want pointers, this can be easily changed. The conversion code is entirely in display_to_var() and var_to_display() helpers.
Tony
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/