linux-kernel - [PATCH v2] power: supply: ds2782: fix possible use-after-free on remove

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]

Message-Id: <20190212162149.21583-1-TheSven73@gmail.com>
Date:   Tue, 12 Feb 2019 11:21:49 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2] power: supply: ds2782: fix possible use-after-free on remove

In remove(), use cancel_delayed_work_sync() to cancel the
delayed work. Otherwise there's a chance that this work
will continue to run until after the device has been removed.

While we're here, fix the deallocation order in remove(),
to correspond to the inverse of the probe() allocation
order. This guarantees that any remaining work can run
to completion with all driver structures still intact.

This issue was detected with the help of Coccinelle.

Signed-off-by: Sven Van Asbroeck <TheSven73@...il.com>
---

v2: fix silly use-after-free in remove()

 drivers/power/supply/ds2782_battery.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/ds2782_battery.c b/drivers/power/supply/ds2782_battery.c
index 019c58493e3d..04b0fe7d7d62 100644
--- a/drivers/power/supply/ds2782_battery.c
+++ b/drivers/power/supply/ds2782_battery.c
@@ -319,17 +319,17 @@ static void ds278x_power_supply_init(struct power_supply_desc *battery)
 static int ds278x_battery_remove(struct i2c_client *client)
 {
 	struct ds278x_info *info = i2c_get_clientdata(client);
+	int id = info->id;
 
 	power_supply_unregister(info->battery);
+	cancel_delayed_work_sync(&info->bat_work);
 	kfree(info->battery_desc.name);
+	kfree(info);
 
 	mutex_lock(&battery_lock);
-	idr_remove(&battery_id, info->id);
+	idr_remove(&battery_id, id);
 	mutex_unlock(&battery_lock);
 
-	cancel_delayed_work(&info->bat_work);
-
-	kfree(info);
 	return 0;
 }
 
-- 
2.17.1