All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Vyukov <dvyukov@google.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	kasan-dev <kasan-dev@googlegroups.com>
Subject: Re: kmemleak not always catching stuff
Date: Tue, 5 Sep 2017 16:23:47 +0200	[thread overview]
Message-ID: <CACT4Y+bpF2X1Dm3rMDPmB3OZ7T6gfpw3HZMW6tLUOi7MnZJwfg@mail.gmail.com> (raw)
In-Reply-To: <20170904100904.v5soe2afqebogefv@armageddon.cambridge.arm.com>

On Mon, Sep 4, 2017 at 12:09 PM, Catalin Marinas
<catalin.marinas@arm.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).

  parent reply	other threads:[~2017-09-05 14:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-01 22:33 kmemleak not always catching stuff Steven Rostedt
2017-09-02 10:35 ` Dmitry Vyukov
2017-09-04 10:09 ` Catalin Marinas
2017-09-05 14:15   ` Steven Rostedt
2017-09-08 17:45     ` Catalin Marinas
2017-09-08 18:34       ` Steven Rostedt
2017-09-05 14:23   ` Dmitry Vyukov [this message]
2017-09-05 14:39     ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACT4Y+bpF2X1Dm3rMDPmB3OZ7T6gfpw3HZMW6tLUOi7MnZJwfg@mail.gmail.com \
    --to=dvyukov@google.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=catalin.marinas@arm.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.