linux-kernel - [PATCH 4.9 011/175] spi: Prevent adding devices below an unregistering controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20201228124853.801123776@linuxfoundation.org>
Date:   Mon, 28 Dec 2020 13:47:44 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Lukas Wunner <lukas@...ner.de>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Octavian Purdila <octavian.purdila@...el.com>,
        Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
        Mark Brown <broonie@...nel.org>,
        Sudip Mukherjee <sudipm.mukherjee@...il.com>
Subject: [PATCH 4.9 011/175] spi: Prevent adding devices below an unregistering controller

From: Lukas Wunner <lukas@...ner.de>

commit ddf75be47ca748f8b12d28ac64d624354fddf189 upstream

CONFIG_OF_DYNAMIC and CONFIG_ACPI allow adding SPI devices at runtime
using a DeviceTree overlay or DSDT patch.  CONFIG_SPI_SLAVE allows the
same via sysfs.

But there are no precautions to prevent adding a device below a
controller that's being removed.  Such a device is unusable and may not
even be able to unbind cleanly as it becomes inaccessible once the
controller has been torn down.  E.g. it is then impossible to quiesce
the device's interrupt.

of_spi_notify() and acpi_spi_notify() do hold a ref on the controller,
but otherwise run lockless against spi_unregister_controller().

Fix by holding the spi_add_lock in spi_unregister_controller() and
bailing out of spi_add_device() if the controller has been unregistered
concurrently.

Fixes: ce79d54ae447 ("spi/of: Add OF notifier handler")
Signed-off-by: Lukas Wunner <lukas@...ner.de>
Cc: stable@...r.kernel.org # v3.19+
Cc: Geert Uytterhoeven <geert+renesas@...der.be>
Cc: Octavian Purdila <octavian.purdila@...el.com>
Cc: Pantelis Antoniou <pantelis.antoniou@...sulko.com>
Link: https://lore.kernel.org/r/a8c3205088a969dc8410eec1eba9aface60f36af.1596451035.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@...nel.org>
[sudip: adjust context]
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@...il.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 drivers/spi/Kconfig |    3 +++
 drivers/spi/spi.c   |   21 ++++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -763,4 +763,7 @@ endif # SPI_MASTER
 
 # (slave support would go here)
 
+config SPI_DYNAMIC
+	def_bool ACPI || OF_DYNAMIC || SPI_SLAVE
+
 endif # SPI
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -422,6 +422,12 @@ static LIST_HEAD(spi_master_list);
  */
 static DEFINE_MUTEX(board_lock);
 
+/*
+ * Prevents addition of devices with same chip select and
+ * addition of devices below an unregistering controller.
+ */
+static DEFINE_MUTEX(spi_add_lock);
+
 /**
  * spi_alloc_device - Allocate a new SPI device
  * @master: Controller to which device is connected
@@ -500,7 +506,6 @@ static int spi_dev_check(struct device *
  */
 int spi_add_device(struct spi_device *spi)
 {
-	static DEFINE_MUTEX(spi_add_lock);
 	struct spi_master *master = spi->master;
 	struct device *dev = master->dev.parent;
 	int status;
@@ -529,6 +534,13 @@ int spi_add_device(struct spi_device *sp
 		goto done;
 	}
 
+	/* Controller may unregister concurrently */
+	if (IS_ENABLED(CONFIG_SPI_DYNAMIC) &&
+	    !device_is_registered(&master->dev)) {
+		status = -ENODEV;
+		goto done;
+	}
+
 	if (master->cs_gpios)
 		spi->cs_gpio = master->cs_gpios[spi->chip_select];
 
@@ -2070,6 +2082,10 @@ static int __unregister(struct device *d
  */
 void spi_unregister_master(struct spi_master *master)
 {
+	/* Prevent addition of new devices, unregister existing ones */
+	if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
+		mutex_lock(&spi_add_lock);
+
 	device_for_each_child(&master->dev, NULL, __unregister);
 
 	if (master->queued) {
@@ -2089,6 +2105,9 @@ void spi_unregister_master(struct spi_ma
 	if (!devres_find(master->dev.parent, devm_spi_release_master,
 			 devm_spi_match_master, master))
 		put_device(&master->dev);
+
+	if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
+		mutex_unlock(&spi_add_lock);
 }
 EXPORT_SYMBOL_GPL(spi_unregister_master);