diff mbox

spi: spidev: Hold spi_lock over all defererences of spi in release()

Message ID 1447689999-20632-1-git-send-email-broonie@kernel.org (mailing list archive)
State Accepted
Commit 56ea1075e7f07724cf9b91039aa0968a0c70112f
Headers show

Commit Message

Mark Brown Nov. 16, 2015, 4:06 p.m. UTC
We use the spi_lock spinlock to protect against races between the device
being removed and file operations on the spidev.  This means that in the
removal path all references to the device need to be done under lock as
in removal we dropping references to the device.

Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spidev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vegard Nossum Nov. 17, 2015, 8:11 a.m. UTC | #1
On 11/16/2015 05:06 PM, Mark Brown wrote:
> We use the spi_lock spinlock to protect against races between the device
> being removed and file operations on the spidev.  This means that in the
> removal path all references to the device need to be done under lock as
> in removal we dropping references to the device.
>
> Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   drivers/spi/spidev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index 91a0fcd72423..d0e7dfc647cf 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -651,11 +651,11 @@ static int spidev_release(struct inode *inode, struct file *filp)
>   		kfree(spidev->rx_buffer);
>   		spidev->rx_buffer = NULL;
>
> +		spin_lock_irq(&spidev->spi_lock);
>   		if (spidev->spi)
>   			spidev->speed_hz = spidev->spi->max_speed_hz;
>
>   		/* ... after we unbound from the underlying device? */
> -		spin_lock_irq(&spidev->spi_lock);
>   		dofree = (spidev->spi == NULL);
>   		spin_unlock_irq(&spidev->spi_lock);
>
>

Thanks for the patch! We should probably add a stable CC too (since it
fixes "spi: spidev: fix possible NULL dereference" which went into stable):

Cc: <stable@vger.kernel.org>


Vegard
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 91a0fcd72423..d0e7dfc647cf 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -651,11 +651,11 @@  static int spidev_release(struct inode *inode, struct file *filp)
 		kfree(spidev->rx_buffer);
 		spidev->rx_buffer = NULL;
 
+		spin_lock_irq(&spidev->spi_lock);
 		if (spidev->spi)
 			spidev->speed_hz = spidev->spi->max_speed_hz;
 
 		/* ... after we unbound from the underlying device? */
-		spin_lock_irq(&spidev->spi_lock);
 		dofree = (spidev->spi == NULL);
 		spin_unlock_irq(&spidev->spi_lock);