Memory leak in rtw88-pci

Memory leak in rtw88-pci

[Larry Finger]

Kmemleak shows the following leaks:

unreferenced object 0xffff888114146a00 (size 512):
   comm "softirq", pid 0, jiffies 4294910753 (age 28.196s)
   hex dump (first 32 bytes):
     08 42 00 00 01 00 5e 00 08 42 00 00 01 00 5e 00  .B....^..B....^.
     00 fb 84 1b 5e f7 6b 02 00 e0 01 00 5e 00 00 fb  ....^.k.....^...
   backtrace:
     [<0000000068bda00b>] kmalloc_reserve+0x2d/0x70
     [<000000006234ee4e>] __alloc_skb+0x8c/0x250
     [<00000000fd066823>] __netdev_alloc_skb+0x3f/0x150
     [<000000002b8b6774>] rtw_pci_rx_napi.constprop.0+0x1c7/0x310 [rtw88_pci]
     [<0000000071d79fc5>] rtw_pci_napi_poll+0x47/0xf0 [rtw88_pci]
     [<000000005b3960c0>] __napi_poll+0x2a/0x160
     [<00000000f87d43ad>] net_rx_action+0x234/0x280
     [<0000000065ab9dcb>] __do_softirq+0xbf/0x285
     [<000000002a7f930b>] do_softirq+0x61/0x80
     [<0000000020308f21>] __local_bh_enable_ip+0x4b/0x50
     [<00000000c4d6ca98>] rtw_pci_interrupt_threadfn+0xb2/0x1f0 [rtw88_pci]
     [<0000000045d500ae>] irq_thread_fn+0x20/0x60
     [<00000000d00af633>] irq_thread+0xa0/0x150
     [<000000007c7898b7>] kthread+0x134/0x150
     [<0000000083df94f0>] ret_from_fork+0x22/0x30

That address in rtw_pci_rx_napi points to the dev_alloc_skb() call in the 
following snippit:

                 /* allocate a new skb for this frame,
                  * discard the frame if none available
                  */
                 new_len = pkt_stat.pkt_len + pkt_offset;
=====>          new = dev_alloc_skb(new_len);
                 if (WARN_ONCE(!new, "rx routine starvation\n"))
                         goto next_rp;

                 /* put the DMA data including rx_desc from phy to new skb */
                 skb_put_data(new, skb->data, new_len);

                 if (pkt_stat.is_c2h) {
                         rtw_fw_c2h_cmd_rx_irqsafe(rtwdev, pkt_offset, new);
                 } else {
                         /* remove rx_desc */
                         skb_pull(new, pkt_offset);

                         rtw_rx_stats(rtwdev, pkt_stat.vif, new);
                         memcpy(new->cb, &rx_status, sizeof(rx_status));
                         ieee80211_rx_napi(rtwdev->hw, NULL, new, napi);
                         rx_done++;
                 }

Clearly, the allocated skb is never freed. These allocated blocks do not 
disappear when the driver is unloaded, thus these reports are not false 
positives, but are real memory leaks.

I followed the code in rtw_fw_c2h_cmd_rx_irqsafe() and determined that it is 
freeing the skb, thus the problem is in the branch that calls 
ieee80211_rx_napi(); however, as far as I can tell, this code matches other drivers.

Larry

Re: Memory leak in rtw88-pci

[Klaus Müller]

Hello all!

May I kindly bring up this reported problem again? Is there anybody working on 
this problem? Or did I miss the already existing fix?


Thanks
Klaus


On 26.03.21 at 17:30 Larry Finger wrote:
> Kmemleak shows the following leaks:
> 
> unreferenced object 0xffff888114146a00 (size 512):
>    comm "softirq", pid 0, jiffies 4294910753 (age 28.196s)
>    hex dump (first 32 bytes):
>      08 42 00 00 01 00 5e 00 08 42 00 00 01 00 5e 00  .B....^..B....^.
>      00 fb 84 1b 5e f7 6b 02 00 e0 01 00 5e 00 00 fb  ....^.k.....^...
>    backtrace:
>      [<0000000068bda00b>] kmalloc_reserve+0x2d/0x70
>      [<000000006234ee4e>] __alloc_skb+0x8c/0x250
>      [<00000000fd066823>] __netdev_alloc_skb+0x3f/0x150
>      [<000000002b8b6774>] rtw_pci_rx_napi.constprop.0+0x1c7/0x310 [rtw88_pci]
>      [<0000000071d79fc5>] rtw_pci_napi_poll+0x47/0xf0 [rtw88_pci]
>      [<000000005b3960c0>] __napi_poll+0x2a/0x160
>      [<00000000f87d43ad>] net_rx_action+0x234/0x280
>      [<0000000065ab9dcb>] __do_softirq+0xbf/0x285
>      [<000000002a7f930b>] do_softirq+0x61/0x80
>      [<0000000020308f21>] __local_bh_enable_ip+0x4b/0x50
>      [<00000000c4d6ca98>] rtw_pci_interrupt_threadfn+0xb2/0x1f0 [rtw88_pci]
>      [<0000000045d500ae>] irq_thread_fn+0x20/0x60
>      [<00000000d00af633>] irq_thread+0xa0/0x150
>      [<000000007c7898b7>] kthread+0x134/0x150
>      [<0000000083df94f0>] ret_from_fork+0x22/0x30
> 
> That address in rtw_pci_rx_napi points to the dev_alloc_skb() call in the 
> following snippit:
> 
>                  /* allocate a new skb for this frame,
>                   * discard the frame if none available
>                   */
>                  new_len = pkt_stat.pkt_len + pkt_offset;
> =====>          new = dev_alloc_skb(new_len);
>                  if (WARN_ONCE(!new, "rx routine starvation\n"))
>                          goto next_rp;
> 
>                  /* put the DMA data including rx_desc from phy to new skb */
>                  skb_put_data(new, skb->data, new_len);
> 
>                  if (pkt_stat.is_c2h) {
>                          rtw_fw_c2h_cmd_rx_irqsafe(rtwdev, pkt_offset, new);
>                  } else {
>                          /* remove rx_desc */
>                          skb_pull(new, pkt_offset);
> 
>                          rtw_rx_stats(rtwdev, pkt_stat.vif, new);
>                          memcpy(new->cb, &rx_status, sizeof(rx_status));
>                          ieee80211_rx_napi(rtwdev->hw, NULL, new, napi);
>                          rx_done++;
>                  }
> 
> Clearly, the allocated skb is never freed. These allocated blocks do not disappear 
> when the driver is unloaded, thus these reports are not false positives, but are 
> real memory leaks.
> 
> I followed the code in rtw_fw_c2h_cmd_rx_irqsafe() and determined that it is 
> freeing the skb, thus the problem is in the branch that calls ieee80211_rx_napi(); 
> however, as far as I can tell, this code matches other drivers.
> 
> Larry
> 

