[PATCH 2/9] driver core: prevent device_add() during system shutdown
David Jeffery <[email protected]>
| Newsgroups | org.infradead.lists.kexec,dev.linux.lists.driver-core,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.linux-scsi |
|---|---|
| Message-ID | <[email protected]> |
From: Tarun Sahu <[email protected]> In Async device shutdown, device_kset->list lock is released to handle asynchronisation and hold again to get entry from device_kset->list. Which will leave window when device_add can try to add the device to device_kset list and temper with ongoing shutdown process. New added device can be async type or sync type and might also introduce new dependency which can cause device_shutdown path to deadlock. S is waiting C to finish but C is never scheduled as it was added recently from device_add path. And C can only be scheduled when main loops continue to reach to C which is waiting on S. So, When a system enters shutdown (SYSTEM_HALT, SYSTEM_POWER_OFF, or SYSTEM_RESTART), new devices should not be allowed to be added. Adding system_state check (system_is_shutting_down()) to avoid device_add incase of these states of the system. While device_add() performs an initial check of system_is_shutting_down(), a race window exists between this initial check and kobject_add(), during which device_shutdown() may already be scanning devices_kset->list. If device_shutdown() passes the device after kobject_add() registers it onto devices_kset->list, device_add() would otherwise complete device initialization and driver matching, leaving an active device running after system shutdown finishes. Fix this TOCTOU race by re-checking system_is_shutting_down() under devices_kset->list_lock right after kobject_add(). Signed-off-by: Tarun Sahu <[email protected]> Signed-off-by: David Jeffery <[email protected]> Tested-by: Laurence Oberman <[email protected]> --- drivers/base/core.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 83263e3fa5d4..bd9c2921e326 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -47,6 +47,22 @@ static bool fw_devlink_drv_reg_done; static bool fw_devlink_best_effort; static struct workqueue_struct *device_link_wq; +/** + * system_is_shutting_down - Check if system state is not active. + * + * When system state is not active and in shutdown state, new devices + * should not be allowed to be added. + * + * If system_state is SYSTEM_HALT || SYSTEM_POWER_OFF || SYSTEM_RESTART + * this function will return true. + */ +static inline bool system_is_shutting_down(void) +{ + return system_state == SYSTEM_HALT || + system_state == SYSTEM_POWER_OFF || + system_state == SYSTEM_RESTART; +} + /** * __fwnode_link_add - Create a link between two fwnode_handles. * @con: Consumer end of the link. @@ -3654,6 +3670,11 @@ int device_add(struct device *dev) if (!dev) goto done; + if (unlikely(system_is_shutting_down())) { + error = -ESHUTDOWN; + goto done; + } + if (!dev->p) { error = device_private_init(dev); if (error) @@ -3703,6 +3724,18 @@ int device_add(struct device *dev) goto Error; } + /* + * Check system_state again under list_lock to prevent a TOCTOU race + * where device_shutdown() runs concurrently and misses this device. + */ + spin_lock(&devices_kset->list_lock); + if (unlikely(system_is_shutting_down())) { + spin_unlock(&devices_kset->list_lock); + error = -ESHUTDOWN; + goto ShutdownError; + } + spin_unlock(&devices_kset->list_lock); + /* notify platform of device entry */ device_platform_notify(dev); @@ -3822,6 +3855,7 @@ int device_add(struct device *dev) attrError: device_platform_notify_remove(dev); kobject_uevent(&dev->kobj, KOBJ_REMOVE); + ShutdownError: glue_dir = get_glue_dir(dev); kobject_del(&dev->kobj); Error: -- 2.55.0