[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48963728-67bd-ace8-68fd-f840a2e65873@quicinc.com>
Date: Wed, 7 Sep 2022 13:10:13 -0700
From: Jeff Johnson <quic_jjohnson@...cinc.com>
To: Jianglei Nie <niejianglei2021@....com>, <kvalo@...nel.org>,
<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>
CC: <ath11k@...ts.infradead.org>, <linux-wireless@...r.kernel.org>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ath11k: mhi: fix potential memory leak in
ath11k_mhi_register()
On 9/7/2022 12:37 AM, Jianglei Nie wrote:
> mhi_alloc_controller() allocates a memory space for mhi_ctrl. When gets
> some error, mhi_ctrl should be freed with mhi_free_controller(). 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.
>
> Signed-off-by: Jianglei Nie <niejianglei2021@....com>
I believe this should have been annotated as -v2 to the following:
<https://lore.kernel.org/ath11k/20220526100227.483609-1-niejianglei2021@163.com/>
Please properly annotate follow-up patches.
Also please add wifi: to the beginning of the subject prefix
> ---
> 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 c44df17719f6..86995e8dc913 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -402,8 +402,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_FLAG_MULTI_MSI_VECTORS, &ab->dev_flags))
> @@ -412,7 +411,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;
> @@ -440,18 +439,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)
Reviewed-by: Jeff Johnson <quic_jjohnson@...cinc.com>
Powered by blists - more mailing lists