| Message ID | 20220825111818.30869-2-quic_mpubbise@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath11k: Enable low power mode when WLAN is not active | expand |
On 8/25/2022 4:18 AM, Manikanta Pubbisetty wrote: > Currently struct ath11k_hal::srng_config pointer is not assigned > to NULL after freeing the memory in ath11k_hal_srng_deinit(). > This could lead to double free issue in a scerario where nit: s/scerario/scenario/ > ath11k_hal_srng_deinit() is invoked back to back. > > In the current code, although the chances are very low, the above > said scenario could happen when hardware recovery has failed and > then there is another FW assert where ath11k_hal_srng_deinit() is > invoked once again as part of recovery. Addressing this issue is > important when low power mode support is enabled in the driver > (will be added by a future patch) where this scenario is likely. > > Fix this by assigning the struct ath11k_hal::srng_config pointer > to NULL after freeing the memory. > > Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 > > Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/hal.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c > index bda71ab5a1f2..ebdf3b1a6661 100644 > --- a/drivers/net/wireless/ath/ath11k/hal.c > +++ b/drivers/net/wireless/ath/ath11k/hal.c > @@ -1319,6 +1319,7 @@ void ath11k_hal_srng_deinit(struct ath11k_base *ab) > ath11k_hal_free_cont_rdp(ab); > ath11k_hal_free_cont_wrp(ab); > kfree(hal->srng_config); > + hal->srng_config = NULL; > } > EXPORT_SYMBOL(ath11k_hal_srng_deinit); >
On 8/25/2022 8:29 PM, Jeff Johnson wrote: > On 8/25/2022 4:18 AM, Manikanta Pubbisetty wrote: >> Currently struct ath11k_hal::srng_config pointer is not assigned >> to NULL after freeing the memory in ath11k_hal_srng_deinit(). >> This could lead to double free issue in a scerario where > > nit: s/scerario/scenario/ > Thanks Jeff, I'll correct this in next revision.
diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c index bda71ab5a1f2..ebdf3b1a6661 100644 --- a/drivers/net/wireless/ath/ath11k/hal.c +++ b/drivers/net/wireless/ath/ath11k/hal.c @@ -1319,6 +1319,7 @@ void ath11k_hal_srng_deinit(struct ath11k_base *ab) ath11k_hal_free_cont_rdp(ab); ath11k_hal_free_cont_wrp(ab); kfree(hal->srng_config); + hal->srng_config = NULL; } EXPORT_SYMBOL(ath11k_hal_srng_deinit);
Currently struct ath11k_hal::srng_config pointer is not assigned to NULL after freeing the memory in ath11k_hal_srng_deinit(). This could lead to double free issue in a scerario where ath11k_hal_srng_deinit() is invoked back to back. In the current code, although the chances are very low, the above said scenario could happen when hardware recovery has failed and then there is another FW assert where ath11k_hal_srng_deinit() is invoked once again as part of recovery. Addressing this issue is important when low power mode support is enabled in the driver (will be added by a future patch) where this scenario is likely. Fix this by assigning the struct ath11k_hal::srng_config pointer to NULL after freeing the memory. Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> --- drivers/net/wireless/ath/ath11k/hal.c | 1 + 1 file changed, 1 insertion(+)