linux-kernel - Re: Potential Double Scanning of struct page Physical Memory by kmemleak

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] [day] [month] [year] [list]

Message-ID: <Z3PulGvf4Zpfe4ND@arm.com>
Date: Tue, 31 Dec 2024 13:16:04 +0000
From: Catalin Marinas <catalin.marinas@....com>
To: Weikang Guo <guoweikang.kernel@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Linux Memory Management List <linux-mm@...ck.org>,
	linux-kernel@...r.kernel.org
Subject: Re: Potential Double Scanning of struct page Physical Memory by
 kmemleak

Hi Guo,

On Wed, Dec 25, 2024 at 04:09:12PM +0800, Weikang Guo wrote:
> Problem Description:
> 
> When CONFIG_SPARSEMEM is enabled, the memory for `struct page`objects
> in the sections is allocated via `sparse_buffer` using the
> `memblock_alloc_xxx` interface. Since memblock does not explicitly
> specify `MEMBLOCK_ALLOC_NOLEAKTRACE`, kmemleak will treat the memory
> as a gray object with mincount=0. In other words, the physical memory
> occupied by `struct page` will be scanned by kmemleak as a gray object
> during the scan thread.
> 
> Additionally, kmemleak also traverses and scans valid struct page
> objects in the zone . As a result, the physical memory occupied by
> struct page may end up being scanned twice by scan thread.

Yes, I can see how this happens. I don't remember how we ended up like
this, maybe kmemleak did not track memblock allocations in the early
days.

> Possible solution: Specify `MEMBLOCK_ALLOC_NOLEAKTRACE` when alloc
> `struct page memory`

I think that's the easiest and just let kmemleak scan the mem_map
explicitly, whether it's in the linear map or vmemmap.

The way we ended up with marking 'nokleaktrace' blocks in the memblock
API isn't great. This "flag" used to be called MEMBLOCK_ALLOC_KASAN and
only used by KASAN (implying accessible). But it's not an actual flag,
just some random value passed as the 'end' argument to memblock_alloc()
and friends. Luckily memmap_alloc() only needs MEMBLOCK_ALLOC_ACCESSIBLE
which is implied by MEMBLOCK_ALLOC_NOLEAKTRACE (though I can't find any
documentation about this).

Anyway, if you fix memmap_alloc(), please add a comment that kmemleak
scans this explicitly. Also add a comment where we define
MEMBLOCK_ALLOC_NOLEAKTRACE to state that it implies
MEMBLOCK_ALLOC_ACCESSIBLE. Ideally we should have named this
MEMBLOCK_ALLOC_ACCESSIBLE_NOLEAKTRACE but it's nearly half the
recommended line length.

Thanks.

-- 
Catalin