Skip to content

Commit bef4a48

Browse files
Mark Hasemeyerbroonie
authored andcommitted
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
1 parent c2ded28 commit bef4a48

File tree

2 files changed

+40
-17
lines changed

2 files changed

+40
-17
lines changed

drivers/spi/spi.c

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3317,33 +3317,52 @@ void spi_unregister_controller(struct spi_controller *ctlr)
33173317
}
33183318
EXPORT_SYMBOL_GPL(spi_unregister_controller);
33193319

3320+
static inline int __spi_check_suspended(const struct spi_controller *ctlr)
3321+
{
3322+
return ctlr->flags & SPI_CONTROLLER_SUSPENDED ? -ESHUTDOWN : 0;
3323+
}
3324+
3325+
static inline void __spi_mark_suspended(struct spi_controller *ctlr)
3326+
{
3327+
mutex_lock(&ctlr->bus_lock_mutex);
3328+
ctlr->flags |= SPI_CONTROLLER_SUSPENDED;
3329+
mutex_unlock(&ctlr->bus_lock_mutex);
3330+
}
3331+
3332+
static inline void __spi_mark_resumed(struct spi_controller *ctlr)
3333+
{
3334+
mutex_lock(&ctlr->bus_lock_mutex);
3335+
ctlr->flags &= ~SPI_CONTROLLER_SUSPENDED;
3336+
mutex_unlock(&ctlr->bus_lock_mutex);
3337+
}
3338+
33203339
int spi_controller_suspend(struct spi_controller *ctlr)
33213340
{
3322-
int ret;
3341+
int ret = 0;
33233342

33243343
/* Basically no-ops for non-queued controllers */
3325-
if (!ctlr->queued)
3326-
return 0;
3327-
3328-
ret = spi_stop_queue(ctlr);
3329-
if (ret)
3330-
dev_err(&ctlr->dev, "queue stop failed\n");
3344+
if (ctlr->queued) {
3345+
ret = spi_stop_queue(ctlr);
3346+
if (ret)
3347+
dev_err(&ctlr->dev, "queue stop failed\n");
3348+
}
33313349

3350+
__spi_mark_suspended(ctlr);
33323351
return ret;
33333352
}
33343353
EXPORT_SYMBOL_GPL(spi_controller_suspend);
33353354

33363355
int spi_controller_resume(struct spi_controller *ctlr)
33373356
{
3338-
int ret;
3339-
3340-
if (!ctlr->queued)
3341-
return 0;
3357+
int ret = 0;
33423358

3343-
ret = spi_start_queue(ctlr);
3344-
if (ret)
3345-
dev_err(&ctlr->dev, "queue restart failed\n");
3359+
__spi_mark_resumed(ctlr);
33463360

3361+
if (ctlr->queued) {
3362+
ret = spi_start_queue(ctlr);
3363+
if (ret)
3364+
dev_err(&ctlr->dev, "queue restart failed\n");
3365+
}
33473366
return ret;
33483367
}
33493368
EXPORT_SYMBOL_GPL(spi_controller_resume);
@@ -4147,8 +4166,7 @@ static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct s
41474166
ctlr->cur_msg = msg;
41484167
ret = __spi_pump_transfer_message(ctlr, msg, was_busy);
41494168
if (ret)
4150-
goto out;
4151-
4169+
dev_err(&ctlr->dev, "noqueue transfer failed\n");
41524170
ctlr->cur_msg = NULL;
41534171
ctlr->fallback = false;
41544172

@@ -4164,7 +4182,6 @@ static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct s
41644182
spi_idle_runtime_pm(ctlr);
41654183
}
41664184

4167-
out:
41684185
mutex_unlock(&ctlr->io_mutex);
41694186
}
41704187

@@ -4187,6 +4204,11 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
41874204
int status;
41884205
struct spi_controller *ctlr = spi->controller;
41894206

4207+
if (__spi_check_suspended(ctlr)) {
4208+
dev_warn_once(&spi->dev, "Attempted to sync while suspend\n");
4209+
return -ESHUTDOWN;
4210+
}
4211+
41904212
status = __spi_validate(spi, message);
41914213
if (status != 0)
41924214
return status;

include/linux/spi/spi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ struct spi_controller {
566566
#define SPI_CONTROLLER_MUST_RX BIT(3) /* Requires rx */
567567
#define SPI_CONTROLLER_MUST_TX BIT(4) /* Requires tx */
568568
#define SPI_CONTROLLER_GPIO_SS BIT(5) /* GPIO CS must select slave */
569+
#define SPI_CONTROLLER_SUSPENDED BIT(6) /* Currently suspended */
569570

570571
/* Flag indicating if the allocation of this struct is devres-managed */
571572
bool devm_allocated;

0 commit comments

Comments
 (0)