On Fri, Feb 14, 2014 at 07:23:32PM +0200, Imre Deak wrote:
> pci_get_class(class, from) drops the refcount for 'from', so the
> extra pci_dev_put we do on it will result in a use after free bug
> sometime later starting with the WARN below.
That's a very nice find.
But you can tidy this loop up even more...
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2d05d7c..4e4fc0c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -347,7 +347,6 @@ void intel_detect_pch(struct drm_device *dev)
> */
> pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
> while (pch) {
...by noting that what you want here is
while ((pch = pci_get_class(ISA<<8, pch)) {
> - struct pci_dev *curr = pch;
> if (pch->vendor == PCI_VENDOR_ID_INTEL) {
> unsigned short id;
> id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
> @@ -385,15 +384,15 @@ void intel_detect_pch(struct drm_device *dev)
> } else {
> goto check_next;
> }
> - pci_dev_put(pch);
> break;
> }
> check_next:
> - pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, curr);
> - pci_dev_put(curr);
> + pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, pch);
> }
> if (!pch)
> DRM_DEBUG_KMS("No PCH found?\n");
> + else
> + pci_dev_put(pch);
> }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2d05d7c..9962aef 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -324,7 +324,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
void intel_detect_pch(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct pci_dev *pch;
+ struct pci_dev *pch = NULL;
/* In all current cases, num_pipes is equivalent to the PCH_NOP setting
* (which really amounts to a PCH but no South Display).
@@ -345,12 +345,9 @@ void intel_detect_pch(struct drm_device *dev)
* all the ISA bridge devices and check for the first match, instead
* of only checking the first one.
*/
- pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
- while (pch) {
- struct pci_dev *curr = pch;
+ while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, pch))) {
if (pch->vendor == PCI_VENDOR_ID_INTEL) {
- unsigned short id;
- id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
+ unsigned short id = pch->device &
INTEL_PCH_DEVICE_ID_MASK;
dev_priv->pch_id = id;
if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) {
@@ -382,18 +379,15 @@ void intel_detect_pch(struct drm_device *dev)
DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
WARN_ON(!IS_HASWELL(dev));
WARN_ON(!IS_ULT(dev));
- } else {
- goto check_next;
- }
+ } else
+ continue;
+
pci_dev_put(pch);
break;
}
-check_next:
- pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, curr);
- pci_dev_put(curr);
}
if (!pch)
- DRM_DEBUG_KMS("No PCH found?\n");
+ DRM_DEBUG_KMS("No PCH found.\n");
}
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
