Message-ID: <CACT4Y+bpF2X1Dm3rMDPmB3OZ7T6gfpw3HZMW6tLUOi7MnZJwfg@mail.gmail.com>
Date:   Tue, 5 Sep 2017 16:23:47 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        kasan-dev <kasan-dev@...glegroups.com>
Subject: Re: kmemleak not always catching stuff

On Mon, Sep 4, 2017 at 12:09 PM, Catalin Marinas
<catalin.marinas@....com> wrote:
> Hi Steve,
>
> On Fri, Sep 01, 2017 at 06:33:11PM -0400, Steven Rostedt wrote:
>> Recently kmemleak discovered a bug in my code where an allocated
>> trampoline for a ftrace function tracer wasn't freed due to an exit
>> path. The thing is, kmemleak was able to catch this 100% when it was
>> triggered by one of my ftrace selftests that happen at bootup. But when
>> I trigger the issue from user space after bootup finished, it would not
>> catch it.
>
> Is this the create_filter() fix that went in recently?
>
>> Now I was thinking that it may be due to the fact that the trampoline
>> is allocated with module_alloc(), and that has some magic kasan goo in
>> it. But when forcing the issue with adding the following code:
>>
>>       void **pblah;
>>       void *blah;
>>
>>       pblah = kmalloc(sizeof(*pblah), GFP_KERNEL);
>>       blah = module_alloc(PAGE_SIZE);
>>       *pblah = blah;
>>       printk("allocated blah %p\n", blah);
>>       kfree(pblah);
>>
>> in a path that I could control, it would catch it only after doing it
>> several times. I was never able to have kmemleak catch the actual bug
>> from user space no matter how many times I triggered it.
>
> module_alloc() uses vmalloc_exec(), so it is tracked by kmemleak but you
> probably hit a false negative with the blah pointer lingering somewhere
> on some stack.
>
>>  # dmesg |grep kmemleak
>> [   16.746832] kmemleak: Kernel memory leak detector initialized
>> [   16.746888] kmemleak: Automatic memory scanning thread started
>>
>> And then I would do:
>>
>>  # echo scan=on > /sys/kernel/debug/kmemleak
>
> scan=on is not necessary since this just enables the scanning thread
> (already started as per dmesg).
>
>>  [do the test]
>>
>>  # echo scan > /sys/kernel/debug/kmemleak
>
> Some heuristics in kmemleak cause the first leak of an object not to be
> reported (too many false positives). You'd need to do "echo scan" at
> least twice after an allocation.
>
> I tried the same test code you have above triggered with an echo ... >
> /sys from user space. After the second scan it shows the leak, both with
> and without KASan.
>
>> Most of the times it found nothing. Even when I switched the above from
>> module_alloc() to kmalloc().
>>
>> Is this normal?
>
> In general, a leak would eventually appear after a few scans or in time
> when some memory location is overridden.
>
> Yet another heuristics in kmemleak is to treat pointers at some offset
> inside an object as valid references (because of the container_of
> tricks). However, the downside is that the bigger the object, the
> greater chances of finding some random data that looks like a pointer.
> We could change this logic to require precise pointers above a certain
> size (e.g. PAGE_SIZE) where the use of container_of() is less likely.
>
> Kmemleak doesn't have a way to inspect false negatives but if you are
> interested in digging further, I could add a "find=0x..." command to
> print all references to an object during scanning. I also need to find
> some time to implement a "stopscan" command which uses stop_machine()
> and skips the heuristics for reducing false positives.

"stopscan" would be great. We would like to deploy continuous testing
with kmemleak, but while it has systematic false positives, it is not
possible. Performance kinda matters, but only when a tool does not
have false positives. We probably will just not enable it on all
machines (it introduces significant slowdown even today).

Powered by blists - more mailing lists