linux-kernel - Re: Possible hungtask issue will be introduced with device_lock() in uevent

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-prev] [day] [month] [year] [list]

Message-ID: <7b535c35-9aeb-4a60-ab42-438f70835137@huawei.com>
Date: Mon, 27 Jan 2025 11:27:01 +0800
From: "zhangzekun (A)" <zhangzekun11@...wei.com>
To: Dan Williams <dan.j.williams@...el.com>
CC: <linux-kernel@...r.kernel.org>, <rafael.j.wysocki@...el.com>,
	<regressions@...ts.linux.dev>, <tj@...nel.org>, <alex.williamson@...hat.com>,
	<chenjun102@...wei.com>, Greg KH <gregkh@...uxfoundation.org>
Subject: Re: Possible hungtask issue will be introduced with device_lock() in
 uevent_show()


> Hi Zekun,
> 
> If you have some cycles to help investigate the replacement fix that
> would be much appreciated.
> 
> So far I came up with this:
> 
> http://lore.kernel.org/172790598832.1168608.4519484276671503678.stgit@dwillia2-xfh.jf.intel.com
> 
> ...but have not had time to debug the 0day report.
> 
> My worry is still that Linux has long since shipped the expectation that
> reading 'uevent' bounces the device_lock() which, among other things,
> makes sure that any in-flight driver probing has completed.
> 
> The report of USB devices disappearing is consistent with a udev
> operation failing due to the driver not being done attaching, or
> something similar.
> 
> So even though you have not seen any issues, I suspect small differences
> in the devices on your system and the reporter's system, or udev rule
> differences could result in a failure to trigger the regression.
> 
> 
Hi, Dan

The patch trys to prevent the potential dead lock by letting 
device_lock() gets before kernfs_get_active(), but I think it might not 
be enough to solve all the potential dead lock. The device_lock() will 
be held while removing the driver in device_release_driver(), which 
means will be held through driver's .remove(). If .remove() is waiting 
for resources to be released by userspace process, and then the 
userspace process call device_lock() in uevent_show(), we will have dead 
locks here.

In the following case:

device_release_driver
device_lock()          blocked by
<operation 1> ---------------------------> <operation 2>
                                            uevent_show()
                                            device_lock()


In [1], It is beacause that pci_stop_and_remove_bus_device() will call 
device_release_driver() first and then call device_del() to remove the 
uevent sysfs attributes （Sorry for mistakes made in [1], 
device_release_driver() is called by pci_stop_bus_device()). So, I think 
it might not be a good idea to hold the device_lock() through uevent_show().

ioctl(..,VFIO_GROUP_GET_DEVICE_FD,..)
         ...
         vfio_device_get_from_name()
         vfio_devcie->refcount -> 2
                                         pci_stop_and_remove_bus_device()
                                         pci_stop_bus_device（）
                                         device_release_driver()
                                         device_lock()
                                         device_remove()
												vfio_unregister_group_dev()
												vfio_device_put_registration()
												vfio_devcie->refcount -> 1
												wait for refcount == 0
uevent_show()
         device_lock()

[1] 
https://lore.kernel.org/lkml/20241231075608.84009-1-zhangzekun11@huawei.com/

Best Regards,
Zekun