Re: [PATCH v1] driver core: Use list_del_init to replace list_del at device_links_purge()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/01/2020 11:34, Luo Jiaxing wrote:

+ linux-scsi, Martin

We found that enabling kernel compilation options CONFIG_SCSI_ENCLOSURE and
CONFIG_ENCLOSURE_SERVICES, repeated initialization and deletion of the same
SCSI device will cause system panic, as follows:
[72.425705] Unable to handle kernel paging request at virtual address
dead000000000108
...
[72.595093] Call trace:
[72.597532] device_del + 0x194 / 0x3a0
[72.601012] enclosure_remove_device + 0xbc / 0xf8
[72.605445] ses_intf_remove + 0x9c / 0xd8
[72.609185] device_del + 0xf8 / 0x3a0
[72.612576] device_unregister + 0x14 / 0x30
[72.616489] __scsi_remove_device + 0xf4 / 0x140
[72.620747] scsi_remove_device + 0x28 / 0x40
[72.624745] scsi_remove_target + 0x1c8 / 0x220

please share the full crash stack frame and the commands used to trigger it. Some people prefer the timestamp removed also.


After analysis, we see that in the error scenario, the ses module has the
following calling sequence:
device_register() -> device_del() -> device_add() -> device_del().
The first call to device_del() is fine, but the second call to device_del()
will cause a system panic.

Through disassembly, we locate that panic happen when device_links_purge()
call list_del() to remove device_links.needs_suppliers from list, and
list_del() will set this list entry's prev and next pointers to poison.
So if INIT_LIST_HEAD() is not re-executed before the next list_del(), It
will cause the system to access a memory address which is posioned.

Therefore, replace list_del() with list_del_init() can avoid such issue.

Fixes: e2ae9bcc4aaa ("driver core: Add support for linking devices during device addition")
Signed-off-by: Luo Jiaxing <luojiaxing@xxxxxxxxxx>
Reviewed-by: John Garry <john.garry@xxxxxxxxxx>

This tag was only implicitly granted, but I thought that the fix looked ok, so:

Reviewed-by: John Garry <john.garry@xxxxxxxxxx>

---
  drivers/base/core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 42a6724..7b9b0d6 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1103,7 +1103,7 @@ static void device_links_purge(struct device *dev)
  	struct device_link *link, *ln;
mutex_lock(&wfs_lock);
-	list_del(&dev->links.needs_suppliers);
+	list_del_init(&dev->links.needs_suppliers);
  	mutex_unlock(&wfs_lock);
/*





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux