[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <qjmnlgowadwtndmwween63mmpi2krtbioagani4paorzgcecaa@e625xdk4ya52>
Date: Fri, 20 Jun 2025 17:45:39 +0200
From: Klara Modin <klarasmodin@...il.com>
To: Hao Ge <hao.ge@...ux.dev>
Cc: Catalin Marinas <catalin.marinas@....com>,
Suren Baghdasaryan <surenb@...gle.com>, Kent Overstreet <kent.overstreet@...ux.dev>,
Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
Hao Ge <gehao@...inos.cn>
Subject: Re: [PATCH] mm/alloc_tag: Fix the kmemleak false positive issue in
the allocation of the percpu variable tag->counters
Hi,
On 2025-06-20 02:31:54 +0800, Hao Ge wrote:
> From: Hao Ge <gehao@...inos.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@...inos.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]
Powered by blists - more mailing lists