Re: [PATCH] memblock: exclude NOMAP regions from kmemleak

From: Anshuman Khandual <anshuman.khandual@arm.com>
To: Mike Rapoport <rppt@kernel.org>, linux-mm@kvack.org
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] memblock: exclude NOMAP regions from kmemleak
Date: Tue, 19 Oct 2021 09:51:29 +0530	[thread overview]
Message-ID: <c8fe322b-51e1-1250-8b8c-b711c929f634@arm.com> (raw)
In-Reply-To: <20211013054756.12177-1-rppt@kernel.org>

On 10/13/21 11:17 AM, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.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@linux.ibm.com>
> Tested-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.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@arm.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 ?