Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Larry Finger <larry.finger@gmail.com>
Subject: Re: [PATCH v2] rtw88: Fix some memory leaks
To:     Pkshih <pkshih@realtek.com>,
        "kvalo@codeaurora.org" <kvalo@codeaurora.org>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        Stable <stable@vger.kernel.org>
References: <20210620194110.7520-1-Larry.Finger@lwfinger.net>
 <19c86cb8dbe04b56b76a386b5faeaa89@realtek.com>
From:   Larry Finger <Larry.Finger@lwfinger.net>
Message-ID: <2c552c7c-bb11-a914-78e8-900d6bae39a9@lwfinger.net>
Date:   Mon, 21 Jun 2021 10:52:05 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <19c86cb8dbe04b56b76a386b5faeaa89@realtek.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 6/21/21 5:40 AM, Pkshih wrote:
> 
> 
>> -----Original Message-----
>> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger
>> Sent: Monday, June 21, 2021 3:41 AM
>> To: kvalo@codeaurora.org
>> Cc: linux-wireless@vger.kernel.org; Larry Finger; Stable
>> Subject: [PATCH v2] rtw88: Fix some memory leaks
>>
>> There are memory leaks of skb's from routines rtw_fw_c2h_cmd_rx_irqsafe()
>> and rtw_coex_info_response(), both arising from C2H operations. There are
>> no leaks from the buffers sent to mac80211.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Stable <stable@vger.kernel.org>
>> ---
>> v2 - add the missinf changelog.
>>
>> ---
>>   drivers/net/wireless/realtek/rtw88/coex.c | 4 +++-
>>   drivers/net/wireless/realtek/rtw88/fw.c   | 2 ++
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c
>> index cedbf3825848..e81bf5070183 100644
>> --- a/drivers/net/wireless/realtek/rtw88/coex.c
>> +++ b/drivers/net/wireless/realtek/rtw88/coex.c
>> @@ -591,8 +591,10 @@ void rtw_coex_info_response(struct rtw_dev *rtwdev, struct sk_buff *skb)
>>   	struct rtw_coex *coex = &rtwdev->coex;
>>   	u8 *payload = get_payload_from_coex_resp(skb);
>>
>> -	if (payload[0] != COEX_RESP_ACK_BY_WL_FW)
>> +	if (payload[0] != COEX_RESP_ACK_BY_WL_FW) {
>> +		dev_kfree_skb_any(skb);
>>   		return;
>> +	}
>>
>>   	skb_queue_tail(&coex->queue, skb);
>>   	wake_up(&coex->wait);
>> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
>> index 797b08b2a494..43525ad8543f 100644
>> --- a/drivers/net/wireless/realtek/rtw88/fw.c
>> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
>> @@ -231,9 +231,11 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset,
>>   	switch (c2h->id) {
>>   	case C2H_BT_MP_INFO:
>>   		rtw_coex_info_response(rtwdev, skb);
>> +		dev_kfree_skb_any(skb);
> 
> The rtw_coex_info_response() puts skb into a skb_queue, so we can't free it here.
> Instead, we should free it after we dequeue and do thing.
> So, we send another patch:
> https://lore.kernel.org/linux-wireless/20210621103023.41928-1-pkshih@realtek.com/T/#u
> 
> I hope this isn't confusing you.

No, it is not confusing. T=You fixed some leaks the I did not know existed.

Kalle: Take P-K's patch and discard mine.

Thanks,

Larry