2018-10-22 08:23:19

by Johannes Stezenbach

[permalink] [raw]
Subject: 4.18.16: memory leak in sta_set_sinfo

Hi,

I used 4.18.11 for a while and noticed a memleak showing
in slabtop as kmalloc-2048. Then I updated to 4.18.16
and enabled kmemleak, it dumps this:

unreferenced object 0xffffa10a5cc49800 (size 2048):
comm "conky", pid 2734, jiffies 4295635396 (age 454.650s)
hex dump (first 32 bytes):
1e 00 00 00 00 00 00 00 ad 13 00 00 00 00 00 00 ................
a1 0d 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000b7b13d1a>] sta_set_sinfo+0x634/0x900 [mac80211]
[<00000000a74db622>] ieee80211_get_station+0x50/0x70 [mac80211]
[<0000000039cfbb15>] cfg80211_wireless_stats+0x103/0x350 [cfg80211]
[<000000003e9c0192>] iw_handler_get_iwstats+0x3b/0x90
[<0000000058160332>] ioctl_standard_iw_point+0xef/0x370
[<000000008af7fc78>] ioctl_standard_call+0x9d/0xb0
[<000000009ad7a410>] wext_handle_ioctl+0x99/0xf0
[<000000006bc483dc>] sock_ioctl+0x1b5/0x330
[<000000004a40c83b>] do_vfs_ioctl+0xa4/0x620
[<00000000c498918f>] ksys_ioctl+0x3a/0x70
[<00000000033ccded>] __x64_sys_ioctl+0x16/0x20
[<0000000038d98ffe>] do_syscall_64+0x56/0x110
[<00000000913ebc09>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[<00000000b4781b29>] 0xffffffffffffffff
unreferenced object 0xffffa10a5cc4c000 (size 2048):
comm "conky", pid 2734, jiffies 4295635396 (age 454.650s)
hex dump (first 32 bytes):
1e 00 00 00 00 00 00 00 ad 13 00 00 00 00 00 00 ................
a1 0d 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000b7b13d1a>] sta_set_sinfo+0x634/0x900 [mac80211]
[<00000000a74db622>] ieee80211_get_station+0x50/0x70 [mac80211]
[<000000009fd8a7aa>] cfg80211_wext_giwrate+0x111/0x2b0 [cfg80211]
[<0000000045f8e84f>] ioctl_standard_call+0x4d/0xb0
[<000000009ad7a410>] wext_handle_ioctl+0x99/0xf0
[<000000006bc483dc>] sock_ioctl+0x1b5/0x330
[<000000004a40c83b>] do_vfs_ioctl+0xa4/0x620
[<00000000c498918f>] ksys_ioctl+0x3a/0x70
[<00000000033ccded>] __x64_sys_ioctl+0x16/0x20
[<0000000038d98ffe>] do_syscall_64+0x56/0x110
[<00000000913ebc09>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[<00000000b4781b29>] 0xffffffffffffffff

etc.

sta_set_sinfo+0x634/0x900 seemt to be the call to
cfg80211_sinfo_alloc_tid_stats().


dmesg:

[ 2975.537043] kmemleak: 1223 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 3140.960503] kmemleak: 851 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 3755.462385] kmemleak: 280 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 4369.876913] kmemleak: 1011 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 4984.711904] kmemleak: 1002 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 5598.838136] kmemleak: 1023 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 6213.236272] kmemleak: 1088 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 6833.692160] kmemleak: 1004 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
[ 7447.151259] kmemleak: 1147 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

I guess it relates to
0fdf1493b41 " mac80211: allocate and fill tidstats only when needed"
which Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")


Johannes


2018-10-22 08:25:27

by Johannes Berg

[permalink] [raw]
Subject: Re: 4.18.16: memory leak in sta_set_sinfo

On Mon, 2018-10-22 at 10:02 +0200, Johannes Stezenbach wrote:
>
> [<00000000b7b13d1a>] sta_set_sinfo+0x634/0x900 [mac80211]
> [<00000000a74db622>] ieee80211_get_station+0x50/0x70 [mac80211]
> [<000000009fd8a7aa>] cfg80211_wext_giwrate+0x111/0x2b0 [cfg80211]

> I guess it relates to
> 0fdf1493b41 " mac80211: allocate and fill tidstats only when needed"
> which Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")

Yes, you're using wext which we don't any more, so we didn't see this.

However, it's already fixed:

commit 848e616e66d4592fe9afc40743d3504deb7632b4
Author: Stefan Seyfried <[email protected]>
Date: Sun Sep 30 12:53:00 2018 +0200

cfg80211: fix wext-compat memory leak

Hopefully that'll eventually propagate to stable.

johannes


2018-10-22 08:35:57

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: 4.18.16: memory leak in sta_set_sinfo

