[PATCH 0/2] Fix locking bugs in error paths

[PATCH 0/2] Fix locking bugs in error paths

[Bart Van Assche]

Hi Martin,

Annotating struct mutex and the mutex_*() functions with Clang thread-safety
attributes led to the discovery of two locking bugs in error paths of SCSI
drivers. This patch series fixes these locking bugs. Please consider this
patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (2):
  scsi: mpi3mr: Fix locking in an error path
  scsi: mpt3sas: Fix a locking bug in an error path

 drivers/scsi/mpi3mr/mpi3mr_app.c    |  1 +
 drivers/scsi/mpt3sas/mpt3sas_base.c | 12 +++++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

[PATCH 1/2] scsi: mpi3mr: Fix locking in an error path

[Bart Van Assche]

Make all error paths unlock rioc->bsg_cmds.mutex.

This patch fixes the following Clang -Wthread-safety errors:

drivers/scsi/mpi3mr/mpi3mr_app.c:2835:1: error: mutex 'mrioc->bsg_cmds.mutex' is not held on every path through here [-Werror,-Wthread-safety-analysis]
 2835 | }
      | ^
drivers/scsi/mpi3mr/mpi3mr_app.c:2332:6: note: mutex acquired here
 2332 |         if (mutex_lock_interruptible(&mrioc->bsg_cmds.mutex))
      |             ^
./include/linux/mutex.h:172:40: note: expanded from macro 'mutex_lock_interruptible'
  172 | #define mutex_lock_interruptible(lock) mutex_lock_interruptible_nested(lock, 0)
      |                                        ^

Cc: Sathya Prakash <sathya.prakash@broadcom.com>
Fixes: fb231d7deffb ("scsi: mpi3mr: Support for preallocation of SGL BSG data buffers part-2")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 26582776749f..c6bd5c8c0ea5 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -2425,6 +2425,7 @@ static long mpi3mr_bsg_process_mpt_cmds(struct bsg_job *job)
 	}
 
 	if (!mrioc->ioctl_sges_allocated) {
+		mutex_unlock(&mrioc->bsg_cmds.mutex);
 		dprint_bsg_err(mrioc, "%s: DMA memory was not allocated\n",
 			       __func__);
 		return -ENOMEM;

[PATCH 2/2] scsi: mpt3sas: Fix a locking bug in an error path

[Bart Van Assche]

Call mutex_unlock(&ioc->hostdiag_unlock_mutex) once from error paths
instead of twice.

This patch fixes the following Clang -Wthread-safety errors:

drivers/scsi/mpt3sas/mpt3sas_base.c:8085:2: error: mutex 'ioc->hostdiag_unlock_mutex' is not held on every path through here [-Werror,-Wthread-safety-analysis]
 8085 |         pci_cfg_access_unlock(ioc->pdev);
      |         ^
drivers/scsi/mpt3sas/mpt3sas_base.c:8019:2: note: mutex acquired here
 8019 |         mutex_lock(&ioc->hostdiag_unlock_mutex);
      |         ^
./include/linux/mutex.h:171:26: note: expanded from macro 'mutex_lock'
  171 | #define mutex_lock(lock) mutex_lock_nested(lock, 0)
      |                          ^
drivers/scsi/mpt3sas/mpt3sas_base.c:8085:2: error: mutex 'ioc->hostdiag_unlock_mutex' is not held on every path through here [-Werror,-Wthread-safety-analysis]
 8085 |         pci_cfg_access_unlock(ioc->pdev);
      |         ^
drivers/scsi/mpt3sas/mpt3sas_base.c:8019:2: note: mutex acquired here
 8019 |         mutex_lock(&ioc->hostdiag_unlock_mutex);
      |         ^
./include/linux/mutex.h:171:26: note: expanded from macro 'mutex_lock'
  171 | #define mutex_lock(lock) mutex_lock_nested(lock, 0)
      |                          ^
drivers/scsi/mpt3sas/mpt3sas_base.c:8087:2: error: releasing mutex 'ioc->hostdiag_unlock_mutex' that was not held [-Werror,-Wthread-safety-analysis]
 8087 |         mutex_unlock(&ioc->hostdiag_unlock_mutex);
      |         ^

Cc: Ranjan Kumar <ranjan.kumar@broadcom.com>
Fixes: c0767560b012 ("scsi: mpt3sas: Reload SBR without rebooting HBA")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index dc43cfa83088..212e3b86bb81 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -8018,7 +8018,7 @@ _base_diag_reset(struct MPT3SAS_ADAPTER *ioc)
 
 	mutex_lock(&ioc->hostdiag_unlock_mutex);
 	if (mpt3sas_base_unlock_and_get_host_diagnostic(ioc, &host_diagnostic))
-		goto out;
+		goto unlock;
 
 	hcb_size = ioc->base_readl(&ioc->chip->HCBSize);
 	drsprintk(ioc, ioc_info(ioc, "diag reset: issued\n"));
@@ -8038,7 +8038,7 @@ _base_diag_reset(struct MPT3SAS_ADAPTER *ioc)
 			ioc_info(ioc,
 			    "Invalid host diagnostic register value\n");
 			_base_dump_reg_set(ioc);
-			goto out;
+			goto unlock;
 		}
 		if (!(host_diagnostic & MPI2_DIAG_RESET_ADAPTER))
 			break;
@@ -8074,17 +8074,19 @@ _base_diag_reset(struct MPT3SAS_ADAPTER *ioc)
 		ioc_err(ioc, "%s: failed going to ready state (ioc_state=0x%x)\n",
 			__func__, ioc_state);
 		_base_dump_reg_set(ioc);
-		goto out;
+		goto fail;
 	}
 
 	pci_cfg_access_unlock(ioc->pdev);
 	ioc_info(ioc, "diag reset: SUCCESS\n");
 	return 0;
 
- out:
+unlock:
+	mutex_unlock(&ioc->hostdiag_unlock_mutex);
+
+fail:
 	pci_cfg_access_unlock(ioc->pdev);
 	ioc_err(ioc, "diag reset: FAILED\n");
-	mutex_unlock(&ioc->hostdiag_unlock_mutex);
 	return -EFAULT;
 }
 

Re: [PATCH 0/2] Fix locking bugs in error paths

[Martin K. Petersen]

Bart,

> Annotating struct mutex and the mutex_*() functions with Clang
> thread-safety attributes led to the discovery of two locking bugs in
> error paths of SCSI drivers. This patch series fixes these locking
> bugs. Please consider this patch series for the next merge window.

Applied to 6.15/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/2] Fix locking bugs in error paths

[Martin K. Petersen]

On Mon, 10 Feb 2025 12:39:34 -0800, Bart Van Assche wrote:

> Annotating struct mutex and the mutex_*() functions with Clang thread-safety
> attributes led to the discovery of two locking bugs in error paths of SCSI
> drivers. This patch series fixes these locking bugs. Please consider this
> patch series for the next merge window.
> 
> Thanks,
> 
> [...]

Applied to 6.15/scsi-queue, thanks!

[1/2] scsi: mpi3mr: Fix locking in an error path
      https://git.kernel.org/mkp/scsi/c/c733741ae1c3
[2/2] scsi: mpt3sas: Fix a locking bug in an error path
      https://git.kernel.org/mkp/scsi/c/38afcf0660f5

-- 
Martin K. Petersen	Oracle Linux Engineering