2005-01-08 19:08:27

by Christoph Hellwig

Subject: [PATCH] fix pci_get_device conversion in intel-agp

- any device teardown must happen between agp_remove_bridge and
agp_put_bridge, before agp_remove_bridge users can still call into
the code
- it's releasing a reference to the wrong device


--- 1.76/drivers/char/agp/intel-agp.c 2004-12-22 05:53:58 +01:00
+++ edited/drivers/char/agp/intel-agp.c 2005-01-08 20:11:38 +01:00
@@ -1720,8 +1720,13 @@
{
struct agp_bridge_data *bridge = pci_get_drvdata(pdev);

- pci_dev_put(pdev);
agp_remove_bridge(bridge);
+
+ if (intel_i810_private.i810_dev)
+ pci_dev_put(intel_i810_private.i830_dev);
+ if (intel_i810_private.i830_dev)
+ pci_dev_put(intel_i830_private.i830_dev);
+
agp_put_bridge(bridge);
}
--- 1.18/drivers/char/agp/intel-mch-agp.c 2004-12-16 07:31:44 +01:00
+++ edited/drivers/char/agp/intel-mch-agp.c 2005-01-08 20:12:18 +01:00
@@ -569,8 +569,11 @@
{
struct agp_bridge_data *bridge = pci_get_drvdata(pdev);

- pci_dev_put(pdev);
agp_remove_bridge(bridge);
+
+ if (intel_i810_private.i830_dev)
+ pci_dev_put(intel_i830_private.i830_dev);
+
agp_put_bridge(bridge);
}

2005-01-08 19:13:18

by Roland Dreier

Subject: Re: [PATCH] fix pci_get_device conversion in intel-agp

> + if (intel_i810_private.i810_dev)
> + pci_dev_put(intel_i810_private.i830_dev);
> + if (intel_i810_private.i830_dev)
> + pci_dev_put(intel_i830_private.i830_dev);

Is there a typo in the patch here -- should the first put be for i810_dev?

- R.

2005-01-08 19:15:08

by Christoph Hellwig

Subject: Re: [PATCH] fix pci_get_device conversion in intel-agp

On Sat, Jan 08, 2005 at 11:13:14AM -0800, Roland Dreier wrote:
> > + if (intel_i810_private.i810_dev)
> > + pci_dev_put(intel_i810_private.i830_dev);
> > + if (intel_i810_private.i830_dev)
> > + pci_dev_put(intel_i830_private.i830_dev);
>
> Is there a typo in the patch here -- should the first put be for i810_dev?

Yes.

2005-01-09 18:29:47

by Greg KH

Subject: Re: [PATCH] fix pci_get_device conversion in intel-agp

On Sat, Jan 08, 2005 at 08:08:15PM +0100, Christoph Hellwig wrote:
> + if (intel_i810_private.i810_dev)
> + pci_dev_put(intel_i810_private.i830_dev);
> + if (intel_i810_private.i830_dev)
> + pci_dev_put(intel_i830_private.i830_dev);

It's legal to call pci_dev_put() with a NULL pointer, so these checks
are not needed.

thanks,

greg k-h