From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C8B2A3321D8 for ; Sat, 28 Feb 2026 17:53:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772301209; cv=none; b=FyJF3E6R0AlLXIwtWFmWtA+HIbtfEyHsmVNv4b3s0jWqNgDmRnBP9lkc2uYZhoycT6PykGGJGoVmI5B/yq4r7p9Bn64kRqf2+EuoHPc/QEGYDYZ8SbnR42KJsNAxiqtwvQICxFBwxkKoWpU2TqZitkuWIhNonYLe+npzUsl+fhw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772301209; c=relaxed/simple; bh=Jsd/d3X05s05PdN6prCCkUlMUtLpBB+KbRiwHJ4mMIo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=W+Cc5fkRE7eM5LxmwRODwlVzaaSfv6SLv0QAPZsiA1ERWi/idHwOFjABoVKfzjIVZSUHLume2ZpFlx26wCQbwGlvU6zyxk9tjbEEyqF1dc62HoYvUIBrX4Am24ts88xsJKK0Hl1whtPoqqbx4Ilc72GoWg+uUWYB0O9zqtboMYs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=n55PsGnm; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="n55PsGnm" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 37933C116D0; Sat, 28 Feb 2026 17:53:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1772301209; bh=Jsd/d3X05s05PdN6prCCkUlMUtLpBB+KbRiwHJ4mMIo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=n55PsGnmGoaJ1uIv90BxL1oCZl424mkEG0TS7yO1Bqob7PoCE0iAnW8ypH7bsYRv5 Mybag9Eql/6J0+0fBWrajf0/6rQsZhOh3SPyk6wBT9TRJs6ShvfPOW9NayMOYBtAUi yboNuMpbuYdUYMHaC2mWmrkNlNuJb/t9iPVp0dbxADjdiTqCVxG/E9uwnrI4vnjefn dxw4ZEBVcsoaDDJ9V0t3qutj9a0cErzEP66KnSB4yl0gAD97/GbrNH71kv+9gPcdGH vKeWIU7FVcY4ZVuI0fdqQCfNx8WNVxzttxs5oXnHPhdSilWGiQDmdgoJCB4wCg4Klr xpG94+wgCfucQ== From: Sasha Levin To: patches@lists.linux.dev Cc: Fabian Godehardt , Mark Brown , Sasha Levin Subject: [PATCH 6.18 384/752] spi: spidev: fix lock inversion between spi_lock and buf_lock Date: Sat, 28 Feb 2026 12:41:35 -0500 Message-ID: <20260228174750.1542406-384-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260228174750.1542406-1-sashal@kernel.org> References: <20260228174750.1542406-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit From: Fabian Godehardt [ Upstream commit 40534d19ed2afb880ecf202dab26a8e7a5808d16 ] 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 Link: https://patch.msgid.link/20260211072616.489522-1-fg@emlix.com Signed-off-by: Mark Brown Signed-off-by: Sasha Levin --- 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 5300c942a2a44..4d53c394101a4 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; @@ -800,7 +782,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.51.0