On Mon, Oct 22, 2018 at 10:25:07AM +0200, Johannes Berg wrote:
> On Mon, 2018-10-22 at 10:02 +0200, Johannes Stezenbach wrote:
> >
> > [<00000000b7b13d1a>] sta_set_sinfo+0x634/0x900 [mac80211]
> > [<00000000a74db622>] ieee80211_get_station+0x50/0x70 [mac80211]
> > [<000000009fd8a7aa>] cfg80211_wext_giwrate+0x111/0x2b0 [cfg80211]
>
> > I guess it relates to
> > 0fdf1493b41 " mac80211: allocate and fill tidstats only when needed"
> > which Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")
>
> Yes, you're using wext which we don't any more, so we didn't see this.

Yes, disappointing that conky still uses wext.

> However, it's already fixed:
>
> commit 848e616e66d4592fe9afc40743d3504deb7632b4
> Author: Stefan Seyfried <[email protected]>
> Date: Sun Sep 30 12:53:00 2018 +0200
>
> cfg80211: fix wext-compat memory leak
>
> Hopefully that'll eventually propagate to stable.

Good to know it's fixed, but "hopefully" makes me wonder,
I'd love to hear the confirmation that it's been queued.
OTOH I'll apply the patch locally and eventually move to 4.19,
so what gives...

Thanks,
Johannes

2018-10-22 08:37:55

by Johannes Berg

[permalink] [raw]
Subject: Re: 4.18.16: memory leak in sta_set_sinfo

On Mon, 2018-10-22 at 10:35 +0200, Johannes Stezenbach wrote:

> > commit 848e616e66d4592fe9afc40743d3504deb7632b4
> > Author: Stefan Seyfried <[email protected]>
> > Date: Sun Sep 30 12:53:00 2018 +0200
> >
> > cfg80211: fix wext-compat memory leak
> >
> > Hopefully that'll eventually propagate to stable.
>
> Good to know it's fixed, but "hopefully" makes me wonder,
> I'd love to hear the confirmation that it's been queued.

Well, normally it works automatically when you have a Fixes tag, so I
don't usually try to queue anything manually. Maybe Greg has had his
hands full with the 4.19 release :-)

johannes


2018-10-22 10:48:36

by Arend van Spriel

[permalink] [raw]
Subject: Re: 4.18.16: memory leak in sta_set_sinfo

On 10/22/2018 10:37 AM, Johannes Berg wrote:
> On Mon, 2018-10-22 at 10:35 +0200, Johannes Stezenbach wrote:
>
>>> commit 848e616e66d4592fe9afc40743d3504deb7632b4
>>> Author: Stefan Seyfried <[email protected]>
>>> Date: Sun Sep 30 12:53:00 2018 +0200
>>>
>>> cfg80211: fix wext-compat memory leak
>>>
>>> Hopefully that'll eventually propagate to stable.
>>
>> Good to know it's fixed, but "hopefully" makes me wonder,
>> I'd love to hear the confirmation that it's been queued.
>
> Well, normally it works automatically when you have a Fixes tag, so I
> don't usually try to queue anything manually. Maybe Greg has had his
> hands full with the 4.19 release :-)

Missed the memo. Does that mean Cc: to stable is no longer needed? It is
still documented as such in Documentation/process/stable-kernel-rules.rst.

Regards,
Arend



2018-10-23 11:59:58

by Kalle Valo

[permalink] [raw]
Subject: Re: 4.18.16: memory leak in sta_set_sinfo

Arend van Spriel <[email protected]> writes:

> On 10/22/2018 10:37 AM, Johannes Berg wrote:
>> On Mon, 2018-10-22 at 10:35 +0200, Johannes Stezenbach wrote:
>>
>>>> commit 848e616e66d4592fe9afc40743d3504deb7632b4
>>>> Author: Stefan Seyfried <[email protected]>
>>>> Date: Sun Sep 30 12:53:00 2018 +0200
>>>>
>>>> cfg80211: fix wext-compat memory leak
>>>>
>>>> Hopefully that'll eventually propagate to stable.
>>>
>>> Good to know it's fixed, but "hopefully" makes me wonder,
>>> I'd love to hear the confirmation that it's been queued.
>>
>> Well, normally it works automatically when you have a Fixes tag, so I
>> don't usually try to queue anything manually. Maybe Greg has had his
>> hands full with the 4.19 release :-)
>
> Missed the memo. Does that mean Cc: to stable is no longer needed? It
> is still documented as such in
> Documentation/process/stable-kernel-rules.rst.

I haven't seen any memos either but in lot of cases having just Fixes
line seems to be enough to get the patch to stable releases. If someone
has any documentation about this, please do share.

--
Kalle Valo