linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).