linux-kernel - [PATCH] PM / sleep: fix use-after-free on async resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 21 Jan 2020 10:00:52 +0900
From:   Chanho Min <chanho.min@....com>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Daewoong Kim <daewoong00.kim@....com>,
        Seokjoo Lee <seokjoo.lee@....com>,
        Lee Gunho <gunho.lee@....com>, Chanho Min <chanho.min@....com>
Subject: [PATCH] PM / sleep: fix use-after-free on async resume

Some device can be released during suspend (e.g. usb disconnection).
But, Its child device still use dev->parent's lock in dpm_wait().
It can be ocurred use-after-free as bellows. This is happened during
usb resume in practice.

device hierarchy: "1-1" <- "1-1:1.2" <- "ep83"

<parent>		<child>
device_resume("1-1:1.2")
dpm_wait("1-1")
			device_resume("ep_83");
			dpm_wait("1-1:1.2");
 usb_disconnect
  put_device("1-1:1.2")

put_device("1-1:1.2")
 usb_release_interface
 kfree(intf) <- "1-1:1.2"'s struct device is freed

			 wait_for_common
			 do {
			 ...
			 spin_lock_irq(&x->wait.lock); <- "1-1:1-2"'s lock
			 } while (!x->done && timeout);

This is call stack of the system hang caused by freed lock value in practice.

Call trace:
[<ffffffc000ef59a8>] _raw_spin_lock_irq+0x38/0x80
[<ffffffc000ef2dac>] wait_for_common+0x12c/0x140
[<ffffffc000ef2dd4>] wait_for_completion+0x14/0x20
[<ffffffc000480c1c>] dpm_wait+0x5c/0xb0
[<ffffffc0004813d8>] device_resume+0x78/0x320
[<ffffffc000481ed4>] async_resume+0x24/0xe0
[<ffffffc0000c671c>] async_run_entry_fn+0x54/0x158
[<ffffffc0000bd720>] process_one_work+0x1e8/0x4b0
[<ffffffc0000bdb10>] worker_thread+0x128/0x4b8
[<ffffffc0000c3a14>] kthread+0x10c/0x110
[<ffffffc00008ddd0>] ret_from_fork+0x10/0x40

To prevent such use-after-free, dpm_wait_for_parent() keeps parent's reference
using get/put_device even if it is disconnected.

Signed-off-by: Chanho Min <chanho.min@....com>
Signed-off-by: Daewoong Kim <daewoong00.kim@....com>
---
 drivers/base/power/main.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index f946511..95a7499 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -234,13 +234,29 @@ static void initcall_debug_report(struct device *dev, ktime_t calltime,
  * @dev: Device to wait for.
  * @async: If unset, wait only if the device's power.async_suspend flag is set.
  */
+static void _dpm_wait(struct device *dev, bool async)
+{
+	if (async || (pm_async_enabled && dev->power.async_suspend))
+		wait_for_completion(&dev->power.completion);
+}
+
 static void dpm_wait(struct device *dev, bool async)
 {
 	if (!dev)
 		return;
 
-	if (async || (pm_async_enabled && dev->power.async_suspend))
-		wait_for_completion(&dev->power.completion);
+	_dpm_wait(dev, async);
+}
+
+static void dpm_wait_for_parent(struct device *dev, bool async)
+{
+	if (dev && dev->parent) {
+		struct device *dev_p = dev->parent;
+
+		get_device(dev_p);
+		_dpm_wait(dev_p, async);
+		put_device(dev_p);
+	}
 }
 
 static int dpm_wait_fn(struct device *dev, void *async_ptr)
@@ -277,7 +293,7 @@ static void dpm_wait_for_suppliers(struct device *dev, bool async)
 
 static void dpm_wait_for_superior(struct device *dev, bool async)
 {
-	dpm_wait(dev->parent, async);
+	dpm_wait_for_parent(dev, async);
 	dpm_wait_for_suppliers(dev, async);
 }
 
-- 
2.7.4