From 3f50b0222e4c6ac59a5c4819f8be0fa500970381 Mon Sep 17 00:00:00 2001 From: Kevin Winchester Date: Tue, 17 Nov 2009 14:38:45 -0800 Subject: [PATCH 1/3] agp: correct missing cleanup on error in agp_add_bridge While investigating a kmemleak detected leak, I encountered the agp_add_bridge function. It appears to be responsible for freeing the agp_bridge_data in the case of a failure, but it is only doing so for some errors. Fix it to always free the bridge data if a failure condition is encountered. Signed-off-by: Kevin Winchester Signed-off-by: Andrew Morton Signed-off-by: Dave Airlie --- drivers/char/agp/backend.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c index a56ca080e108..c3ab46da51a3 100644 --- a/drivers/char/agp/backend.c +++ b/drivers/char/agp/backend.c @@ -285,18 +285,22 @@ int agp_add_bridge(struct agp_bridge_data *bridge) { int error; - if (agp_off) - return -ENODEV; + if (agp_off) { + error = -ENODEV; + goto err_put_bridge; + } if (!bridge->dev) { printk (KERN_DEBUG PFX "Erk, registering with no pci_dev!\n"); - return -EINVAL; + error = -EINVAL; + goto err_put_bridge; } /* Grab reference on the chipset driver. */ if (!try_module_get(bridge->driver->owner)) { dev_info(&bridge->dev->dev, "can't lock chipset driver\n"); - return -EINVAL; + error = -EINVAL; + goto err_put_bridge; } error = agp_backend_initialize(bridge); @@ -326,6 +330,7 @@ frontend_err: agp_backend_cleanup(bridge); err_out: module_put(bridge->driver->owner); +err_put_bridge: agp_put_bridge(bridge); return error; } From 67fe63b0715ccfaefa0af8a6e705c5470ee5cada Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 7 Jan 2010 12:58:51 -0700 Subject: [PATCH 2/3] agp/hp: fixup hp agp after ACPI changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 15b8dd53f5ffa changed the string in info->hardware_id from a static array to a pointer and added a length field. But instead of changing "sizeof(array)" to "length", we changed it to "sizeof(length)" (== 4), which corrupts the string we're trying to null-terminate. We no longer even need to null-terminate the string, but we *do* need to check whether we found a HID. If there's no HID, we used to have an empty array, but now we have a null pointer. The combination of these defects causes this oops: Unable to handle kernel NULL pointer dereference (address 0000000000000003) modprobe[895]: Oops 8804682956800 [1] ip is at zx1_gart_probe+0xd0/0xcc0 [hp_agp] http://marc.info/?l=linux-ia64&m=126264484923647&w=2 Signed-off-by: Bjorn Helgaas Reported-by: Émeric Maschino Signed-off-by: Dave Airlie --- drivers/char/agp/hp-agp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/char/agp/hp-agp.c b/drivers/char/agp/hp-agp.c index 9047b2714653..dc8a6f70483b 100644 --- a/drivers/char/agp/hp-agp.c +++ b/drivers/char/agp/hp-agp.c @@ -488,9 +488,8 @@ zx1_gart_probe (acpi_handle obj, u32 depth, void *context, void **ret) handle = obj; do { status = acpi_get_object_info(handle, &info); - if (ACPI_SUCCESS(status)) { + if (ACPI_SUCCESS(status) && (info->valid & ACPI_VALID_HID)) { /* TBD check _CID also */ - info->hardware_id.string[sizeof(info->hardware_id.length)-1] = '\0'; match = (strcmp(info->hardware_id.string, "HWP0001") == 0); kfree(info); if (match) { From 3d4a7882b11299104a0e74425dece2e26ac98024 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 7 Jan 2010 12:58:56 -0700 Subject: [PATCH 3/3] agp/hp: fail gracefully if we don't find an IOC Bail out if we don't find an enclosing IOC. Previously, if we didn't find one, we tried to set things up using garbage for the SBA/IOC register address, which causes a crash. This crash only happens if firmware supplies a defective ACPI namespace, so it doesn't fix any problems in the field. Signed-off-by: Bjorn Helgaas Signed-off-by: Dave Airlie --- drivers/char/agp/hp-agp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/char/agp/hp-agp.c b/drivers/char/agp/hp-agp.c index dc8a6f70483b..58752b70efea 100644 --- a/drivers/char/agp/hp-agp.c +++ b/drivers/char/agp/hp-agp.c @@ -508,6 +508,9 @@ zx1_gart_probe (acpi_handle obj, u32 depth, void *context, void **ret) handle = parent; } while (ACPI_SUCCESS(status)); + if (ACPI_FAILURE(status)) + return AE_OK; /* found no enclosing IOC */ + if (hp_zx1_setup(sba_hpa + HP_ZX1_IOC_OFFSET, lba_hpa)) return AE_OK;