Skip to content

Commit

Permalink
driver core : Fix use after free of dev->parent in device_shutdown
Browse files Browse the repository at this point in the history
The put_device(dev) at the bottom of the loop of device_shutdown
may result in the dev being cleaned up. In device_create_release,
the dev is kfreed.

However, device_shutdown attempts to use the dev pointer again after
put_device by referring to dev->parent.

Copy the parent pointer instead to avoid this condition.

This bug was found on Chromium OS's chromeos-3.8, which is based on v3.8.11.
See bug report : https://code.google.com/p/chromium/issues/detail?id=297842
This can easily be reproduced when shutting down with
hidraw devices that report battery condition.
Two examples are the HP Bluetooth Mouse X4000b and the Apple Magic Mouse.
For example, with the magic mouse :
The dev in question is "hidraw0"
dev->parent is "magicmouse"

In the course of the shutdown for this device, the input event cleanup calls
a put on hidraw0, decrementing its reference count.
When we finally get to put_device(dev) in device_shutdown, kobject_cleanup
is called and device_create_release does kfree(dev).
dev->parent is no longer valid, and we may crash in
put_device(dev->parent).

This change should be applied on any kernel with this change :
d1c6c03

Cc: [email protected]
Signed-off-by: Benson Leung <[email protected]>
Reviewed-by: Ming Lei <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
bleungatchromium authored and gregkh committed Sep 26, 2013
1 parent 667b410 commit f123db8
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions drivers/base/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -2017,7 +2017,7 @@ EXPORT_SYMBOL_GPL(device_move);
*/
void device_shutdown(void)
{
struct device *dev;
struct device *dev, *parent;

spin_lock(&devices_kset->list_lock);
/*
Expand All @@ -2034,7 +2034,7 @@ void device_shutdown(void)
* prevent it from being freed because parent's
* lock is to be held
*/
get_device(dev->parent);
parent = get_device(dev->parent);
get_device(dev);
/*
* Make sure the device is off the kset list, in the
Expand All @@ -2044,8 +2044,8 @@ void device_shutdown(void)
spin_unlock(&devices_kset->list_lock);

/* hold lock to avoid race with probe/release */
if (dev->parent)
device_lock(dev->parent);
if (parent)
device_lock(parent);
device_lock(dev);

/* Don't allow any more runtime suspends */
Expand All @@ -2063,11 +2063,11 @@ void device_shutdown(void)
}

device_unlock(dev);
if (dev->parent)
device_unlock(dev->parent);
if (parent)
device_unlock(parent);

put_device(dev);
put_device(dev->parent);
put_device(parent);

spin_lock(&devices_kset->list_lock);
}
Expand Down

0 comments on commit f123db8

Please sign in to comment.