[OOPS] linux-2.6.7-bk20: possible use-after-free in platform_device_unregister()

[OOPS] linux-2.6.7-bk20: possible use-after-free in platform_device_unregister()

[Denis Vlasenko]

I compile kernels with
CONFIG_DEBUG_KERNEL=y
CONFIG_DEBUG_STACKOVERFLOW=y
CONFIG_DEBUG_STACK_USAGE=y
CONFIG_DEBUG_SLAB=y
CONFIG_DEBUG_SPINLOCK_SLEEP=y
CONFIG_FRAME_POINTER=y

Kernel oopsed while probing for devices in depca.c:

static void __init depca_platform_probe (void)
{
        int i;
        struct platform_device *pldev;

        for (i = 0; depca_io_ports[i].iobase; i++) {
                depca_io_ports[i].device = NULL;

                /* if an address has been specified on the command
                 * line, use it (if valid) */
                if (io && io != depca_io_ports[i].iobase)
                        continue;

                if (!(pldev = kmalloc (sizeof (*pldev), GFP_KERNEL)))
                        continue;

                memset (pldev, 0, sizeof (*pldev));
                pldev->name = depca_string;
                pldev->id   = i;
                pldev->dev.platform_data = (void *) depca_io_ports[i].iobase;
                pldev->dev.release       = depca_platform_release;
                depca_io_ports[i].device = pldev;

                if (platform_device_register (pldev)) {
                        kfree (pldev);
                        depca_io_ports[i].device = NULL;
                        continue;
                }

                if (!pldev->dev.driver) {
                /* The driver was not bound to this device, there was
                 * no hardware at this address. Unregister it, as the
                 * release fuction will take care of freeing the
                 * allocated structure */

                        depca_io_ports[i].device = NULL;
===>                    platform_device_unregister (pldev);
                }
        }
}

void platform_device_unregister(struct platform_device * pdev)
{
        int i;

        if (pdev) {
                device_unregister(&pdev->dev);

                for (i = 0; i < pdev->num_resources; i++) {
                        struct resource *r = &pdev->resource[i];
===>                    if (r->flags & (IORESOURCE_MEM|IORESOURCE_IO))
                                release_resource(r);
                }
        }
}

That if() corresponds to "testl  $0x300,0xc(%eax)" instruction in assembler.
%eax == 6b6b6b6b there at that moment (looks like poison). oops.

I verified that pdev->num_resources==0 upon entry to
platform_device_unregister().

device_unregister() must be deallocating *pdev, poisoning
its contents. pdev->num_resources!=0, and pdev->resource[i]
is garbage too.

However, I don't see where does it free *pdev.

Source of device_unregister() is below for easy reference.
--
vda

void device_unregister(struct device * dev)
{
        pr_debug("DEV: Unregistering device. ID = '%s'\n", dev->bus_id);
        device_del(dev);
        put_device(dev);
}

void device_del(struct device * dev)
{
        struct device * parent = dev->parent;

        down_write(&devices_subsys.rwsem);
        if (parent)
                list_del_init(&dev->node);
        up_write(&devices_subsys.rwsem);

        /* Notify the platform of the removal, in case they
         * need to do anything...
         */
        if (platform_notify_remove)
                platform_notify_remove(dev);
        bus_remove_device(dev);
        device_pm_remove(dev);
        kobject_del(&dev->kobj);
        if (parent)
                put_device(parent);
}

Re: [OOPS] linux-2.6.7-bk20: possible use-after-free in platform_device_unregister()

[Dmitry Torokhov]

On Saturday 10 July 2004 01:31 pm, Denis Vlasenko wrote:
> void platform_device_unregister(struct platform_device * pdev)
> {
> ? ? ? ? int i;
> 
> ? ? ? ? if (pdev) {
> ? ? ? ? ? ? ? ? device_unregister(&pdev->dev);
>

Here depca_platform_release kicks in and frees platform device structure.
platform_device_unregister should save pointer to resources. 
 
> ? ? ? ? ? ? ? ? for (i = 0; i < pdev->num_resources; i++) {
> ? ? ? ? ? ? ? ? ? ? ? ? struct resource *r = &pdev->resource[i];
> ===> ? ? ? ? ? ? ? ? ? ?if (r->flags & (IORESOURCE_MEM|IORESOURCE_IO))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? release_resource(r);
> ? ? ? ? ? ? ? ? }
> ? ? ? ? }
> }
> 

Does the following help (hopefully it will apply - I have just a tad
different tree):

===== drivers/base/platform.c 1.21 vs edited =====
--- 1.21/drivers/base/platform.c	2004-07-04 19:56:37 -05:00
+++ edited/drivers/base/platform.c	2004-07-10 14:44:51 -05:00
@@ -143,13 +143,15 @@
  */
 void platform_device_unregister(struct platform_device * pdev)
 {
-	int i;
-
 	if (pdev) {
+		int i, num_resources = pdev->num_resources;
+		struct resource *r, *resources = pdev->resource;
+
+		/* this call may free pdev, that's why we saved resource data */
 		device_unregister(&pdev->dev);
 
-		for (i = 0; i < pdev->num_resources; i++) {
-			struct resource *r = &pdev->resource[i];
+		for (i = 0; i < num_resources; i++) {
+			r = &resources[i];
 			if (r->flags & (IORESOURCE_MEM|IORESOURCE_IO))
 				release_resource(r);
 		}