Thread (2 messages) 2 messages, 2 authors, 2008-08-28

Re: [PATCH 2/2] tdfxfb: fix frame buffer name overrun

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2008-08-28 07:02:02

On Wed, 27 Aug 2008, Krzysztof Helt wrote:
From: Krzysztof Helt <redacted>

If there are more then one graphics card handled 
by the tdfxfb driver the name of the frame buffer 
overruns reserved name size.
Nice spot! Multiple strcat() will start overwriting other fields in
fb_fix_screeninfo.
quoted hunk ↗ jump to hunk
---
This also is good candidate for 2.6.27 fix.

 drivers/video/tdfxfb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/tdfxfb.c b/drivers/video/tdfxfb.c
index fa7cbec..ed86ef3 100644
--- a/drivers/video/tdfxfb.c
+++ b/drivers/video/tdfxfb.c
@@ -95,7 +95,6 @@ static inline int mtrr_del(int reg, unsigned long base,
 #define VOODOO5_MAX_PIXCLOCK 350000
 
 static struct fb_fix_screeninfo tdfx_fix __devinitdata = {
-	.id =		"3Dfx",
 	.type =		FB_TYPE_PACKED_PIXELS,
 	.visual =	FB_VISUAL_PSEUDOCOLOR,
 	.ypanstep =	1,
@@ -1196,6 +1195,7 @@ static int __devinit tdfxfb_probe(struct pci_dev *pdev,
 		return -ENOMEM;
 
 	default_par = info->par;
+	strcpy(tdfx_fix.id, "3Dfx");
 
 	/* Configure the default fb_fix_screeninfo first */
 	switch (pdev->device) {
At first I thought: but this doesn't fix all problems with multiple
cards, as tdfx_fix is still shared among all of them? But that's not
true (tdfx_fix is copied to info->fix later), so it is correct.

IMHO, as tdfx_fix is used as some `temporary' variable in tdfxfb_probe() only,
a better fix would be to change the operation mode from:

    info = framebuffer_alloc(...);
    tdfx_fix.foo = ...; /* depends on probe value */
    tdfx_fix.bar = ...; /* depends on probe value */
    info->fix = tdfx_fix;

to

    info = framebuffer_alloc(...);
    info->fix = tdfx_fix;
    info->fix.foo = ...; /* depends on probe value */
    info->fix.bar = ...; /* depends on probe value */

and make tdfx_fix const. Then you don't have to initialize tdfx_fix.id
for every card, and make it clear that tdfx_fix is just used as a template.

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 Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help