Re: [PATCH v2] ath11k: mhi: fix potential memory leak in ath11k_mhi_register()
From: Jeff Johnson <hidden>
Date: 2022-05-31 21:51:36
Also in:
ath11k, linux-wireless, lkml
On 5/30/2022 1:06 AM, Jianglei Nie wrote:
quoted hunk ↗ jump to hunk
mhi_alloc_controller() allocates a memory space for mhi_ctrl. When some errors occur, mhi_ctrl should be freed by mhi_free_controller() and set ab_pci->mhi_ctrl = NULL because ab_pci->mhi_ctrl has a dangling pointer to the freed memory. But when ath11k_mhi_read_addr_from_dt() fails, the function returns without calling mhi_free_controller(), which will lead to a memory leak. We can fix it by calling mhi_free_controller() when ath11k_mhi_read_addr_from_dt() fails and set ab_pci->mhi_ctrl = NULL in all of the places where we call mhi_free_controller(). Signed-off-by: Jianglei Nie <redacted> --- drivers/net/wireless/ath/ath11k/mhi.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c index fc3524e83e52..fc1bbf91c58e 100644 --- a/drivers/net/wireless/ath/ath11k/mhi.c +++ b/drivers/net/wireless/ath/ath11k/mhi.c@@ -367,8 +367,7 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci) ret = ath11k_mhi_get_msi(ab_pci); if (ret) { ath11k_err(ab, "failed to get msi for mhi\n"); - mhi_free_controller(mhi_ctrl); - return ret; + goto free_controller; } if (!test_bit(ATH11K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))@@ -377,7 +376,7 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci) if (test_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags)) { ret = ath11k_mhi_read_addr_from_dt(mhi_ctrl); if (ret < 0) - return ret; + goto free_controller; } else { mhi_ctrl->iova_start = 0; mhi_ctrl->iova_stop = 0xFFFFFFFF;@@ -405,18 +404,22 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci) default: ath11k_err(ab, "failed assign mhi_config for unknown hw rev %d\n", ab->hw_rev); - mhi_free_controller(mhi_ctrl); - return -EINVAL; + ret = -EINVAL; + goto free_controller; } ret = mhi_register_controller(mhi_ctrl, ath11k_mhi_config); if (ret) { ath11k_err(ab, "failed to register to mhi bus, err = %d\n", ret); - mhi_free_controller(mhi_ctrl); - return ret; + goto free_controller; } return 0; + +free_controller: + mhi_free_controller(mhi_ctrl); + ab_pci->mhi_ctrl = NULL; + return ret; } void ath11k_mhi_unregister(struct ath11k_pci *ab_pci)
LGTM, thanks Reviewed-by: Jeff Johnson <redacted>