linux-kernel - Re: [PATCH] memblock: exclude NOMAP regions from 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: <c8fe322b-51e1-1250-8b8c-b711c929f634@arm.com>
Date:   Tue, 19 Oct 2021 09:51:29 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     Mike Rapoport <rppt@...nel.org>, linux-mm@...ck.org
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] memblock: exclude NOMAP regions from kmemleak



On 10/13/21 11:17 AM, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@...ux.ibm.com>
> 
> Vladimir Zapolskiy reports:
> 
> commit a7259df76702 ("memblock: make memblock_find_in_range method private")
> invokes a kernel panic while running kmemleak on OF platforms with nomaped
> regions:
> 
>   Unable to handle kernel paging request at virtual address fff000021e00000
>   [...]
>     scan_block+0x64/0x170
>     scan_gray_list+0xe8/0x17c
>     kmemleak_scan+0x270/0x514
>     kmemleak_write+0x34c/0x4ac
> 
> Indeed, NOMAP regions don't have linear map entries so an attempt to scan
> these areas would fault.
> 
> Prevent such faults by excluding NOMAP regions from kmemleak.
> 
> Link: https://lore.kernel.org/all/8ade5174-b143-d621-8c8e-dc6a1898c6fb@linaro.org
> Fixes: a7259df76702 ("memblock: make memblock_find_in_range method private")
> Signed-off-by: Mike Rapoport <rppt@...ux.ibm.com>
> Tested-by: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>
> ---
>  mm/memblock.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 184dcd2e5d99..5c3503c98b2f 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -936,7 +936,12 @@ int __init_memblock memblock_mark_mirror(phys_addr_t base, phys_addr_t size)
>   */
>  int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
>  {
> -	return memblock_setclr_flag(base, size, 1, MEMBLOCK_NOMAP);
> +	int ret = memblock_setclr_flag(base, size, 1, MEMBLOCK_NOMAP);
> +
> +	if (!ret)
> +		kmemleak_free_part_phys(base, size);
> +
> +	return ret;
>  }
>  
>  /**
> 
> base-commit: 64570fbc14f8d7cb3fe3995f20e26bc25ce4b2cc
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@....com>

A small nit though.

Just wondering. Should not the comment for memblock_mark_nomap() be
updated (or add a comment in the function) to explain the reason to
call kmemleak_free_part_phys(), to emphasize that a scan would fail
for such memory ranges due to lack of linear mapping ?