Re: [PATCH] kmemleak: skip scanning holes in the .bss section

[Qian Cai]

Fixing some email addresses.

On Tue, 2019-03-12 at 15:14 -0400, Qian Cai wrote:
> The commit 2d4f567103ff ("KVM: PPC: Introduce kvm_tmp framework") adds
> kvm_tmp[] into the .bss section and then free the rest of unused spaces
> back to the page allocator.
> 
> kernel_init
> Â kvm_guest_init
> ÂÂÂÂkvm_free_tmp
> ÂÂÂÂÂÂfree_reserved_area
> ÂÂÂÂÂÂÂÂfree_unref_page
> ÂÂÂÂÂÂÂÂÂÂfree_unref_page_prepare
> 
> With DEBUG_PAGEALLOC=y, it will unmap those pages from kernel. As the
> result, kmemleak scan will trigger a panic below when it scans the .bss
> section with unmapped pages.
> 
> Since this is done way before the first kmemleak_scan(), just go
> lockless to make the implementation simple and skip those pages when
> scanning the .bss section. Later, those pages could be tracked by
> kmemleak again once allocated by the page allocator. Overall, this is
> such a special case, so no need to make it a generic to let kmemleak
> gain an ability to skip blocks in scan_large_block().
> 
> BUG: Unable to handle kernel data access at 0xc000000001610000
> Faulting instruction address: 0xc0000000003cc178
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA pSeries
> CPU: 3 PID: 130 Comm: kmemleak Kdump: loaded Not tainted 5.0.0+ #9
> REGS: c0000004b05bf940 TRAP: 0300ÂÂÂNot taintedÂÂ(5.0.0+)
> NIP [c0000000003cc178] scan_block+0xa8/0x190
> LR [c0000000003cc170] scan_block+0xa0/0x190
> Call Trace:
> [c0000004b05bfbd0] [c0000000003cc170] scan_block+0xa0/0x190 (unreliable)
> [c0000004b05bfc30] [c0000000003cc2c0] scan_large_block+0x60/0xa0
> [c0000004b05bfc70] [c0000000003ccc64] kmemleak_scan+0x254/0x960
> [c0000004b05bfd40] [c0000000003cdd50] kmemleak_scan_thread+0xec/0x12c
> [c0000004b05bfdb0] [c000000000104388] kthread+0x1b8/0x1c0
> [c0000004b05bfe20] [c00000000000b364] ret_from_kernel_thread+0x5c/0x78
> Instruction dump:
> 7fa3eb78 4844667d 60000000 60000000 60000000 60000000 3bff0008 7fbcf840
> 409d00b8 4bfffeed 2fa30000 409e00ac <e87f0000> e93e0128 7fa91840
> 419dffdc
> 
> Signed-off-by: Qian Cai <cai@xxxxxx>
> ---
> Âarch/powerpc/kernel/kvm.c |ÂÂ3 +++
> Âinclude/linux/kmemleak.hÂÂ|ÂÂ4 ++++
> Âmm/kmemleak.cÂÂÂÂÂÂÂÂÂÂÂÂÂ| 25 ++++++++++++++++++++++++-
> Â3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
> index 683b5b3805bd..5cddc8fc56bb 100644
> --- a/arch/powerpc/kernel/kvm.c
> +++ b/arch/powerpc/kernel/kvm.c
> @@ -26,6 +26,7 @@
> Â#include <linux/slab.h>
> Â#include <linux/of.h>
> Â#include <linux/pagemap.h>
> +#include <linux/kmemleak.h>
> Â
> Â#include <asm/reg.h>
> Â#include <asm/sections.h>
> @@ -712,6 +713,8 @@ static void kvm_use_magic_page(void)
> Â
> Âstatic __init void kvm_free_tmp(void)
> Â{
> +	kmemleak_bss_hole(&kvm_tmp[kvm_tmp_index],
> +			ÂÂ&kvm_tmp[ARRAY_SIZE(kvm_tmp)]);
> Â	free_reserved_area(&kvm_tmp[kvm_tmp_index],
> Â			ÂÂÂ&kvm_tmp[ARRAY_SIZE(kvm_tmp)], -1, NULL);
> Â}
> diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
> index 5ac416e2d339..3d8949b9c6f5 100644
> --- a/include/linux/kmemleak.h
> +++ b/include/linux/kmemleak.h
> @@ -46,6 +46,7 @@ extern void kmemleak_alloc_phys(phys_addr_t phys, size_t
> size, int min_count,
> Âextern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref;
> Âextern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;
> Âextern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
> +extern void kmemleak_bss_hole(void *start, void *stop);
> Â
> Âstatic inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
> Â					ÂÂÂÂint min_count, slab_flags_t
> flags,
> @@ -131,6 +132,9 @@ static inline void kmemleak_not_leak_phys(phys_addr_t
> phys)
> Âstatic inline void kmemleak_ignore_phys(phys_addr_t phys)
> Â{
> Â}
> +static inline void kmemleak_bss_hole(void *start, void *stop)
> +{
> +}
> Â
> Â#endif	/* CONFIG_DEBUG_KMEMLEAK */
> Â
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 707fa5579f66..42349cd9ef7a 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -237,6 +237,10 @@ static int kmemleak_skip_disable;
> Â/* If there are leaks that can be reported */
> Âstatic bool kmemleak_found_leaks;
> Â
> +/* Skip scanning of a range in the .bss section. */
> +static void *bss_hole_start;
> +static void *bss_hole_stop;
> +
> Âstatic bool kmemleak_verbose;
> Âmodule_param_named(verbose, kmemleak_verbose, bool, 0600);
> Â
> @@ -1265,6 +1269,18 @@ void __ref kmemleak_ignore_phys(phys_addr_t phys)
> Â}
> ÂEXPORT_SYMBOL(kmemleak_ignore_phys);
> Â
> +/**
> + * kmemleak_bss_hole - skip scanning a range in the .bss section
> + *
> + * @start:	start of the range
> + * @stop:	end of the range
> + */
> +void kmemleak_bss_hole(void *start, void *stop)
> +{
> +	bss_hole_start = start;
> +	bss_hole_stop = stop;
> +}
> +
> Â/*
> Â * Update an object's checksum and return true if it was modified.
> Â */
> @@ -1531,7 +1547,14 @@ static void kmemleak_scan(void)
> Â
> Â	/* data/bss scanning */
> Â	scan_large_block(_sdata, _edata);
> -	scan_large_block(__bss_start, __bss_stop);
> +
> +	if (bss_hole_start) {
> +		scan_large_block(__bss_start, bss_hole_start);
> +		scan_large_block(bss_hole_stop, __bss_stop);
> +	} else {
> +		scan_large_block(__bss_start, __bss_stop);
> +	}
> +
> Â	scan_large_block(__start_ro_after_init, __end_ro_after_init);
> Â
> Â#ifdef CONFIG_SMP