linux-kernel - Re: [Resend RFC PATCH] mm: introduce __GFP_TRACKLEAK to track in-kernel allocation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGWkznHzCnjUUjceAvj_E_WCSwGP5_SoGYDeKaHL9dTeUinEBQ@mail.gmail.com>
Date:   Tue, 6 Sep 2022 15:29:52 +0800
From:   Zhaoyang Huang <huangzhaoyang@...il.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        "zhaoyang.huang" <zhaoyang.huang@...soc.com>,
        Catalin Marinas <catalin.marinas@....com>,
        "open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>, Ke Wang <ke.wang@...soc.com>
Subject: Re: [Resend RFC PATCH] mm: introduce __GFP_TRACKLEAK to track
 in-kernel allocation

On Sat, Sep 3, 2022 at 3:08 AM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Fri, Sep 02, 2022 at 11:58:39AM -0700, Andrew Morton wrote:
> > Cc willy for page-flags changes.
>
> Thanks.  This is probably OK.  The biggest problem is that it won't
> work for drivers which allocate memory and then map it to userspace.
> If they try, they'll get a nice splat, but it may limit the usefulness
> of this option.  We should probably document that limitation in this
> patch.
>
> > On Fri, 2 Sep 2022 18:59:07 +0800 "zhaoyang.huang" <zhaoyang.huang@...soc.com> wrote:
> > > +++ b/mm/page_alloc.c
> > > @@ -1361,6 +1361,8 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > >             page->mapping = NULL;
> > >     if (memcg_kmem_enabled() && PageMemcgKmem(page))
> > >             __memcg_kmem_uncharge_page(page, order);
> > > +   if (PageTrackleak(page))
> > > +           kmemleak_free(page);
>
> Don't we also need to __ClearPageTrackleak()?
ok
>
> > > +   if (gfp & __GFP_TRACKLEAK) {
> >
> > And we'd want __GFP_TRACKLEAK to evaluate to zero at compile time if
> > CONFIG_HAVE_DEBUG_KMEMLEAK=n.
> >
> > > +           kmemleak_alloc(page_address(page), PAGE_SIZE << order, 1, gfp & ~__GFP_TRACKLEAK);
> > > +           __SetPageTrackleak(page);
> > > +   }
>
> We only set this on the first page we allocate.  I think there's a
> problem for multi-page, non-compound allocations, no?  Particularly
> when you consider the problem fixed in e320d3012d25.
Please correct me if I am wrong. AFAICT, the leak_object is created
for tracking from page_address(page) to page_address(page) + PAGE_SIZE
<< order via checksum of the whole area while not referring to
page->cnt. Non-compound high order tail pages will not be checked
anymore since the leak_object has been freed together with head page
by put_page, whereas, it would not be a problem as the tail pages
should NOT be accessed also as they should be considered as buddy
pages, no?
>
> I'm not opposed to this tracking, it just needs a bit more thought and
> awareness of some of the corner cases of the VM.  A few test cases would
> be nice; they could demonstrate that this works for both compound and
> non-compound high-order allocations.