Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-11-28 23:21:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

spi: spidev: fix a race condition when accessing spidev->spi

Browse Source 
There's a spinlock in place that is taken in file_operations callbacks
whenever we check if spidev->spi is still alive (not null). It's also
taken when spidev->spi is set to NULL in remove().

This however doesn't protect the code against driver unbind event while
one of the syscalls is still in progress. To that end we need a lock taken
continuously as long as we may still access spidev->spi. As both the file
ops and the remove callback are never called from interrupt context, we
can replace the spinlock with a mutex.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Link: https://lore.kernel.org/r/20230106100719.196243-1-brgl@bgdev.pl
Signed-off-by: Mark Brown <broonie@kernel.org>
This commit is contained in:
Bartosz Golaszewski 2023-01-06 11:07:18 +01:00 committed by Mark Brown
parent 819cfea7d6
commit 1f4d2dd45b
No known key found for this signature in database
GPG Key ID: 24D68B725D5487D0
1 changed files with 18 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
drivers/spi/spidev.c
View File

@ -68,7 +68,7 @@ static_assert(N_SPI_MINORS > 0 && N_SPI_MINORS <= 256);
struct spidev_data { struct spidev_data {
dev_t devt; dev_t devt;
spinlock_t spi_lock; struct mutex spi_lock;
struct spi_device *spi; struct spi_device *spi;
struct list_head device_entry; struct list_head device_entry;
@ -95,9 +95,8 @@ spidev_sync(struct spidev_data *spidev, struct spi_message *message)
int status; int status;
struct spi_device *spi; struct spi_device *spi;
spin_lock_irq(&spidev->spi_lock); mutex_lock(&spidev->spi_lock);
spi = spidev->spi; spi = spidev->spi;
spin_unlock_irq(&spidev->spi_lock);
if (spi == NULL) if (spi == NULL)
status = -ESHUTDOWN; status = -ESHUTDOWN;
@ -107,6 +106,7 @@ spidev_sync(struct spidev_data *spidev, struct spi_message *message)
if (status == 0) if (status == 0)
status = message->actual_length; status = message->actual_length;
mutex_unlock(&spidev->spi_lock);
return status; return status;
} }
@ -359,12 +359,12 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
* we issue this ioctl. * we issue this ioctl.
*/ */
spidev = filp->private_data; spidev = filp->private_data;
spin_lock_irq(&spidev->spi_lock); mutex_lock(&spidev->spi_lock);
spi = spi_dev_get(spidev->spi); spi = spi_dev_get(spidev->spi);
spin_unlock_irq(&spidev->spi_lock); if (spi == NULL) {
mutex_unlock(&spidev->spi_lock);
if (spi == NULL)
return -ESHUTDOWN; return -ESHUTDOWN;
}
/* use the buffer lock here for triple duty: /* use the buffer lock here for triple duty:
* - prevent I/O (from us) so calling spi_setup() is safe; * - prevent I/O (from us) so calling spi_setup() is safe;
@ -508,6 +508,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
mutex_unlock(&spidev->buf_lock); mutex_unlock(&spidev->buf_lock);
spi_dev_put(spi); spi_dev_put(spi);
mutex_unlock(&spidev->spi_lock);
return retval; return retval;
} }
@ -529,12 +530,12 @@ spidev_compat_ioc_message(struct file *filp, unsigned int cmd,
* we issue this ioctl. * we issue this ioctl.
*/ */
spidev = filp->private_data; spidev = filp->private_data;
spin_lock_irq(&spidev->spi_lock); mutex_lock(&spidev->spi_lock);
spi = spi_dev_get(spidev->spi); spi = spi_dev_get(spidev->spi);
spin_unlock_irq(&spidev->spi_lock); if (spi == NULL) {
mutex_unlock(&spidev->spi_lock);
if (spi == NULL)
return -ESHUTDOWN; return -ESHUTDOWN;
}
/* SPI_IOC_MESSAGE needs the buffer locked "normally" */ /* SPI_IOC_MESSAGE needs the buffer locked "normally" */
mutex_lock(&spidev->buf_lock); mutex_lock(&spidev->buf_lock);
@ -561,6 +562,7 @@ spidev_compat_ioc_message(struct file *filp, unsigned int cmd,
done: done:
mutex_unlock(&spidev->buf_lock); mutex_unlock(&spidev->buf_lock);
spi_dev_put(spi); spi_dev_put(spi);
mutex_unlock(&spidev->spi_lock);
return retval; return retval;
} }
@ -640,10 +642,10 @@ static int spidev_release(struct inode *inode, struct file *filp)
spidev = filp->private_data; spidev = filp->private_data;
filp->private_data = NULL; filp->private_data = NULL;
spin_lock_irq(&spidev->spi_lock); mutex_lock(&spidev->spi_lock);
/* ... after we unbound from the underlying device? */ /* ... after we unbound from the underlying device? */
dofree = (spidev->spi == NULL); dofree = (spidev->spi == NULL);
spin_unlock_irq(&spidev->spi_lock); mutex_unlock(&spidev->spi_lock);
/* last close? */ /* last close? */
spidev->users--; spidev->users--;
@ -776,7 +778,7 @@ static int spidev_probe(struct spi_device *spi)
/* Initialize the driver data */ /* Initialize the driver data */
spidev->spi = spi; spidev->spi = spi;
spin_lock_init(&spidev->spi_lock); mutex_init(&spidev->spi_lock);
mutex_init(&spidev->buf_lock); mutex_init(&spidev->buf_lock);
INIT_LIST_HEAD(&spidev->device_entry); INIT_LIST_HEAD(&spidev->device_entry);
@ -821,9 +823,9 @@ static void spidev_remove(struct spi_device *spi)
/* prevent new opens */ /* prevent new opens */
mutex_lock(&device_list_lock); mutex_lock(&device_list_lock);
/* make sure ops on existing fds can abort cleanly */ /* make sure ops on existing fds can abort cleanly */
spin_lock_irq(&spidev->spi_lock); mutex_lock(&spidev->spi_lock);
spidev->spi = NULL; spidev->spi = NULL;
spin_unlock_irq(&spidev->spi_lock); mutex_unlock(&spidev->spi_lock);
list_del(&spidev->device_entry); list_del(&spidev->device_entry);
device_destroy(spidev_class, spidev->devt); device_destroy(spidev_class, spidev->devt);