Re: [PATCH] cyber2000fb: New framebuffer_alloc API and class_dev changes

2 messages, 2 authors, 2003-09-15 · open the first message on its own page

Re: [PATCH] cyber2000fb: New framebuffer_alloc API and class_dev changes

From: Russell King <hidden>
Date: 2003-09-15 21:07:47 

On Mon, Sep 15, 2003 at 09:43:29PM +0200, Kronos wrote:
quoted hunk
Hi,
this patch converts driver/video/cyber200fb.c to framebuffer_alloc:

======== drivers/video/cyber2000fb.c 1.33 ========
D 1.33 03/09/13 23:21:10+02:00 kronos@kronoz.cjb.net 35 34 108/99/1650
P drivers/video/cyber2000fb.c
C switch to framebuffer_alloc
------------------------------------------------

===== drivers/video/cyber2000fb.c 1.32 vs 1.33 =====
--- 1.32/drivers/video/cyber2000fb.c	Fri Aug 22 08:27:08 2003
+++ 1.33/drivers/video/cyber2000fb.c	Sat Sep 13 23:21:10 2003
@@ -62,7 +62,7 @@
 #include "cyber2000fb.h"
 
 struct cfb_info {
-	struct fb_info		fb;
+	struct fb_info		*fb;
Oh god, do we have to add yet another level of indirection all over
the framebuffer code?
quoted hunk
@@ -1635,6 +1638,16 @@
 	return err;
 }
 
+static void release_cfb_info(struct fb_info *info) {
+	struct cfb_info *cfb = info->par;
+
+	iounmap(cfb->region);
+	fb_alloc_cmap(&info->cmap, 0, 0);
+
+	if (cfb->dev)
+		pci_release_regions(cfb->dev);
+}
+
 static void __devexit cyberpro_pci_remove(struct pci_dev *dev)
 {
 	struct cfb_info *cfb = pci_get_drvdata(dev);
Who says "cfb->dev" remains valid after the PCI device has been removed.
This looks like a perfect use-after-free bug waiting to happen.

-- 
Russell King (rmk@arm.linux.org.uk)	http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
  2.6 ARM Linux   - http://www.arm.linux.org.uk/
  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
  2.6 Serial core


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: [PATCH] cyber2000fb: New framebuffer_alloc API and class_dev changes

From: Kronos <hidden>
Date: 2003-09-15 21:29:06 

Il Mon, Sep 15, 2003 at 10:07:42PM +0100, Russell King ha scritto:
quoted
 struct cfb_info {
-	struct fb_info		fb;
+	struct fb_info		*fb;
Oh god, do we have to add yet another level of indirection all over
the framebuffer code?
Ok, I've been to vague...

Now there is  a class_dev embedded in fb_info which  registered with the
driver model. We need a dynamically allocated struct fb_info.

quoted
@@ -1635,6 +1638,16 @@
 	return err;
 }
 
+static void release_cfb_info(struct fb_info *info) {
+	struct cfb_info *cfb = info->par;
+
+	iounmap(cfb->region);
+	fb_alloc_cmap(&info->cmap, 0, 0);
+
+	if (cfb->dev)
+		pci_release_regions(cfb->dev);
+}
+
 static void __devexit cyberpro_pci_remove(struct pci_dev *dev)
 {
 	struct cfb_info *cfb = pci_get_drvdata(dev);
Who says "cfb->dev" remains valid after the PCI device has been removed.
This looks like a perfect use-after-free bug waiting to happen.
cfb->dev is  refcounted, it  won't go  away until we  are done  with the
cleanup. Maybe I misread  driver core code...

Luca
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
Windows NT: Designed for the Internet. The Internet: Designed for Unix.


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help