* [PATCH 1/2] scsi: mpi3mr: Fix locking in an error path
2025-02-10 20:39 [PATCH 0/2] Fix locking bugs in error paths Bart Van Assche
@ 2025-02-10 20:39 ` Bart Van Assche
2025-02-10 20:39 ` [PATCH 2/2] scsi: mpt3sas: Fix a locking bug " Bart Van Assche
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2025-02-10 20:39 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, Bart Van Assche, Sathya Prakash, Kashyap Desai,
Sumit Saxena, Sreekanth Reddy, James E.J. Bottomley,
Nathan Chancellor, Chandrakanth patil
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;
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] scsi: mpt3sas: Fix a locking bug in an error path
2025-02-10 20:39 [PATCH 0/2] Fix locking bugs in error paths Bart Van Assche
2025-02-10 20:39 ` [PATCH 1/2] scsi: mpi3mr: Fix locking in an error path Bart Van Assche
@ 2025-02-10 20:39 ` Bart Van Assche
2025-02-19 2:36 ` [PATCH 0/2] Fix locking bugs in error paths Martin K. Petersen
2025-02-25 0:32 ` Martin K. Petersen
3 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2025-02-10 20:39 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, Bart Van Assche, Ranjan Kumar, Sathya Prakash,
Sreekanth Reddy, Suganath Prabu Subramani, James E.J. Bottomley,
Nathan Chancellor
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] Fix locking bugs in error paths
2025-02-10 20:39 [PATCH 0/2] Fix locking bugs in error paths Bart Van Assche
2025-02-10 20:39 ` [PATCH 1/2] scsi: mpi3mr: Fix locking in an error path Bart Van Assche
2025-02-10 20:39 ` [PATCH 2/2] scsi: mpt3sas: Fix a locking bug " Bart Van Assche
@ 2025-02-19 2:36 ` Martin K. Petersen
2025-02-25 0:32 ` Martin K. Petersen
3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2025-02-19 2:36 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] Fix locking bugs in error paths
2025-02-10 20:39 [PATCH 0/2] Fix locking bugs in error paths Bart Van Assche
` (2 preceding siblings ...)
2025-02-19 2:36 ` [PATCH 0/2] Fix locking bugs in error paths Martin K. Petersen
@ 2025-02-25 0:32 ` Martin K. Petersen
3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2025-02-25 0:32 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi
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
^ permalink raw reply [flat|nested] 5+ messages in thread