All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
	Alexander Duyck <alexander.h.duyck@linux.intel.com>,
	Mark Brown <broonie@kernel.org>,
	linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org
Subject: [PATCH 3/3] driver core: Avoid adding children below a dead parent
Date: Wed, 8 Jul 2020 15:27:03 +0200	[thread overview]
Message-ID: <f2d349b5ba67b5ca70cb19577725167642eb69c5.1594214103.git.lukas@wunner.de> (raw)
In-Reply-To: <cover.1594214103.git.lukas@wunner.de>

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@wunner.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 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


  parent reply	other threads:[~2020-07-08 13:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-08 13:27 [PATCH 0/3] Fix races on device removal Lukas Wunner
2020-07-08 13:27 ` [PATCH 1/3] driver core: Avoid binding drivers to dead devices Lukas Wunner
2020-07-08 13:27 ` [PATCH 2/3] driver core: Use rwsem for kill_device() serialization Lukas Wunner
2020-07-30  6:53   ` Greg Kroah-Hartman
2020-07-30  9:56     ` Lukas Wunner
2020-07-31  6:45       ` Greg Kroah-Hartman
2020-07-31  9:53         ` Lukas Wunner
2020-07-08 13:27 ` Lukas Wunner [this message]
2020-07-24 14:29   ` [driver core] e3b1cb5c89: WARNING:possible_recursive_locking_detected kernel test robot
2020-07-25 11:07     ` Lukas Wunner
2020-07-30  6:54 ` [PATCH 0/3] Fix races on device removal Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f2d349b5ba67b5ca70cb19577725167642eb69c5.1594214103.git.lukas@wunner.de \
    --to=lukas@wunner.de \
    --cc=alexander.h.duyck@linux.intel.com \
    --cc=broonie@kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=pantelis.antoniou@konsulko.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.