Re: Memory leak in rtw88-pci

[Larry Finger]

On 4/8/21 11:12 PM, Klaus Müller wrote:
> 
> May I kindly bring up this reported problem again? Is there anybody working on 
> this problem? Or did I miss the already existing fix?
> 

The Realtek developer suggested an approach that failed. I  have been busy with 
some other high-priority issues, but I will be working on this issue soon.

Larry

Re: Memory leak in rtw88-pci

[Larry Finger]

On 4/8/21 11:12 PM, Klaus Müller wrote:
> May I kindly bring up this reported problem again? Is there anybody working on 
> this problem? Or did I miss the already existing fix?

A fix has been found. The patched code is available at 
https://GitHub.com/lwfinger/rtw88.git. Patches are being prepared for 
wireless-next. From there, they will propagate into the Linux distributions.

Thanks for your patience,
Larry

Re: Memory leak in rtw88-pci

[Klaus Müller]

On 11.04.21 at 21:35 Larry Finger wrote:
> On 4/8/21 11:12 PM, Klaus Müller wrote:
>> May I kindly bring up this reported problem again? Is there anybody working on this problem? Or did I miss the already existing fix?
> 
> A fix has been found. The patched code is available at https://GitHub.com/lwfinger/rtw88.git. Patches are being prepared for wireless-next. From there, they will propagate into the Linux distributions.

Thanks Larry for doing QA and fixing the problem! I'm additionally very thankful for your rtw88 git repository, which provides the possibility to use actual drivers independent of the kernel version.
The actual version works fine for me.


Kind regards
Klaus

Re: Memory leak in rtw88-pci

[Brian Norris]

On Sun, Apr 11, 2021 at 12:35 PM Larry Finger <Larry.Finger@lwfinger.net> wrote:
>
> On 4/8/21 11:12 PM, Klaus Müller wrote:
> > May I kindly bring up this reported problem again? Is there anybody working on
> > this problem? Or did I miss the already existing fix?
>
> A fix has been found. The patched code is available at
> https://GitHub.com/lwfinger/rtw88.git. Patches are being prepared for
> wireless-next. From there, they will propagate into the Linux distributions.
Did you ever submit the second half of that patch?
https://github.com/lwfinger/rtw88/commit/0eed97166d54cf6fa03e20735b9c208375b8c949
(modifications to rtw_fw_c2h_cmd_rx_irqsafe())

We're still seeing leaks here with kmemleak, although I'm using a
5.4.y kernel, so maybe don't have all the latest fixes yet.

NB: I *do* have this: https://git.kernel.org/linus/191f6b08bfef
as it made it to 5.4.y.

Thanks,
Brian

Re: Memory leak in rtw88-pci

[Larry Finger]

On 6/17/21 5:54 PM, Brian Norris wrote:
> On Sun, Apr 11, 2021 at 12:35 PM Larry Finger <Larry.Finger@lwfinger.net> wrote:
>>
>> On 4/8/21 11:12 PM, Klaus Müller wrote:
>>> May I kindly bring up this reported problem again? Is there anybody working on
>>> this problem? Or did I miss the already existing fix?
>>
>> A fix has been found. The patched code is available at
>> https://GitHub.com/lwfinger/rtw88.git. Patches are being prepared for
>> wireless-next. From there, they will propagate into the Linux distributions.
> Did you ever submit the second half of that patch?
> https://github.com/lwfinger/rtw88/commit/0eed97166d54cf6fa03e20735b9c208375b8c949
> (modifications to rtw_fw_c2h_cmd_rx_irqsafe())
> 
> We're still seeing leaks here with kmemleak, although I'm using a
> 5.4.y kernel, so maybe don't have all the latest fixes yet.
> 
> NB: I *do* have this: https://git.kernel.org/linus/191f6b08bfef
> as it made it to 5.4.y.

Brian,

I fixed the leaks, but forgot to submit the patch. My repo at 
https://github.com/lafingerrtw88.git has the fixes, and does have any leaks on 
my system. The patch just pushed has the same fixes.

Larry

Re: Memory leak in rtw88-pci

[Brian Norris]

On Sun, Jun 20, 2021 at 12:33 PM Larry Finger <Larry.Finger@lwfinger.net> wrote:
> I fixed the leaks, but forgot to submit the patch. My repo at
> https://github.com/lafingerrtw88.git has the fixes, and does have any leaks on
> my system. The patch just pushed has the same fixes.

Thanks for sending! It sounds like Realtek folks also addressed some
similar issues and more there (I bumped them privately, pointing
here), so I believe we're in good shape now.

Thanks,
Brian