| Message ID | 20250206221317.3845663-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jeff Johnson |
| Headers | show |
| Series | wifi: ath12k: Fix locking in error paths | expand |
On 2/7/25 03:43, Bart Van Assche wrote: > If ag->mutex has been locked, unlock it before returning. If it has not > been locked, do not unlock it before returning. These bugs have been > detected by the Clang thread-safety analyzer. > > Cc: Karthikeyan Periyasamy<quic_periyasa@quicinc.com> > Cc: Jeff Johnson<jeff.johnson@oss.qualcomm.com> > Fixes: ee146e11b4d9 ("wifi: ath12k: refactor core start based on hardware group") > Signed-off-by: Bart Van Assche<bvanassche@acm.org> > --- Reviewed-by: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
On 2/6/2025 2:13 PM, Bart Van Assche wrote: > If ag->mutex has been locked, unlock it before returning. If it has not > been locked, do not unlock it before returning. These bugs have been > detected by the Clang thread-safety analyzer. > > Cc: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> > Cc: Jeff Johnson <jeff.johnson@oss.qualcomm.com> > Fixes: ee146e11b4d9 ("wifi: ath12k: refactor core start based on hardware group") > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/net/wireless/ath/ath12k/core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c > index 0606116d6b9c..212cd935e60a 100644 > --- a/drivers/net/wireless/ath/ath12k/core.c > +++ b/drivers/net/wireless/ath/ath12k/core.c > @@ -1122,16 +1122,18 @@ int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab) > ath12k_core_stop(ab); > mutex_unlock(&ab->core_lock); > } > + mutex_unlock(&ag->mutex); > goto exit; > > err_dp_free: > ath12k_dp_free(ab); > mutex_unlock(&ab->core_lock); > + mutex_unlock(&ag->mutex); > + > err_firmware_stop: > ath12k_qmi_firmware_stop(ab); > > exit: > - mutex_unlock(&ag->mutex); > return ret; > } > I made the subject a bit more descriptive in the pending branch: https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pending&id=09a2421b6e5f617cfbeab906fa8037dda4aa95b8 /jeff
On 2/7/25 20:40, Jeff Johnson wrote: > On 2/6/2025 2:13 PM, Bart Van Assche wrote: >> If ag->mutex has been locked, unlock it before returning. If it has not >> been locked, do not unlock it before returning. These bugs have been >> detected by the Clang thread-safety analyzer. >> >> Cc: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> >> Cc: Jeff Johnson <jeff.johnson@oss.qualcomm.com> >> Fixes: ee146e11b4d9 ("wifi: ath12k: refactor core start based on hardware group") >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> drivers/net/wireless/ath/ath12k/core.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c >> index 0606116d6b9c..212cd935e60a 100644 >> --- a/drivers/net/wireless/ath/ath12k/core.c >> +++ b/drivers/net/wireless/ath/ath12k/core.c >> @@ -1122,16 +1122,18 @@ int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab) >> ath12k_core_stop(ab); >> mutex_unlock(&ab->core_lock); >> } >> + mutex_unlock(&ag->mutex); >> goto exit; >> >> err_dp_free: >> ath12k_dp_free(ab); >> mutex_unlock(&ab->core_lock); >> + mutex_unlock(&ag->mutex); >> + >> err_firmware_stop: >> ath12k_qmi_firmware_stop(ab); >> >> exit: >> - mutex_unlock(&ag->mutex); >> return ret; >> } >> > > I made the subject a bit more descriptive in the pending branch: > https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pending&id=09a2421b6e5f617cfbeab906fa8037dda4aa95b8 Better now, thanks.
On Thu, 06 Feb 2025 14:13:17 -0800, Bart Van Assche wrote: > If ag->mutex has been locked, unlock it before returning. If it has not > been locked, do not unlock it before returning. These bugs have been > detected by the Clang thread-safety analyzer. > > Applied, thanks! [1/1] wifi: ath12k: Fix locking in error paths commit: b9c7299a3341a737622e4de45b9c27e60ad01e3b Best regards,
diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c index 0606116d6b9c..212cd935e60a 100644 --- a/drivers/net/wireless/ath/ath12k/core.c +++ b/drivers/net/wireless/ath/ath12k/core.c @@ -1122,16 +1122,18 @@ int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab) ath12k_core_stop(ab); mutex_unlock(&ab->core_lock); } + mutex_unlock(&ag->mutex); goto exit; err_dp_free: ath12k_dp_free(ab); mutex_unlock(&ab->core_lock); + mutex_unlock(&ag->mutex); + err_firmware_stop: ath12k_qmi_firmware_stop(ab); exit: - mutex_unlock(&ag->mutex); return ret; }
If ag->mutex has been locked, unlock it before returning. If it has not been locked, do not unlock it before returning. These bugs have been detected by the Clang thread-safety analyzer. Cc: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> Cc: Jeff Johnson <jeff.johnson@oss.qualcomm.com> Fixes: ee146e11b4d9 ("wifi: ath12k: refactor core start based on hardware group") Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/net/wireless/ath/ath12k/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)