spi: Fix null dereference on suspend

A race condition exists where a synchronous (noqueue) transfer can be
active during a system suspend. This can cause a null pointer
dereference exception to occur when the system resumes.

Example order of events leading to the exception:
1. spi_sync() calls __spi_transfer_message_noqueue() which sets
   ctlr->cur_msg
2. Spi transfer begins via spi_transfer_one_message()
3. System is suspended interrupting the transfer context
4. System is resumed
6. spi_controller_resume() calls spi_start_queue() which resets cur_msg
   to NULL
7. Spi transfer context resumes and spi_finalize_current_message() is
   called which dereferences cur_msg (which is now NULL)

Wait for synchronous transfers to complete before suspending by
acquiring the bus mutex and setting/checking a suspend flag.

Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
Link: https://lore.kernel.org/r/20231107144743.v1.1.I7987f05f61901f567f7661763646cb7d7919b528@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@kernel.org

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 791df0e691054762f139a49df38bb66ea1b14fcf..8ead7acb99f3481aa941b5e8879ac05444e8b46f 100644 (file)
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3317,33 +3317,52 @@ void spi_unregister_controller(struct spi_controller *ctlr)
 }
 EXPORT_SYMBOL_GPL(spi_unregister_controller);
 
+static inline int __spi_check_suspended(const struct spi_controller *ctlr)
+{
+       return ctlr->flags & SPI_CONTROLLER_SUSPENDED ? -ESHUTDOWN : 0;
+}
+
+static inline void __spi_mark_suspended(struct spi_controller *ctlr)
+{
+       mutex_lock(&ctlr->bus_lock_mutex);
+       ctlr->flags |= SPI_CONTROLLER_SUSPENDED;
+       mutex_unlock(&ctlr->bus_lock_mutex);
+}
+
+static inline void __spi_mark_resumed(struct spi_controller *ctlr)
+{
+       mutex_lock(&ctlr->bus_lock_mutex);
+       ctlr->flags &= ~SPI_CONTROLLER_SUSPENDED;
+       mutex_unlock(&ctlr->bus_lock_mutex);
+}
+
 int spi_controller_suspend(struct spi_controller *ctlr)
 {
-       int ret;
+       int ret = 0;
 
        /* Basically no-ops for non-queued controllers */
-       if (!ctlr->queued)
-               return 0;
-
-       ret = spi_stop_queue(ctlr);
-       if (ret)
-               dev_err(&ctlr->dev, "queue stop failed\n");
+       if (ctlr->queued) {
+               ret = spi_stop_queue(ctlr);
+               if (ret)
+                       dev_err(&ctlr->dev, "queue stop failed\n");
+       }
 
+       __spi_mark_suspended(ctlr);
        return ret;
 }
 EXPORT_SYMBOL_GPL(spi_controller_suspend);
 
 int spi_controller_resume(struct spi_controller *ctlr)
 {
-       int ret;
-
-       if (!ctlr->queued)
-               return 0;
+       int ret = 0;
 
-       ret = spi_start_queue(ctlr);
-       if (ret)
-               dev_err(&ctlr->dev, "queue restart failed\n");
+       __spi_mark_resumed(ctlr);
 
+       if (ctlr->queued) {
+               ret = spi_start_queue(ctlr);
+               if (ret)
+                       dev_err(&ctlr->dev, "queue restart failed\n");
+       }
        return ret;
 }
 EXPORT_SYMBOL_GPL(spi_controller_resume);
@@ -4147,8 +4166,7 @@ static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct s
        ctlr->cur_msg = msg;
        ret = __spi_pump_transfer_message(ctlr, msg, was_busy);
        if (ret)
-               goto out;
-
+               dev_err(&ctlr->dev, "noqueue transfer failed\n");
        ctlr->cur_msg = NULL;
        ctlr->fallback = false;
 
@@ -4164,7 +4182,6 @@ static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct s
                spi_idle_runtime_pm(ctlr);
        }
 
-out:
        mutex_unlock(&ctlr->io_mutex);
 }
 
@@ -4187,6 +4204,11 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
        int status;
        struct spi_controller *ctlr = spi->controller;
 
+       if (__spi_check_suspended(ctlr)) {
+               dev_warn_once(&spi->dev, "Attempted to sync while suspend\n");
+               return -ESHUTDOWN;
+       }
+
        status = __spi_validate(spi, message);
        if (status != 0)
                return status;

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 86825c88b5767a5e6960d89f2a1129189f26d0fd..255a0562aea5a868b4e3bd78ecd2497e8f3f105f 100644 (file)
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -566,6 +566,7 @@ struct spi_controller {
 #define SPI_CONTROLLER_MUST_RX         BIT(3)  /* Requires rx */
 #define SPI_CONTROLLER_MUST_TX         BIT(4)  /* Requires tx */
 #define SPI_CONTROLLER_GPIO_SS         BIT(5)  /* GPIO CS must select slave */
+#define SPI_CONTROLLER_SUSPENDED       BIT(6)  /* Currently suspended */
 
        /* Flag indicating if the allocation of this struct is devres-managed */
        bool                    devm_allocated;