Re: [PATCH] mm/alloc_tag: Fix the kmemleak false positive issue in the allocation of the percpu variable tag->counters

[Klara Modin <klarasmodin@gmail.com>]

Hi,

On 2025-06-20 02:31:54 +0800, Hao Ge wrote:
> From: Hao Ge <gehao@kylinos.cn>
> 
> When loading a module, as long as the module has memory
> allocation operations, kmemleak produces a false positive
> report that resembles the following:
> 
> unreferenced object (percpu) 0x7dfd232a1650 (size 16):
>   comm "modprobe", pid 1301, jiffies 4294940249
>   hex dump (first 16 bytes on cpu 2):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace (crc 0):
>     kmemleak_alloc_percpu+0xb4/0xd0
>     pcpu_alloc_noprof+0x700/0x1098
>     load_module+0xd4/0x348
>     codetag_module_init+0x20c/0x450
>     codetag_load_module+0x70/0xb8
>     load_module+0xef8/0x1608
>     init_module_from_file+0xec/0x158
>     idempotent_init_module+0x354/0x608
>     __arm64_sys_finit_module+0xbc/0x150
>     invoke_syscall+0xd4/0x258
>     el0_svc_common.constprop.0+0xb4/0x240
>     do_el0_svc+0x48/0x68
>     el0_svc+0x40/0xf8
>     el0t_64_sync_handler+0x10c/0x138
>     el0t_64_sync+0x1ac/0x1b0
> 
> This is because the module can only indirectly reference alloc_tag_counters
> through the alloc_tag section, which misleads kmemleak.
> 
> However, we don't have a kmemleak ignore interface for percpu
> allocations yet. So let's create one and invoke it for tag->counters.
> 
> Fixes: 12ca42c23775 ("alloc_tag: allocate percpu counters for module tags dynamically")
> Signed-off-by: Hao Ge <gehao@kylinos.cn>
> ---
>  include/linux/kmemleak.h |  1 +
>  lib/alloc_tag.c          |  8 +++++++-
>  mm/kmemleak.c            | 14 ++++++++++++++
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
> index 93a73c076d16..2ea8e66bf689 100644
> --- a/include/linux/kmemleak.h
> +++ b/include/linux/kmemleak.h
> @@ -28,6 +28,7 @@ extern void kmemleak_update_trace(const void *ptr) __ref;
>  extern void kmemleak_not_leak(const void *ptr) __ref;
>  extern void kmemleak_transient_leak(const void *ptr) __ref;
>  extern void kmemleak_ignore(const void *ptr) __ref;

> +extern void kmemleak_igonore_percpu(const void __percpu *ptr) __ref;

Note that there's no stub defined in the #else block, which means this
will fail to build if CONFIG_DEBUG_KMEMLEAK is disabled.

>  extern void kmemleak_scan_area(const void *ptr, size_t size, gfp_t gfp) __ref;
>  extern void kmemleak_no_scan(const void *ptr) __ref;
>  extern void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index d48b80f3f007..de6dcf4ea0f5 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -10,6 +10,7 @@
>  #include <linux/seq_buf.h>
>  #include <linux/seq_file.h>
>  #include <linux/vmalloc.h>
> +#include <linux/kmemleak.h>
>  
>  #define ALLOCINFO_FILE_NAME		"allocinfo"
>  #define MODULE_ALLOC_TAG_VMAP_SIZE	(100000UL * sizeof(struct alloc_tag))
> @@ -632,8 +633,13 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
>  			       mod->name);
>  			return -ENOMEM;
>  		}
> -	}
>  
> +		/*
> +		 * Avoid a kmemleak false positive. The pointer to the counters is stored
> +		 * in the alloc_tag section of the module and cannot be directly accessed.
> +		 */
> +		kmemleak_igonore_percpu(tag->counters);
> +	}
>  	return 0;
>  }
>  
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index da9cee34ee1b..8797fe88861e 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1246,6 +1246,20 @@ void __ref kmemleak_transient_leak(const void *ptr)
>  }
>  EXPORT_SYMBOL(kmemleak_transient_leak);
>  
> +/**

> + * kmemleak_ignore_phys - similar to kmemleak_ignore but taking a percpu

This should match with the function below.

> + *			  address argument
> + * @ptr:	percpu address of the object
> + */
> +void __ref kmemleak_igonore_percpu(const void __percpu *ptr)
> +{
> +	pr_debug("%s(0x%px)\n", __func__, ptr);
> +
> +	if (kmemleak_enabled && ptr && !IS_ERR_PCPU(ptr))
> +		make_black_object((unsigned long)ptr, OBJECT_PERCPU);
> +}
> +EXPORT_SYMBOL_GPL(kmemleak_igonore_percpu);
> +
>  /**
>   * kmemleak_ignore - ignore an allocated object
>   * @ptr:	pointer to beginning of the object
> -- 
> 2.25.1
> 

s/igonore/ignore/

The commit summary should probably also be shortened, it is recommended
to be at most 70-75 characters[1]. Perhaps something like

mm/alloc_tag: Fix kmemleak false positive in percpu tag->counters

could be appropriate?

Regards,
Klara Modin

Link: https://www.kernel.org/doc/html/latest/process/submitting-patches.html [1]