Skip to content

Commit 1d4e3a6

Browse files
committed
spi: Fix null dereference on suspend
JIRA: https://issues.redhat.com/browse/RHEL-38218 CVE: CVE-2023-52749 Conflicts: Due to missing commit 82238d2 ("spi: Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS") some context diffs were introduced. That commit touches too much of SPI, let's take that as part of a subsystem upgrade but not for this one patch. commit bef4a48 Author: Mark Hasemeyer <markhas@chromium.org> Date: Tue Nov 7 14:47:43 2023 -0700 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 Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
1 parent dff1218 commit 1d4e3a6

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
@@ -3303,33 +3303,52 @@ void spi_unregister_controller(struct spi_controller *ctlr)
33033303
}
33043304
EXPORT_SYMBOL_GPL(spi_unregister_controller);
33053305

3306+
static inline int __spi_check_suspended(const struct spi_controller *ctlr)
3307+
{
3308+
return ctlr->flags & SPI_CONTROLLER_SUSPENDED ? -ESHUTDOWN : 0;
3309+
}
3310+
3311+
static inline void __spi_mark_suspended(struct spi_controller *ctlr)
3312+
{
3313+
mutex_lock(&ctlr->bus_lock_mutex);
3314+
ctlr->flags |= SPI_CONTROLLER_SUSPENDED;
3315+
mutex_unlock(&ctlr->bus_lock_mutex);
3316+
}
3317+
3318+
static inline void __spi_mark_resumed(struct spi_controller *ctlr)
3319+
{
3320+
mutex_lock(&ctlr->bus_lock_mutex);
3321+
ctlr->flags &= ~SPI_CONTROLLER_SUSPENDED;
3322+
mutex_unlock(&ctlr->bus_lock_mutex);
3323+
}
3324+
33063325
int spi_controller_suspend(struct spi_controller *ctlr)
33073326
{
3308-
int ret;
3327+
int ret = 0;
33093328

33103329
/* Basically no-ops for non-queued controllers */
3311-
if (!ctlr->queued)
3312-
return 0;
3313-
3314-
ret = spi_stop_queue(ctlr);
3315-
if (ret)
3316-
dev_err(&ctlr->dev, "queue stop failed\n");
3330+
if (ctlr->queued) {
3331+
ret = spi_stop_queue(ctlr);
3332+
if (ret)
3333+
dev_err(&ctlr->dev, "queue stop failed\n");
3334+
}
33173335

3336+
__spi_mark_suspended(ctlr);
33183337
return ret;
33193338
}
33203339
EXPORT_SYMBOL_GPL(spi_controller_suspend);
33213340

33223341
int spi_controller_resume(struct spi_controller *ctlr)
33233342
{
3324-
int ret;
3325-
3326-
if (!ctlr->queued)
3327-
return 0;
3343+
int ret = 0;
33283344

3329-
ret = spi_start_queue(ctlr);
3330-
if (ret)
3331-
dev_err(&ctlr->dev, "queue restart failed\n");
3345+
__spi_mark_resumed(ctlr);
33323346

3347+
if (ctlr->queued) {
3348+
ret = spi_start_queue(ctlr);
3349+
if (ret)
3350+
dev_err(&ctlr->dev, "queue restart failed\n");
3351+
}
33333352
return ret;
33343353
}
33353354
EXPORT_SYMBOL_GPL(spi_controller_resume);
@@ -4054,8 +4073,7 @@ static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct s
40544073
ctlr->cur_msg = msg;
40554074
ret = __spi_pump_transfer_message(ctlr, msg, was_busy);
40564075
if (ret)
4057-
goto out;
4058-
4076+
dev_err(&ctlr->dev, "noqueue transfer failed\n");
40594077
ctlr->cur_msg = NULL;
40604078
ctlr->fallback = false;
40614079

@@ -4071,7 +4089,6 @@ static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct s
40714089
spi_idle_runtime_pm(ctlr);
40724090
}
40734091

4074-
out:
40754092
mutex_unlock(&ctlr->io_mutex);
40764093
}
40774094

@@ -4094,6 +4111,11 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
40944111
int status;
40954112
struct spi_controller *ctlr = spi->controller;
40964113

4114+
if (__spi_check_suspended(ctlr)) {
4115+
dev_warn_once(&spi->dev, "Attempted to sync while suspend\n");
4116+
return -ESHUTDOWN;
4117+
}
4118+
40974119
status = __spi_validate(spi, message);
40984120
if (status != 0)
40994121
return status;

include/linux/spi/spi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,7 @@ struct spi_controller {
541541
#define SPI_CONTROLLER_MUST_TX BIT(4) /* Requires tx */
542542

543543
#define SPI_MASTER_GPIO_SS BIT(5) /* GPIO CS must select slave */
544+
#define SPI_CONTROLLER_SUSPENDED BIT(6) /* Currently suspended */
544545

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

0 commit comments

Comments
 (0)