[PATCH 3/3] driver core: Avoid adding children below a dead parent

From: Lukas Wunner
Date: Wed Jul 08 2020 - 09:38:00 EST


If CONFIG_OF_DYNAMIC or CONFIG_ACPI is enabled, SPI devices may be added
below a controller at runtime by a DeviceTree overlay or DSDT patch.
But there are no precautions to prevent adding a device below a
controller that's being removed.

This seems like something that should be guarded against in the driver
core because it's not specific to SPI: Adding a child below a parent
that's going away seems like a bad idea regardless of the bus type.

Take advantage of kill_device() which was added by commit 00289cd87676
("drivers/base: Introduce kill_device()"), call it upon removal of an
SPI controller and teach the driver core to refuse device addition below
a killed parent. To make this race-free, device_add() needs to take the
parent's dead_sem before checking its "dead" flag and until the child
device has been added to the parent's klist_children.

Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
Cc: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
Cc: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
---
drivers/base/core.c | 18 ++++++++++++++++--
drivers/spi/spi.c | 3 +++
2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 057da42b1a660..1d4e39696f996 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2597,6 +2597,14 @@ int device_add(struct device *dev)
pr_debug("device: '%s': %s\n", dev_name(dev), __func__);

parent = get_device(dev->parent);
+ if (parent) {
+ down_read(&parent->p->dead_sem);
+ if (parent->p->dead) {
+ error = -ENODEV;
+ goto parent_error;
+ }
+ }
+
kobj = get_device_parent(dev, parent);
if (IS_ERR(kobj)) {
error = PTR_ERR(kobj);
@@ -2679,9 +2687,11 @@ int device_add(struct device *dev)
}

bus_probe_device(dev);
- if (parent)
+ if (parent) {
klist_add_tail(&dev->p->knode_parent,
&parent->p->klist_children);
+ up_read(&parent->p->dead_sem);
+ }

if (dev->class) {
mutex_lock(&dev->class->p->mutex);
@@ -2722,6 +2732,8 @@ int device_add(struct device *dev)
Error:
cleanup_glue_dir(dev, glue_dir);
parent_error:
+ if (parent)
+ up_read(&parent->p->dead_sem);
put_device(parent);
name_error:
kfree(dev->p);
@@ -2785,7 +2797,9 @@ EXPORT_SYMBOL_GPL(put_device);
* kill_device - declare device dead
* @dev: device in question
*
- * Declare @dev dead to prevent it from binding to a driver.
+ * Declare @dev dead to prevent it from binding to a driver and
+ * to prevent addition of children.
+ *
* Return true if it was killed or false if it was already dead.
*/
bool kill_device(struct device *dev)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8158e281f3540..005eca4bae089 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2764,6 +2764,9 @@ void spi_unregister_controller(struct spi_controller *ctlr)
struct spi_controller *found;
int id = ctlr->bus_num;

+ /* Prevent addition of new children, then remove existing ones */
+ if (IS_ENABLED(CONFIG_OF_DYNAMIC) || IS_ENABLED(CONFIG_ACPI))
+ kill_device(&ctlr->dev);
device_for_each_child(&ctlr->dev, NULL, __unregister);

/* First make sure that this controller was ever added */
--
2.27.0