Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: rtlwifi: Memory leak in rtl92c_set_fw_rsvdpagepkt()
To:     Pkshih <pkshih@realtek.com>,
        Larry Finger <Larry.Finger@lwfinger.net>
Cc:     "colin.king@canonical.com" <colin.king@canonical.com>,
        "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
References: <989debc9-8602-0ce3-71a7-2bf783b2c22b@gmx.net>
 <5B2DA6FDDF928F4E855344EE0A5C39D1D5C90CAD@RTITMBSVM04.realtek.com.tw>
From:   Stefan Wahren <wahrenst@gmx.net>
Message-ID: <72e5ee88-4859-8134-825a-eb103926d2f0@gmx.net>
Date:   Sat, 2 Nov 2019 01:54:26 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <5B2DA6FDDF928F4E855344EE0A5C39D1D5C90CAD@RTITMBSVM04.realtek.com.tw>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk


Am 01.11.19 um 02:18 schrieb Pkshih:
>
>> -----Original Message-----
>> From: Stefan Wahren [mailto:wahrenst@gmx.net]
>> Sent: Thursday, October 31, 2019 9:57 PM
>> To: Pkshih; Larry Finger
>> Cc: colin.king@canonical.com; kvalo@codeaurora.org; linux-wireless@vger=
.kernel.org;
>> netdev@vger.kernel.org
>> Subject: rtlwifi: Memory leak in rtl92c_set_fw_rsvdpagepkt()
>>
>> Hi,
>>
>> i tested the EDIMAX EW-7612 on Raspberry Pi 3B+ with Linux 5.4-rc5
>> (multi_v7_defconfig + rtlwifi + kmemleak) and noticed a single memory
>> leak during probe:
>>
>> unreferenced object 0xec13ee40 (size 176):
>> =C2=A0 comm "kworker/u8:1", pid 36, jiffies 4294939321 (age 5580.790s)
>> =C2=A0 hex dump (first 32 bytes):
>> =C2=A0=C2=A0=C2=A0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00=C2=
=A0 ................
>> =C2=A0=C2=A0=C2=A0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00=C2=
=A0 ................
>> =C2=A0 backtrace:
>> =C2=A0=C2=A0=C2=A0 [<fc1bbb3e>] __netdev_alloc_skb+0x9c/0x164
>> =C2=A0=C2=A0=C2=A0 [<863dfa6e>] rtl92c_set_fw_rsvdpagepkt+0x254/0x340 [=
rtl8192c_common]
>> =C2=A0=C2=A0=C2=A0 [<9572be0d>] rtl92cu_set_hw_reg+0xf48/0xfa4 [rtl8192=
cu]
>> =C2=A0=C2=A0=C2=A0 [<116df4d8>] rtl_op_bss_info_changed+0x234/0x96c [rt=
lwifi]
>> =C2=A0=C2=A0=C2=A0 [<8933575f>] ieee80211_bss_info_change_notify+0xb8/0=
x264 [mac80211]
>> =C2=A0=C2=A0=C2=A0 [<d4061e86>] ieee80211_assoc_success+0x934/0x1798 [m=
ac80211]
>> =C2=A0=C2=A0=C2=A0 [<e55adb56>] ieee80211_rx_mgmt_assoc_resp+0x174/0x31=
4 [mac80211]
>> =C2=A0=C2=A0=C2=A0 [<5974629e>] ieee80211_sta_rx_queued_mgmt+0x3f4/0x7f=
0 [mac80211]
>> =C2=A0=C2=A0=C2=A0 [<d91091c6>] ieee80211_iface_work+0x208/0x318 [mac80=
211]
>> =C2=A0=C2=A0=C2=A0 [<ac5fcae4>] process_one_work+0x22c/0x564
>> =C2=A0=C2=A0=C2=A0 [<f5e6d3b6>] worker_thread+0x44/0x5d8
>> =C2=A0=C2=A0=C2=A0 [<82c7b073>] kthread+0x150/0x154
>> =C2=A0=C2=A0=C2=A0 [<b43e1b7d>] ret_from_fork+0x14/0x2c
>> =C2=A0=C2=A0=C2=A0 [<794dff30>] 0x0
>>
>> It looks like the allocated skd is never freed.
>>
>> Would be nice to get this fixed.
>>
> Hi,
>
> This is due to 8192cu doesn't implement usb_cmd_send_packet(). Could you=
 help
> following patch?
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c b/drive=
rs/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
> index 56cc3bc30860..f070f25bb735 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
> @@ -1540,6 +1540,8 @@ static bool usb_cmd_send_packet(struct ieee80211_h=
w *hw, struct sk_buff *skb)
>     * This is maybe necessary:
>     * rtlpriv->cfg->ops->fill_tx_cmddesc(hw, buffer, 1, 1, skb);
>     */
> +       dev_kfree_skb(skb);
> +
>         return true;
>  }
>
> This patch just frees the skb to resolve memleak problem. Since 8192cu d=
oesn't
> turn on fwctrl_lps that needs to download command packet to firmware, ap=
ply
> this patch isn't worse than before.

Yes, the patch fixes the memleak.

Thanks
Stefan

>
> ---
> PK
>