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

On Mon, Sep 15, 2003 at 11:28:09PM +0200, Kronos wrote:

quoted

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...

pci_request_regions / pci_release_regions does not perform any reference
counting on the pci device.

-- 
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

Il Mon, Sep 15, 2003 at 10:33:31PM +0100, Russell King ha scritto: 

quoted

quoted

quoted

+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...

pci_request_regions / pci_release_regions does not perform any reference
counting on the pci device.

I was referring to class_dev stuff, class_device_add in
drivers/base/class.c:262

Luca
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
"L'abilita` politica e` l'abilita` di prevedere quello che
 accadra` domani, la prossima settimana, il prossimo mese e
 l'anno prossimo. E di essere cosi` abili, piu` tardi,
 da spiegare  perche' non e` accaduto."


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