From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 32144C48BE5 for ; Tue, 15 Jun 2021 05:15:38 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C09AD6140C for ; Tue, 15 Jun 2021 05:15:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C09AD6140C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 588D46B0036; Tue, 15 Jun 2021 01:15:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 50C446B006E; Tue, 15 Jun 2021 01:15:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 35F396B0070; Tue, 15 Jun 2021 01:15:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0183.hostedemail.com [216.40.44.183]) by kanga.kvack.org (Postfix) with ESMTP id F243E6B0036 for ; Tue, 15 Jun 2021 01:15:36 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 78D761DFC for ; Tue, 15 Jun 2021 05:15:36 +0000 (UTC) X-FDA: 78254795472.17.8769A5A Received: from mail-qt1-f173.google.com (mail-qt1-f173.google.com [209.85.160.173]) by imf13.hostedemail.com (Postfix) with ESMTP id 8B22CE000240 for ; Tue, 15 Jun 2021 05:15:27 +0000 (UTC) Received: by mail-qt1-f173.google.com with SMTP id z4so10352534qts.4 for ; Mon, 14 Jun 2021 22:15:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=R0wAP9EOwe53ziCwYyTPzQod1ybR+EAoh8nvLC2RE/U=; b=pyJzjUWHLEKplY05J/dGDBz62Lc4PMwKMUaIZMtL9LDPVwMQgadZl6Ew84ERUTqCkK L3rIUagVLAKyKm99kmfxOJtY1iWAy4nq8ac2zt9sYSwWoK086GipVkVfZkoA2QS7q2PU 914/S4AUq1CFHUD7tgIXS9ugUZXrFo97MmixGNKzBJjWtQolgQCQx+AiiaKzq4GFKWKy jgYCZuADfMBD/+RDBf9qoOqLy95Tf6x98N3Z/adzjuoJ2dV7WwUvXM2Kmr8MVEi1yFcV x26fYXTQsxGeh/GC79wR+t+jE8bkKiVwx5c0me8gLsDgv7Y+Xq0xuyzYzCX5BkK9TbfV /NPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=R0wAP9EOwe53ziCwYyTPzQod1ybR+EAoh8nvLC2RE/U=; b=V6gJ8JOnVhwQXwnSouloqaj+ZPS6PEyGx2V6KapRB+qe9k78Memi69aesYmUfWGv9e vU/1yRkiXeLhoAzWXlvx/UW5apThe87S3LZL8F1LO5YZB4H6DRPuPfihbw8nMW1IldcJ +DY3P80GVmh3iAiIGZ6KDJxywCWZprgH2AfSHeNFPvZ4mYzOxWJsYOvcE5tmV8Ek8q46 46PMwdDhSjEDKdqsAj3wvYu6K4VZcVQpsmlYnGAhVJo8sGLkmDK/+SlU2lGkYU0Hyj+5 7ZkoHqh6dUv/WdxB0g4mQXqW0Tfa+4eyRRklf8+Ih5ZaNnRinT/ahzVV0bqqhXgR95XJ CpQw== X-Gm-Message-State: AOAM533ust+/7YQ6kd6PXhleqJ1+P75IXoSgVCuQveq+sTwyCXTVDzJK lsqKdMj5d+GnyTvUh1KaYc6IUdsfFzbjxOGVhKEpQw== X-Google-Smtp-Source: ABdhPJyVp5fM6tkAzUnPPlLhc3CK5IVsP8w1kRWIvqxqM1LK2gcvuvzu58pgFfNGUkXfSRlqPbIV0KYnUTd4dIoBl5U= X-Received: by 2002:ac8:5949:: with SMTP id 9mr2405020qtz.67.1623734135120; Mon, 14 Jun 2021 22:15:35 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dmitry Vyukov Date: Tue, 15 Jun 2021 07:15:24 +0200 Message-ID: Subject: Re: kmemleak memory scanning To: Rustam Kovhaev Cc: Catalin Marinas , Andrew Morton , Linux-MM , LKML , Greg Kroah-Hartman Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=pyJzjUWH; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of dvyukov@google.com designates 209.85.160.173 as permitted sender) smtp.mailfrom=dvyukov@google.com X-Stat-Signature: 3nj4cyaz8rik894z1pd8nfudo7n4y59t X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 8B22CE000240 X-HE-Tag: 1623734127-282128 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Jun 14, 2021 at 10:31 PM Rustam Kovhaev wrote: > > hello Catalin, Andrew! > > while troubleshooting a false positive syzbot kmemleak report i have > noticed an interesting behavior in kmemleak and i wonder whether it is > behavior by design and should be documented, or maybe something to > improve. > apologies if some of the questions do not make sense, i am still going > through kmemleak code.. > > a) kmemleak scans struct page (kmemleak.c:1462), but it does not scan > the actual contents (page_address(page)) of the page. > if we allocate an object with kmalloc(), then allocate page with > alloc_page(), and if we put kmalloc pointer somewhere inside that page, > kmemleak will report kmalloc pointer as a false positive. > should we improve kmemleak and make it scan page contents? > or will this bring too many false negatives? Hi Rustam, Nice debugging! I assume lots of pages are allocated for slab and we don't want to scan the whole page if only a few slab objects are alive on the page. However alloc_pages() can be called by end kernel code as well. I grepped for any kmemleak annotations around existing calls to alloc_pages, but did not find any... Does it require an explicit kmemleak_alloc() after allocating the page and kmemleak_free () before freeing the page? If there are more than one use case for this, I guess we could add some GFP flag for this maybe. > b) when kmemleak object gets created (kmemleak.c:598) it gets checksum > of 0, by the time user requests kmemleak "scan" via debugfs the pointer > will be most likely changed to some value by the kernel and during > first scan kmemleak won't report the object as orphan even if it did not > find any reference to it, because it will execute update_checksum() and > after that will proceed to updating object->count (kmemleak.c:1502). > and so the user will have to initiate a second "scan" via debugfs and > only then kmemleak will produce the report. > should we document this? > > below i am attaching a simplified reproducer for the false positive > kmemleak report (a). > i could have done it in the module, but i found it to be easier and > faster to test when done in a syscall, so that i did not have to > modprobe/modprobe -r. > > tyvm! > > --- > arch/x86/entry/syscalls/syscall_64.tbl | 1 + > include/linux/syscalls.h | 1 + > mm/Makefile | 2 +- > mm/kmemleak_test.c | 45 ++++++++++++++++++++++++++ > 4 files changed, 48 insertions(+), 1 deletion(-) > create mode 100644 mm/kmemleak_test.c > > diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl > index ce18119ea0d0..da967a87eb78 100644 > --- a/arch/x86/entry/syscalls/syscall_64.tbl > +++ b/arch/x86/entry/syscalls/syscall_64.tbl > @@ -343,6 +343,7 @@ > 332 common statx sys_statx > 333 common io_pgetevents sys_io_pgetevents > 334 common rseq sys_rseq > +335 common kmemleak_test sys_kmemleak_test > # don't use numbers 387 through 423, add new calls after the last > # 'common' entry > 424 common pidfd_send_signal sys_pidfd_send_signal > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > index 050511e8f1f8..0602308aabf4 100644 > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -1029,6 +1029,7 @@ asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags, > unsigned mask, struct statx __user *buffer); > asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len, > int flags, uint32_t sig); > +asmlinkage long sys_kmemleak_test() > asmlinkage long sys_open_tree(int dfd, const char __user *path, unsigned flags); > asmlinkage long sys_move_mount(int from_dfd, const char __user *from_path, > int to_dfd, const char __user *to_path, > diff --git a/mm/Makefile b/mm/Makefile > index bf71e295e9f6..878783838fa1 100644 > --- a/mm/Makefile > +++ b/mm/Makefile > @@ -97,7 +97,7 @@ obj-$(CONFIG_CGROUP_HUGETLB) += hugetlb_cgroup.o > obj-$(CONFIG_GUP_TEST) += gup_test.o > obj-$(CONFIG_MEMORY_FAILURE) += memory-failure.o > obj-$(CONFIG_HWPOISON_INJECT) += hwpoison-inject.o > -obj-$(CONFIG_DEBUG_KMEMLEAK) += kmemleak.o > +obj-$(CONFIG_DEBUG_KMEMLEAK) += kmemleak.o kmemleak_test.o > obj-$(CONFIG_DEBUG_RODATA_TEST) += rodata_test.o > obj-$(CONFIG_DEBUG_VM_PGTABLE) += debug_vm_pgtable.o > obj-$(CONFIG_PAGE_OWNER) += page_owner.o > diff --git a/mm/kmemleak_test.c b/mm/kmemleak_test.c > new file mode 100644 > index 000000000000..828246e20b7f > --- /dev/null > +++ b/mm/kmemleak_test.c > @@ -0,0 +1,45 @@ > +// SPDX-License-Identifier: GPL-2.0+ > + > +#include > +#include > +#include > +#include > + > +struct kmemleak_check { > + unsigned long canary; > + struct work_struct work; > + struct page **pages; > +}; > + > +static void work_func(struct work_struct *work) > +{ > + struct page **pages; > + struct kmemleak_check *ptr; > + > + set_current_state(TASK_INTERRUPTIBLE); > + schedule_timeout(3600*HZ); > + > + ptr = container_of(work, struct kmemleak_check, work); > + pages = ptr->pages; > + __free_page(pages[0]); > + kvfree(pages); > +} > + > +SYSCALL_DEFINE0(kmemleak_test) > +{ > + struct page **pages, *page; > + struct kmemleak_check *ptr; > + > + pages = kzalloc(sizeof(*pages), GFP_KERNEL); > + page = alloc_page(GFP_KERNEL); > + pages[0] = page; > + ptr = page_address(page); > + ptr->canary = 0x00FF00FF00FF00FF; > + ptr->pages = pages; > + pr_info("DEBUG: pages %px page %px ptr %px\n", pages, page, ptr); > + > + INIT_WORK(&ptr->work, work_func); > + schedule_work(&ptr->work); > + > + return 0; > +} > -- > 2.30.2 >