| 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
| ||
|
Message-ID: <20260211072616.489522-1-fg@emlix.com>
Date: Wed, 11 Feb 2026 08:26:16 +0100
From: Fabian Godehardt <fg@...ix.com>
To: Mark Brown <broonie@...nel.org>
Cc: linux-spi@...r.kernel.org,
linux-kernel@...r.kernel.org,
Fabian Godehardt <fg@...ix.com>
Subject: [PATCH] spi: spidev: fix lock inversion between spi_lock and buf_lock
The spidev driver previously used two mutexes, spi_lock and buf_lock,
but acquired them in different orders depending on the code path:
write()/read(): buf_lock -> spi_lock
ioctl(): spi_lock -> buf_lock
This AB-BA locking pattern triggers lockdep warnings and can
cause real deadlocks:
WARNING: possible circular locking dependency detected
spidev_ioctl() -> mutex_lock(&spidev->buf_lock)
spidev_sync_write() -> mutex_lock(&spidev->spi_lock)
*** DEADLOCK ***
The issue is reproducible with a simple userspace program that
performs write() and SPI_IOC_WR_MAX_SPEED_HZ ioctl() calls from
separate threads on the same spidev file descriptor.
Fix this by simplifying the locking model and removing the lock
inversion entirely. spidev_sync() no longer performs any locking,
and all callers serialize access using spi_lock.
buf_lock is removed since its functionality is fully covered by
spi_lock, eliminating the possibility of lock ordering issues.
This removes the lock inversion and prevents deadlocks without
changing userspace ABI or behaviour.
Signed-off-by: Fabian Godehardt <fg@...ix.com>
---
drivers/spi/spidev.c | 63 ++++++++++++++++----------------------------
1 file changed, 22 insertions(+), 41 deletions(-)
diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 9a0160f6dc3d..f28528ed1c24 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -74,7 +74,6 @@ struct spidev_data {
struct list_head device_entry;
/* TX/RX buffers are NULL unless this device is open (users > 0) */
- struct mutex buf_lock;
unsigned users;
u8 *tx_buffer;
u8 *rx_buffer;
@@ -102,24 +101,6 @@ spidev_sync_unlocked(struct spi_device *spi, struct spi_message *message)
return status;
}
-static ssize_t
-spidev_sync(struct spidev_data *spidev, struct spi_message *message)
-{
- ssize_t status;
- struct spi_device *spi;
-
- mutex_lock(&spidev->spi_lock);
- spi = spidev->spi;
-
- if (spi == NULL)
- status = -ESHUTDOWN;
- else
- status = spidev_sync_unlocked(spi, message);
-
- mutex_unlock(&spidev->spi_lock);
- return status;
-}
-
static inline ssize_t
spidev_sync_write(struct spidev_data *spidev, size_t len)
{
@@ -132,7 +113,8 @@ spidev_sync_write(struct spidev_data *spidev, size_t len)
spi_message_init(&m);
spi_message_add_tail(&t, &m);
- return spidev_sync(spidev, &m);
+
+ return spidev_sync_unlocked(spidev->spi, &m);
}
static inline ssize_t
@@ -147,7 +129,8 @@ spidev_sync_read(struct spidev_data *spidev, size_t len)
spi_message_init(&m);
spi_message_add_tail(&t, &m);
- return spidev_sync(spidev, &m);
+
+ return spidev_sync_unlocked(spidev->spi, &m);
}
/*-------------------------------------------------------------------------*/
@@ -157,7 +140,7 @@ static ssize_t
spidev_read(struct file *filp, char __user *buf, size_t count, loff_t *f_pos)
{
struct spidev_data *spidev;
- ssize_t status;
+ ssize_t status = -ESHUTDOWN;
/* chipselect only toggles at start or end of operation */
if (count > bufsiz)
@@ -165,7 +148,11 @@ spidev_read(struct file *filp, char __user *buf, size_t count, loff_t *f_pos)
spidev = filp->private_data;
- mutex_lock(&spidev->buf_lock);
+ mutex_lock(&spidev->spi_lock);
+
+ if (spidev->spi == NULL)
+ goto err_spi_removed;
+
status = spidev_sync_read(spidev, count);
if (status > 0) {
unsigned long missing;
@@ -176,7 +163,9 @@ spidev_read(struct file *filp, char __user *buf, size_t count, loff_t *f_pos)
else
status = status - missing;
}
- mutex_unlock(&spidev->buf_lock);
+
+err_spi_removed:
+ mutex_unlock(&spidev->spi_lock);
return status;
}
@@ -187,7 +176,7 @@ spidev_write(struct file *filp, const char __user *buf,
size_t count, loff_t *f_pos)
{
struct spidev_data *spidev;
- ssize_t status;
+ ssize_t status = -ESHUTDOWN;
unsigned long missing;
/* chipselect only toggles at start or end of operation */
@@ -196,13 +185,19 @@ spidev_write(struct file *filp, const char __user *buf,
spidev = filp->private_data;
- mutex_lock(&spidev->buf_lock);
+ mutex_lock(&spidev->spi_lock);
+
+ if (spidev->spi == NULL)
+ goto err_spi_removed;
+
missing = copy_from_user(spidev->tx_buffer, buf, count);
if (missing == 0)
status = spidev_sync_write(spidev, count);
else
status = -EFAULT;
- mutex_unlock(&spidev->buf_lock);
+
+err_spi_removed:
+ mutex_unlock(&spidev->spi_lock);
return status;
}
@@ -379,14 +374,6 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
ctlr = spi->controller;
- /* use the buffer lock here for triple duty:
- * - prevent I/O (from us) so calling spi_setup() is safe;
- * - prevent concurrent SPI_IOC_WR_* from morphing
- * data fields while SPI_IOC_RD_* reads them;
- * - SPI_IOC_MESSAGE needs the buffer locked "normally".
- */
- mutex_lock(&spidev->buf_lock);
-
switch (cmd) {
/* read requests */
case SPI_IOC_RD_MODE:
@@ -510,7 +497,6 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
break;
}
- mutex_unlock(&spidev->buf_lock);
spi_dev_put(spi);
mutex_unlock(&spidev->spi_lock);
return retval;
@@ -541,9 +527,6 @@ spidev_compat_ioc_message(struct file *filp, unsigned int cmd,
return -ESHUTDOWN;
}
- /* SPI_IOC_MESSAGE needs the buffer locked "normally" */
- mutex_lock(&spidev->buf_lock);
-
/* Check message and copy into scratch area */
ioc = spidev_get_ioc_message(cmd, u_ioc, &n_ioc);
if (IS_ERR(ioc)) {
@@ -564,7 +547,6 @@ spidev_compat_ioc_message(struct file *filp, unsigned int cmd,
kfree(ioc);
done:
- mutex_unlock(&spidev->buf_lock);
spi_dev_put(spi);
mutex_unlock(&spidev->spi_lock);
return retval;
@@ -802,7 +784,6 @@ static int spidev_probe(struct spi_device *spi)
/* Initialize the driver data */
spidev->spi = spi;
mutex_init(&spidev->spi_lock);
- mutex_init(&spidev->buf_lock);
INIT_LIST_HEAD(&spidev->device_entry);
--
2.47.3
Powered by blists - more mailing lists