From bef4a48f4ef798c4feddf045d49e53c8a97d5e37 Mon Sep 17 00:00:00 2001 From: Mark Hasemeyer Date: Tue, 7 Nov 2023 14:47:43 -0700 Subject: 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 Link: https://lore.kernel.org/r/20231107144743.v1.1.I7987f05f61901f567f7661763646cb7d7919b528@changeid Signed-off-by: Mark Brown Cc: stable@kernel.org --- drivers/spi/spi.c | 56 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 17 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 791df0e69105..8ead7acb99f3 100644 --- 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; -- cgit