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]

Date:   Fri, 2 Sep 2022 20:08:52 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     "zhaoyang.huang" <zhaoyang.huang@...soc.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Zhaoyang Huang <huangzhaoyang@...il.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, ke.wang@...soc.com
Subject: Re: [Resend RFC PATCH] mm: introduce __GFP_TRACKLEAK to track
 in-kernel allocation

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()?

> > +	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.

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.