2022-01-11 07:49:38

by Paul Menzel

[permalink] [raw]
Subject: kmemleak detects leak in msr_build_context

Dear Linux folks,


Running Linux from commit bf4eebf8cfa2 (Merge tag
'linux-kselftest-kunit-5.17-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest),
kmemleak reported the leak below:

```
unreferenced object 0xffff8914823de500 (size 64):
comm "swapper/0", pid 1, jiffies 4294667581 (age 1253.406s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 04 10 01 c0 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<000000007f3b05e9>] __kmalloc+0x177/0x330
[<0000000008104cca>] msr_build_context.constprop.0+0x32/0xbe
[<00000000012bab4e>] msr_save_cpuid_features+0x28/0x2f
[<00000000b7a2262e>] pm_check_save_msr+0x2e/0x40
[<00000000cbe9d92d>] do_one_initcall+0x44/0x200
[<0000000094deab7b>] kernel_init_freeable+0x1fc/0x273
[<00000000d3dbaa56>] kernel_init+0x16/0x160
[<0000000058c4a8b3>] ret_from_fork+0x22/0x30
```


Kind regards,

Paul


2022-01-11 15:26:40

by Dave Hansen

[permalink] [raw]
Subject: Re: kmemleak detects leak in msr_build_context

On 1/10/22 23:49, Paul Menzel wrote:
> Running Linux from commit bf4eebf8cfa2 (Merge tag
> 'linux-kselftest-kunit-5.17-rc1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest),
> kmemleak reported the leak below:
>
> ```
> unreferenced object 0xffff8914823de500 (size 64):
>   comm "swapper/0", pid 1, jiffies 4294667581 (age 1253.406s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 04 10 01 c0 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<000000007f3b05e9>] __kmalloc+0x177/0x330
>     [<0000000008104cca>] msr_build_context.constprop.0+0x32/0xbe
>     [<00000000012bab4e>] msr_save_cpuid_features+0x28/0x2f
>     [<00000000b7a2262e>] pm_check_save_msr+0x2e/0x40
>     [<00000000cbe9d92d>] do_one_initcall+0x44/0x200
>     [<0000000094deab7b>] kernel_init_freeable+0x1fc/0x273
>     [<00000000d3dbaa56>] kernel_init+0x16/0x160
>     [<0000000058c4a8b3>] ret_from_fork+0x22/0x30

Thanks for the report.

I've taken a look through arch/x86/power/cpu.c, and nothing obvious
jumped out at me. msr_build_context() could probably be cleaned up by
using kvrealloc(), but it hasn't been touched recently in a way that I
would expect things to leak.

I suspect this is a false positive from kmemleak.

If you could share your full dmesg, it might help spot something. But,
I didn't see anything.

2022-01-11 21:05:51

by Paul Menzel

[permalink] [raw]
Subject: Re: kmemleak detects leak in msr_build_context

Dear Dave,


Thank you for your quick reply.

Am 11.01.22 um 16:26 schrieb Dave Hansen:
> On 1/10/22 23:49, Paul Menzel wrote:
>> Running Linux from commit bf4eebf8cfa2 (Merge tag
>> 'linux-kselftest-kunit-5.17-rc1' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest),
>> kmemleak reported the leak below:
>>
>> ```
>> unreferenced object 0xffff8914823de500 (size 64):
>>    comm "swapper/0", pid 1, jiffies 4294667581 (age 1253.406s)
>>    hex dump (first 32 bytes):
>>      00 00 00 00 00 00 00 00 04 10 01 c0 00 00 00 00  ................
>>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>    backtrace:
>>      [<000000007f3b05e9>] __kmalloc+0x177/0x330
>>      [<0000000008104cca>] msr_build_context.constprop.0+0x32/0xbe
>>      [<00000000012bab4e>] msr_save_cpuid_features+0x28/0x2f
>>      [<00000000b7a2262e>] pm_check_save_msr+0x2e/0x40
>>      [<00000000cbe9d92d>] do_one_initcall+0x44/0x200
>>      [<0000000094deab7b>] kernel_init_freeable+0x1fc/0x273
>>      [<00000000d3dbaa56>] kernel_init+0x16/0x160
>>      [<0000000058c4a8b3>] ret_from_fork+0x22/0x30
>
> Thanks for the report.

Thank you for your quick response.

> I've taken a look through arch/x86/power/cpu.c, and nothing obvious
> jumped out at me.  msr_build_context() could probably be cleaned up by
> using kvrealloc(), but it hasn't been touched recently in a way that I
> would expect things to leak.

Thank you for checking. I do not know if it’s a regression, as I didn’t
watch out for that error in the last months.

> I suspect this is a false positive from kmemleak.

Maybe Catalin can check.

> If you could share your full dmesg, it might help spot something.  But,
> I didn't see anything.

Please find it attached.

I am able to reproduce it doing `dpkg -i ../*deb` on the Linux packages
built with `make bindeb-pkg`.


Kind regards,

Paul


Attachments:
20220111-linux-5.16+-messages-kmemleak.txt (143.42 kB)

2022-01-11 23:16:23

by Borislav Petkov

[permalink] [raw]
Subject: Re: kmemleak detects leak in msr_build_context

On Tue, Jan 11, 2022 at 10:05:43PM +0100, Paul Menzel wrote:
> [ 0.672475] smpboot: CPU0: AMD A6-6400K APU with Radeon(tm) HD Graphics (family: 0x15, model: 0x13, stepping: 0x1)

I have a similar box to yours:

[ 0.382127] smpboot: CPU0: AMD PRO A12-8800B R7, 12 Compute Cores 4C+8G (family: 0x15, model: 0x60, stepping: 0x1)

...

[ 0.974044] x86/pm: family 0x15 cpu detected, MSR saving is needed during suspending.

well, Bulldozer-based at least, and booting with kmemleak enabled is fine:

$ cat /sys/kernel/debug/kmemleak
$

I wonder whether you have something else applied and forgotten along
with all those random printks I'm seeing in dmesg...

Also,

unreferenced object 0xffff8914823de500 (size 64):
^^^^^^^^

That's strange too - sizeof(struct saved_msr) is 40 and not 64 so that
looks weird...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-01-14 11:09:59

by Catalin Marinas

[permalink] [raw]
Subject: Re: kmemleak detects leak in msr_build_context

On Wed, Jan 12, 2022 at 12:16:17AM +0100, Borislav Petkov wrote:
> On Tue, Jan 11, 2022 at 10:05:43PM +0100, Paul Menzel wrote:
> > [ 0.672475] smpboot: CPU0: AMD A6-6400K APU with Radeon(tm) HD Graphics (family: 0x15, model: 0x13, stepping: 0x1)
>
> I have a similar box to yours:
>
> [ 0.382127] smpboot: CPU0: AMD PRO A12-8800B R7, 12 Compute Cores 4C+8G (family: 0x15, model: 0x60, stepping: 0x1)
>
> ...
>
> [ 0.974044] x86/pm: family 0x15 cpu detected, MSR saving is needed during suspending.
>
> well, Bulldozer-based at least, and booting with kmemleak enabled is fine:
>
> $ cat /sys/kernel/debug/kmemleak
> $
>
> I wonder whether you have something else applied and forgotten along
> with all those random printks I'm seeing in dmesg...
>
> Also,
>
> unreferenced object 0xffff8914823de500 (size 64):
> ^^^^^^^^
>
> That's strange too - sizeof(struct saved_msr) is 40 and not 64 so that
> looks weird...

That's probably because slab_post_alloc_hook() doesn't get the original
kmalloc() size, so it uses the slab object size when calling the
kmemleak hook.

--
Catalin

2022-01-14 11:44:44

by Catalin Marinas

[permalink] [raw]
Subject: Re: kmemleak detects leak in msr_build_context

On Tue, Jan 11, 2022 at 10:05:43PM +0100, Paul Menzel wrote:
> Am 11.01.22 um 16:26 schrieb Dave Hansen:
> > On 1/10/22 23:49, Paul Menzel wrote:
> > > Running Linux from commit bf4eebf8cfa2 (Merge tag
> > > 'linux-kselftest-kunit-5.17-rc1' of
> > > git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest),
> > > kmemleak reported the leak below:
> > >
> > > ```
> > > unreferenced object 0xffff8914823de500 (size 64):
> > > ?? comm "swapper/0", pid 1, jiffies 4294667581 (age 1253.406s)
> > > ?? hex dump (first 32 bytes):
> > > ???? 00 00 00 00 00 00 00 00 04 10 01 c0 00 00 00 00? ................
> > > ???? 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00? ................
> > > ?? backtrace:
> > > ???? [<000000007f3b05e9>] __kmalloc+0x177/0x330
> > > ???? [<0000000008104cca>] msr_build_context.constprop.0+0x32/0xbe
> > > ???? [<00000000012bab4e>] msr_save_cpuid_features+0x28/0x2f
> > > ???? [<00000000b7a2262e>] pm_check_save_msr+0x2e/0x40
> > > ???? [<00000000cbe9d92d>] do_one_initcall+0x44/0x200
> > > ???? [<0000000094deab7b>] kernel_init_freeable+0x1fc/0x273
> > > ???? [<00000000d3dbaa56>] kernel_init+0x16/0x160
> > > ???? [<0000000058c4a8b3>] ret_from_fork+0x22/0x30
[...]
> > I've taken a look through arch/x86/power/cpu.c, and nothing obvious
> > jumped out at me.? msr_build_context() could probably be cleaned up by
> > using kvrealloc(), but it hasn't been touched recently in a way that I
> > would expect things to leak.
[...]
> > I suspect this is a false positive from kmemleak.
>
> Maybe Catalin can check.

I can't see anything obviously wrong with msr_build_context(), unless it
can somehow be called concurrently, the saved_msrs.array update wouldn't
be safe. Another place to look at is whether saved_context.saved_msrs is
getting corrupt somehow.

If you force another kmemleak scan (through echo scan > /sys/...), does
the leak report disappear? This would be a good indication of a false
positive, though this normally happens with structures that are changed
frequently.

A way to confirm (or rule out) a kmemleak false positive would be to
check that saved_context.saved_msrs.array still points to the
unreferenced object listed above (you may need some kernel
instrumentation). If it doesn't, we'd need to figure out what happened
with the previous array.

--
Catalin