power: remove possible deadlock when unregistering power_supply
If a device gets removed right after having registered a power_supply node,
we might enter in a deadlock between the remove call (that has a lock on
the parent device) and the deferred register work.
Allow the deferred register work to exit without taking the lock when
we are in the remove state.
Stack trace on a Ubuntu 16.04:
[16072.109121] INFO: task kworker/u16:2:1180 blocked for more than 120 seconds.
[16072.109127] Not tainted 4.13.0-41-generic #46~16.04.1-Ubuntu
[16072.109129] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[16072.109132] kworker/u16:2 D 0 1180 2 0x80000000
[16072.109142] Workqueue: events_power_efficient power_supply_deferred_register_work
[16072.109144] Call Trace:
[16072.109152] __schedule+0x3d6/0x8b0
[16072.109155] schedule+0x36/0x80
[16072.109158] schedule_preempt_disabled+0xe/0x10
[16072.109161] __mutex_lock.isra.2+0x2ab/0x4e0
[16072.109166] __mutex_lock_slowpath+0x13/0x20
[16072.109168] ? __mutex_lock_slowpath+0x13/0x20
[16072.109171] mutex_lock+0x2f/0x40
[16072.109174] power_supply_deferred_register_work+0x2b/0x50
[16072.109179] process_one_work+0x15b/0x410
[16072.109182] worker_thread+0x4b/0x460
[16072.109186] kthread+0x10c/0x140
[16072.109189] ? process_one_work+0x410/0x410
[16072.109191] ? kthread_create_on_node+0x70/0x70
[16072.109194] ret_from_fork+0x35/0x40
[16072.109199] INFO: task test:2257 blocked for more than 120 seconds.
[16072.109202] Not tainted 4.13.0-41-generic #46~16.04.1-Ubuntu
[16072.109204] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[16072.109206] test D 0 2257 2256 0x00000004
[16072.109208] Call Trace:
[16072.109211] __schedule+0x3d6/0x8b0
[16072.109215] schedule+0x36/0x80
[16072.109218] schedule_timeout+0x1f3/0x360
[16072.109221] ? check_preempt_curr+0x5a/0xa0
[16072.109224] ? ttwu_do_wakeup+0x1e/0x150
[16072.109227] wait_for_completion+0xb4/0x140
[16072.109230] ? wait_for_completion+0xb4/0x140
[16072.109233] ? wake_up_q+0x70/0x70
[16072.109236] flush_work+0x129/0x1e0
[16072.109240] ? worker_detach_from_pool+0xb0/0xb0
[16072.109243] __cancel_work_timer+0x10f/0x190
[16072.109247] ? device_del+0x264/0x310
[16072.109250] ? __wake_up+0x44/0x50
[16072.109253] cancel_delayed_work_sync+0x13/0x20
[16072.109257] power_supply_unregister+0x37/0xb0
[16072.109260] devm_power_supply_release+0x11/0x20
[16072.109263] release_nodes+0x110/0x200
[16072.109266] devres_release_group+0x7c/0xb0
[16072.109274] wacom_remove+0xc2/0x110 [wacom]
[16072.109279] hid_device_remove+0x6e/0xd0 [hid]
[16072.109284] device_release_driver_internal+0x158/0x210
[16072.109288] device_release_driver+0x12/0x20
[16072.109291] bus_remove_device+0xec/0x160
[16072.109293] device_del+0x1de/0x310
[16072.109298] hid_destroy_device+0x27/0x60 [hid]
[16072.109303] usbhid_disconnect+0x51/0x70 [usbhid]
[16072.109308] usb_unbind_interface+0x77/0x270
[16072.109311] device_release_driver_internal+0x158/0x210
[16072.109315] device_release_driver+0x12/0x20
[16072.109318] usb_driver_release_interface+0x77/0x80
[16072.109321] proc_ioctl+0x20f/0x250
[16072.109325] usbdev_do_ioctl+0x57f/0x1140
[16072.109327] ? __wake_up+0x44/0x50
[16072.109331] usbdev_ioctl+0xe/0x20
[16072.109336] do_vfs_ioctl+0xa4/0x600
[16072.109339] ? vfs_write+0x15a/0x1b0
[16072.109343] SyS_ioctl+0x79/0x90
[16072.109347] entry_SYSCALL_64_fastpath+0x24/0xab
[16072.109349] RIP: 0033:0x7f20da807f47
[16072.109351] RSP: 002b:00007ffc422ae398 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[16072.109353] RAX: ffffffffffffffda RBX: 00000000010b8560 RCX: 00007f20da807f47
[16072.109355] RDX: 00007ffc422ae3a0 RSI: 00000000c0105512 RDI: 0000000000000009
[16072.109356] RBP: 0000000000000000 R08: 00007ffc422ae3e0 R09: 0000000000000010
[16072.109357] R10: 00000000000000a6 R11: 0000000000000246 R12: 0000000000000000
[16072.109359] R13: 00000000010b8560 R14: 00007ffc422ae2e0 R15: 0000000000000000
Reported-and-tested-by: Richard Hughes <rhughes@redhat.com>
Tested-by: Aaron Skomra <Aaron.Skomra@wacom.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Fixes: 7f1a57fdd6
("power_supply: Fix possible NULL pointer dereference on early uevent")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
This commit is contained in:
parent
ce397d215c
commit
3ffa6583e2
|
@ -14,6 +14,7 @@
|
||||||
#include <linux/types.h>
|
#include <linux/types.h>
|
||||||
#include <linux/init.h>
|
#include <linux/init.h>
|
||||||
#include <linux/slab.h>
|
#include <linux/slab.h>
|
||||||
|
#include <linux/delay.h>
|
||||||
#include <linux/device.h>
|
#include <linux/device.h>
|
||||||
#include <linux/notifier.h>
|
#include <linux/notifier.h>
|
||||||
#include <linux/err.h>
|
#include <linux/err.h>
|
||||||
|
@ -140,8 +141,13 @@ static void power_supply_deferred_register_work(struct work_struct *work)
|
||||||
struct power_supply *psy = container_of(work, struct power_supply,
|
struct power_supply *psy = container_of(work, struct power_supply,
|
||||||
deferred_register_work.work);
|
deferred_register_work.work);
|
||||||
|
|
||||||
if (psy->dev.parent)
|
if (psy->dev.parent) {
|
||||||
mutex_lock(&psy->dev.parent->mutex);
|
while (!mutex_trylock(&psy->dev.parent->mutex)) {
|
||||||
|
if (psy->removing)
|
||||||
|
return;
|
||||||
|
msleep(10);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
power_supply_changed(psy);
|
power_supply_changed(psy);
|
||||||
|
|
||||||
|
@ -1082,6 +1088,7 @@ EXPORT_SYMBOL_GPL(devm_power_supply_register_no_ws);
|
||||||
void power_supply_unregister(struct power_supply *psy)
|
void power_supply_unregister(struct power_supply *psy)
|
||||||
{
|
{
|
||||||
WARN_ON(atomic_dec_return(&psy->use_cnt));
|
WARN_ON(atomic_dec_return(&psy->use_cnt));
|
||||||
|
psy->removing = true;
|
||||||
cancel_work_sync(&psy->changed_work);
|
cancel_work_sync(&psy->changed_work);
|
||||||
cancel_delayed_work_sync(&psy->deferred_register_work);
|
cancel_delayed_work_sync(&psy->deferred_register_work);
|
||||||
sysfs_remove_link(&psy->dev.kobj, "powers");
|
sysfs_remove_link(&psy->dev.kobj, "powers");
|
||||||
|
|
|
@ -269,6 +269,7 @@ struct power_supply {
|
||||||
spinlock_t changed_lock;
|
spinlock_t changed_lock;
|
||||||
bool changed;
|
bool changed;
|
||||||
bool initialized;
|
bool initialized;
|
||||||
|
bool removing;
|
||||||
atomic_t use_cnt;
|
atomic_t use_cnt;
|
||||||
#ifdef CONFIG_THERMAL
|
#ifdef CONFIG_THERMAL
|
||||||
struct thermal_zone_device *tzd;
|
struct thermal_zone_device *tzd;
|
||||||
|
|
Loading…
Reference in New Issue