[PATCH]: Fix NULL-ptr dereference in pm2fb_probe

[PATCH]: Fix NULL-ptr dereference in pm2fb_probe

[Patrick McHardy]

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

This patch fixes a NULL-pointer dereference in pm2fb_probe.
The memset sets info->par to 0, it is dereferenced shortly
afterwards. framebuffer_alloc already initializes the memory
to 0, so it can simply be removed.

Regards
Patrick



[-- Attachment #2: pm2fb_probe.diff --]
[-- Type: text/x-patch, Size: 639 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/04/23 04:05:58+02:00 kaber@trash.net 
#   Fix NULL-ptr dereference in pm2fb_probe
# 
# drivers/video/pm2fb.c
#   2004/04/23 03:59:41+02:00 kaber@trash.net +0 -2
#   Fix NULL-ptr dereference in pm2fb_probe
# 
diff -Nru a/drivers/video/pm2fb.c b/drivers/video/pm2fb.c
--- a/drivers/video/pm2fb.c	Fri Apr 23 04:06:53 2004
+++ b/drivers/video/pm2fb.c	Fri Apr 23 04:06:53 2004
@@ -1035,8 +1035,6 @@
 	info = framebuffer_alloc(size, &pdev->dev);
 	if ( !info )
 		return -ENOMEM;
-	memset(info, 0, size);
-    
 	default_par = info->par;
  
 	switch (pdev->device) {

Re: [PATCH]: Fix NULL-ptr dereference in pm2fb_probe

[Jim Hague]

On 23-Apr-2004 Patrick McHardy wrote:
> This patch fixes a NULL-pointer dereference in pm2fb_probe.
> The memset sets info->par to 0, it is dereferenced shortly
> afterwards. framebuffer_alloc already initializes the memory
> to 0, so it can simply be removed.

Thanks for your work. We've overlapped on this one, I'm afraid - a couple of
days ago I sent a patch off to James and the fbdev list that addresses this and
a bug in pm2fb_blank().

Out of interest, can I ask if you're running pm2fb and if so on what hardware?

-- 
Jim Hague - jim.hague@acm.org          Never trust a computer you can't lift.

Re: [PATCH]: Fix NULL-ptr dereference in pm2fb_probe

[Patrick McHardy]

Jim Hague wrote:

>Out of interest, can I ask if you're running pm2fb and if so on what hardware?
>  
>
I'm running pm2fb on a TVP4020, lspci -vvv looks like this:

0000:01:00.0 Display controller: Texas Instruments TVP4020 [Permedia 2] 
(rev 11)
        Subsystem: Elsa AG GLoria Synergy
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- 
ParErr- Stepping- SERR- FastB2B-        Status: Cap+ 66MHz+ UDF- 
FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 32 (48000ns min, 48000ns max)
        Interrupt: pin A routed to IRQ 11
        Region 0: Memory at dfde0000 (32-bit, non-prefetchable) 
[size=dfdd0000]
        Region 1: Memory at df000000 (32-bit, non-prefetchable) [size=8M]
        Region 2: Memory at de800000 (32-bit, non-prefetchable) [size=8M]
        Expansion ROM at 00010000 [disabled]
        Capabilities: [4c] Power Management version 1
                Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA 
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [40] AGP version 1.0
                Status: RQ=32 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64- 
HTrans- 64bit- FW- AGP3- Rate=x1
                Command: RQ=1 ArqSz=0 Cal=0 SBA- AGP- GART64- 64bit- FW- 
Rate=<none>

Regards,
Patrick

Re: [PATCH]: Fix NULL-ptr dereference in pm2fb_probe

[James Simmons]

Okay I seen alot of patches going around. So the patch doesn't get lost 
can someone send me the final patch. I can then forward it to Andrew.

Re: [PATCH]: Fix NULL-ptr dereference in pm2fb_probe

[Jim Hague]

On 23-Apr-2004 James Simmons wrote:
> Okay I seen alot of patches going around. So the patch doesn't get lost 
> can someone send me the final patch. I can then forward it to Andrew.

This patch (against 2.6.6-rc1) please. It fixes the NULL pointer dereference
and also a problem in pm2fb_blank().


===== pm2fb.c 1.25 vs 1.26 =====
--- 1.25/drivers/video/pm2fb.c  Fri Apr 16 17:30:02 2004
+++ 1.26/drivers/video/pm2fb.c  Wed Apr 21 00:26:58 2004
@@ -63,6 +63,16 @@
 #endif
 
 /*
+ * The 2.4 driver calls reset_card() at init time, where it also sets the
+ * initial mode. I don't think the driver should touch the chip until
+ * the console sets a video mode. So I was calling this at the start
+ * of setting a mode. However, certainly on 1280x1024 depth 16 on my
+ * PCI Graphics Blaster Exxtreme this causes the display to smear
+ * slightly.  I don't know why. Guesses to jim.hague@acm.org.
+ */
+#undef RESET_CARD_ON_MODE_SET
+
+/*
  * Driver data 
  */
 static char *mode __initdata = NULL;
@@ -340,16 +350,7 @@
        }
 }
 
-#if 0
-/*
- * FIXME:
- * The 2.4 driver calls this at init time, where it also sets the
- * initial mode. I don't think the driver should touch the chip
- * until the console sets a video mode. So I was calling this
- * at the start of setting a mode. However, certainly on 1280x1024
- * depth 16 this causes the display to smear slightly.
- * I don't know why. Guesses to jim.hague@acm.org.
- */
+#ifdef RESET_CARD_ON_MODE_SET
 static void reset_card(struct pm2fb_par* p)
 {
        if (p->type == PM2_TYPE_PERMEDIA2V)
@@ -501,6 +502,8 @@
        u32 vsync;
 
        vsync = video;
+
+       DPRINTK("video = 0x%x\n", video);
        
        /*
         * The hardware cursor needs +vsync to recognise vert retrace.
@@ -660,6 +663,9 @@
        u32 xres;
        int data64;
 
+#ifdef RESET_CARD_ON_MODE_SET
+       reset_card(par);
+#endif 
        reset_config(par);
        clear_palette(par);
     
@@ -721,8 +727,7 @@
 
        info->fix.visual =
                (depth == 8) ? FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
-       info->fix.line_length =
-               info->var.xres * ((info->var.bits_per_pixel + 7) >> 3);
+       info->fix.line_length = info->var.xres * depth / 8;
        info->cmap.len = 256;
 
        /*
@@ -803,6 +808,8 @@
                break;
        }
        set_pixclock(par, pixclock);
+       DPRINTK("Setting graphics mode at %dx%d depth %d\n",
+               info->var.xres, info->var.yres, info->var.bits_per_pixel);
        return 0;       
 }
 
@@ -843,7 +850,8 @@
         *   var->{color}.offset contains start of bitfield
         *   var->{color}.length contains length of bitfield
         *   {hardwarespecific} contains width of DAC
-        *   cmap[X] is programmed to (X << red.offset) | (X << green.offset) | 
(X << blue.offset)
+        *   cmap[X] is programmed to
+        *   (X << red.offset) | (X << green.offset) | (X << blue.offset)
         *   RAMDAC[X] is programmed to (red, green, blue)
         *
         * Pseudocolor:
@@ -856,8 +864,9 @@
         *    does not use RAMDAC (usually has 3 of them).
         *    var->{color}.offset contains start of bitfield
         *    var->{color}.length contains length of bitfield
-        *    cmap is programmed to (red << red.offset) | (green << green.offset
) |
-        *                      (blue << blue.offset) | (transp << transp.offset
)
+        *    cmap is programmed to
+        *    (red << red.offset) | (green << green.offset) |
+        *    (blue << blue.offset) | (transp << transp.offset)
         *    RAMDAC does not exist
         */
 #define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
@@ -962,6 +971,11 @@
        struct pm2fb_par *par = (struct pm2fb_par *) info->par;
        u32 video = par->video;
 
+       DPRINTK("blank_mode %d\n", blank_mode);
+
+       /* Turn everything on, then disable as requested. */
+       video |= (PM2F_VIDEO_ENABLE | PM2F_HSYNC_MASK | PM2F_VSYNC_MASK);
+
        switch (blank_mode) {
        case 0:         /* Screen: On; HSync: On, VSync: On */
                break;
@@ -1030,15 +1044,12 @@
                return err;
        }
 
-       size = sizeof(struct fb_info) + sizeof(struct pm2fb_par) + 256 * sizeof(
u32);
-
+       size = sizeof(struct pm2fb_par) + 256 * sizeof(u32);
        info = framebuffer_alloc(size, &pdev->dev);
        if ( !info )
                return -ENOMEM;
-       memset(info, 0, size);
-    
-       default_par = info->par;
- 
+       default_par = (struct pm2fb_par *) info->par;
+
        switch (pdev->device) {
        case  PCI_DEVICE_ID_TI_TVP4020:
                strcpy(pm2fb_fix.id, "TVP4020");
@@ -1112,7 +1123,6 @@
 
        info->fbops             = &pm2fb_ops;
        info->fix               = pm2fb_fix;    
-       info->par               = default_par;
        info->pseudo_palette    = (void *)(default_par + 1); 
        info->flags             = FBINFO_FLAG_DEFAULT;
 


-- 
Jim Hague - jim.hague@acm.org          Never trust a computer you can't lift.