linux-kernel - [PATCH] driver core: Do not add a device that has been removed in device_shutdown to devices

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:   Thu, 21 Jul 2022 20:33:25 +0800
From:   "Jiazi.Li" <jqqlijiazi@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     "Jiazi.Li" <jiazi.li@...nssion.com>, linux-kernel@...r.kernel.org
Subject: [PATCH] driver core: Do not add a device that has been removed in device_shutdown to devices_kset list again.

There is a race between device_shutdown and devices_kset_move_last:
1. device_shutdown remove dev from devices_kset, unlock list_lock

2. supplier call device_link_add move this consummer dev to
last of devices_kset:
devices_kset_move_last+0x184/0x1fc
device_reorder_to_tail+0x50/0x17c
device_link_add+0x670/0x9c0
phy_get+0x88/0x300

3. device_shutdown call dev->bus->shutdown first time

4. in next loop, device_shutdown pick this dev from devices_kset
again, and call dev->bus->shutdown for the second time.

If the protection mechanism of dev->bus->shutdown is not perfect,
the following crash will occur:
[37.366651] [T1000001] Unable to handle kernel NULL pointer
dereference at virtual address 0000000000000070
[37.414053] [T1600001] Kernel Offset: 0x2c73800000 from 0xffffffc010000000
[37.414908] [T1600001] PHYS_OFFSET: 0x40000000
[37.415458] [T1600001] pstate: 60400005 (nZCv daif +PAN -UAO)
[37.416172] [T1600001] pc : [0xffffffec83eccd40] kernfs_find_ns+0x18/0x154
[37.417025] [T1600001] lr : [0xffffffec83ed8714] sysfs_unmerge_group+0x70/0x2e0
[37.417931] [T1600001] sp : ffffffc01008ba90
[37.418456] [T1600001] x29: ffffffc01008ba90 x28: ffffffec863c1000
[37.419221] [T1600001] x27: ffffffec85234e30 x26: ffffffec86490000
[37.419985] [T1600001] x25: ffffff80c854a4c0 x24: ffffff80c854a428
[37.420750] [T1600001] x23: 0000000000400100 x22: ffffffec85c4c5c8
[37.421514] [T1600001] x21: ffffffec85c4c5a0 x20: 0000000000000000
[37.422277] [T1600001] x19: ffffff80c0178000 x18: ffffffc010083068
[37.423043] [T1600001] x17: 0000000000000000 x16: 00000000000000d8
[37.423807] [T1600001] x15: ffffffec842b9314 x14: ffffffec85b04050
[37.424572] [T1600001] x13: 0000000000000000 x12: 000000000000000a
[37.425336] [T1600001] x11: 0000000000000000 x10: ffffffec863e9000
[37.426102] [T1600001] x9 : 0000000000000001 x8 : 0000000000000000
[37.426869] [T1600001] x7 : 332e37332020205b x6 : ffffffec863e6e7e
[37.427633] [T1600001] x5 : ffffffffffffffff x4 : 0000000000000000
[37.428396] [T1600001] x3 : 000000000000003c x2 : 0000000000000000
[37.429161] [T1600001] x1 : ffffffec85c4c5c8 x0 : 0000000000000000
[37.612410] [T1600001] Call trace:
[37.612831] [T1600001]  dump_backtrace.cfi_jt+0x0/0x8
[37.613457] [T1600001]  dump_stack_lvl+0xc4/0x140
[37.614038] [T1600001]  dump_stack+0x1c/0x2c
[37.614588] [T1600001]  mrdump_common_die+0x3a8/0x544 [mrdump]
[37.615326] [T1600001]  ipanic_die+0x24/0x38 [mrdump]
[37.615951] [T1600001]  die+0x344/0x748
[37.616425] [T1600001]  die_kernel_fault+0x84/0x94
[37.617016] [T1600001]  __do_kernel_fault+0x230/0x27c
[37.617642] [T1600001]  do_page_fault+0xb4/0x754
[37.618212] [T1600001]  do_translation_fault+0x48/0x64
[37.618846] [T1600001]  do_mem_abort+0x6c/0x164
[37.619406] [T1600001]  el1_abort+0x44/0x68
[37.619921] [T1600001]  el1_sync_handler+0x58/0x88
[37.620512] [T1600001]  el1_sync+0x8c/0x140
[37.621028] [T1600001]  kernfs_find_ns+0x18/0x154
[37.621608] [T1600001]  sysfs_unmerge_group+0x70/0x2e0
[37.622246] [T1600001]  device_del+0x198/0xd00
[37.622794] [T1600001]  device_unregister+0x1c/0x3c
[37.623409] [T1600001]  charger_device_unregister+0x40/0x54 [charger_class]
[37.624277] [T1600001]  sgm41516d_shutdown+0x54/0x84 [sgm41516d]
[37.625021] [T1600001]  i2c_device_shutdown+0x68/0x118
[37.625656] [T1600001]  device_shutdown+0x234/0x614
[37.626259] [T1600001]  kernel_restart+0x74/0x1e8
[37.626840] [T1600001]  __arm64_sys_reboot+0x3b0/0x424
[37.627475] [T1600001]  el0_svc_common+0xd4/0x270
[37.628056] [T1600001]  el0_svc+0x28/0x88
[37.628549] [T1600001]  el0_sync_handler+0x8c/0xf0
[37.629141] [T1600001]  el0_sync+0x1b4/0x1c0

because dev->kobject.sd has been set to NULL in fisrt shutdown.

Signed-off-by: Jiazi.Li <jiazi.li@...nssion.com>
---
 drivers/base/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 753e7cca0f40..301c75639d85 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2850,6 +2850,12 @@ void devices_kset_move_last(struct device *dev)
 	if (!devices_kset)
 		return;
 	pr_debug("devices_kset: Moving %s to end of list\n", dev_name(dev));
+	/*
+	 * if dev has been removed from devices_kset in device_shutdown,
+	 * just return.
+	 */
+	if (list_empty(&dev->kobj.entry))
+		return
 	spin_lock(&devices_kset->list_lock);
 	list_move_tail(&dev->kobj.entry, &devices_kset->list);
 	spin_unlock(&devices_kset->list_lock);
-- 
2.17.1