[PATCH v2] driver core: Don't take the device lock in waiting_for_supplier_show()

[Nickolai Zeldovich]

Reading a device's waiting_for_supplier attribute while the device is
being probed hangs the reader and the probing task for good if the probe
succeeds: driver_bound() removes the file under the device lock,
kernfs_drain() waits for the reader's active reference, and the reader
waits in device_lock(). The file is world readable and exists for every
unbound device with a firmware node under the default fw_devlink=on, so
any sysfs walk during a probe (udev, a plain cat) can wedge a module
load and everything that takes that device's lock afterwards.

The device lock is not needed: fwnode_links_check_suppliers() reads the
supplier list under fwnode_link_lock, which every writer holds, and the
active reference keeps the device alive during the callback. Drop it.

Reproduced on x86_64 under QEMU with sym53c8xx as a module, whose probe
waits for the SCSI settle delay: a read of the PCI device's
waiting_for_supplier file during the load hangs before the change and
returns 0 after it.

Fixes: da6d647598a6 ("driver core: Add waiting_for_supplier sysfs file for devices")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: LLM
Signed-off-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
---

Notes:
    Changes in v2:
      - Cc: stable (Greg's patch bot: the Fixes commit is in a released
        kernel)
      - shorter commit message
    v1: https://lore.kernel.org/all/20261018001837.797946-1-nickolai@xxxxxxxxxxxxx/
    
    Lockdep does not report the hang: dev->mutex is lockdep-novalidate.
    
    Every writer of the fwnode supplier list (fwnode_link_add(), the purge
    helpers, __fwnode_links_move_consumers() via
    fw_devlink_pickup_dangling_consumers(), and the cycle/link code under
    fw_devlink_link_device()) holds fwnode_link_lock, so the walk in
    fwnode_links_check_suppliers() needs nothing more.
    
    Reproducer (x86_64 defconfig + CONFIG_SCSI_SYM53C8XX_2=m + CONFIG_DETECT_HUNG_TASK,
    QEMU with -device lsi53c895a,id=scsi0 -device scsi-hd,drive=d0,bus=scsi0.0; the
    PCI device has an ACPI companion, so it owns a waiting_for_supplier file until a
    driver binds; sym2_probe() resets the SCSI bus and the synchronous scan waits for
    the settle delay, so the probe takes "settle" seconds). Busybox /init, with
    sym53c8xx.ko in the initramfs:
    
    #!/bin/sh
    mount -t proc proc /proc
    mount -t sysfs sysfs /sys
    mount -t devtmpfs devtmpfs /dev
    for d in /sys/bus/pci/devices/*; do
    	read v < $d/vendor; read i < $d/device
    	[ "$v $i" = "0x1000 0x0012" ] && dev=${d##*/}
    done
    (insmod /sym53c8xx.ko settle=8 && echo "init: module load done") &
    sleep 1
    (echo "init: read returned $(cat /sys/bus/pci/devices/$dev/waiting_for_supplier), attribute access done") &
    sleep 30
    echo "init: after 30 s:"
    ps | grep -E 'insmod|cat /sys'
    echo w > /proc/sysrq-trigger
    
    qemu-system-x86_64 -cpu max -smp 2 -m 512M -nographic -no-reboot -kernel bzImage \
        -initrd initramfs.cpio -drive file=disk.img,if=none,id=d0,format=raw \
        -device lsi53c895a,id=scsi0 -device scsi-hd,drive=d0,bus=scsi0.0 \
        -append "console=ttyS0 panic=-1 rdinit=/init loglevel=7"
    
    Before:
      INFO: task insmod:72 blocked for more than 10 seconds.
       kernfs_drain+0xf6/0x170
       __kernfs_remove+0x188/0x250
       kernfs_remove_by_name_ns+0x78/0xd0
       device_links_driver_bound+0xd4/0x350
       driver_bound+0x46/0x180
       really_probe+0x1b1/0x360
       __driver_attach+0xfc/0x1d0
       bus_add_driver+0x15c/0x270
       init_module+0xc9/0xff0 [sym53c8xx]
      INFO: task cat:79 blocked for more than 10 seconds.
       __mutex_lock+0x395/0xa00
       waiting_for_supplier_show+0x1f/0xa0
       dev_attr_show+0x17/0x60
      INFO: task cat:79 is blocked on a mutex likely owned by task insmod:72.
      init: after 30 s:
         72 0        {insmod} /bin/sh /init
         79 0        cat /sys/bus/pci/devices/0000:00:04.0/waiting_for_supplier
      (a power-off afterwards hangs too: device_shutdown() takes the same lock)
    After:
      init: read returned 0, attribute access done
      sd 2:0:0:0: [sda] Attached SCSI disk
      init: module load done

 drivers/base/core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4c0c373998a1..a3a28d83d339 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1317,10 +1317,8 @@ static ssize_t waiting_for_supplier_show(struct device *dev,
 {
 	bool val;
 
-	device_lock(dev);
 	scoped_guard(mutex, &fwnode_link_lock)
 		val = !!fwnode_links_check_suppliers(dev->fwnode);
-	device_unlock(dev);
 	return sysfs_emit(buf, "%u\n", val);
 }
 static const DEVICE_ATTR_RO(waiting_for_supplier);

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.53.0