Re: [PATCH] perf symbols: Don't let a module's last symbol overlap the next module

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Sep 28, 2026 at 04:32:27PM -0700, Alireza Haghdoost via B4 Relay wrote:
> From: Alireza Haghdoost <haghdoost@uber.com>
> 
> symbols__fixup_end() extends a zero-size symbol that is the last one in
> the kernel or in a module to the end of the next page. On x86 the next
> module can start in that page, so the stretched symbol overlaps it, and
> a lookup can return the wrong symbol. For example:
> 
>   ffffffffc0c4aa50 t dca_sysfs_exit    [dca]
>   ffffffffc0c4b000 t __nft_trace_packet [nf_tables]
>   ffffffffc0c4b0b0 t nft_do_chain      [nf_tables]
> 
> dca_sysfs_exit gets end 0xffffffffc0c4c000, and samples in nft_do_chain
> are reported as dca_sysfs_exit.
> 
> This patch clamps the end to the next symbol's start. The ARM case this
> code was added for, where the next module is far away, is unchanged.
> 
> Add a "Kallsyms symbol ends" test case to the Symbols suite that covers
> the kernel/module boundary, adjacent modules and a distant module.
> 
> Fixes: 8799ebce84d6 ("perf symbol: Update symbols__fixup_end()")
> Fixes: bacefe0c7b77 ("perf tools: Fixup module symbol end address properly")
> Signed-off-by: Alireza Haghdoost <haghdoost@uber.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/util/symbol.c   |  4 ++
>  tools/perf/tests/symbols.c | 96 +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 99 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 163652f071c6f..f0a7f5a30e696 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -302,6 +302,10 @@ void symbols__fixup_end(struct rb_root_cached *symbols, bool is_kallsyms)
>  			else
>  				prev->end = curr->start;
>  
> +			/* The next module can start within that page */
> +			if (prev->end > curr->start)
> +				prev->end = curr->start;
> +
>  			pr_debug4("%s sym:%s end:%#" PRIx64 "\n",
>  				  __func__, prev->name, prev->end);
>  		}
> diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c
> index c09e04f36035a..b644d95c6be9a 100644
> --- a/tools/perf/tests/symbols.c
> +++ b/tools/perf/tests/symbols.c
> @@ -2,6 +2,7 @@
>  #include <linux/compiler.h>
>  #include <linux/string.h>
>  #include <sys/mman.h>
> +#include <inttypes.h>
>  #include <limits.h>
>  #include "debug.h"
>  #include "dso.h"
> @@ -223,4 +224,97 @@ static int test__symbols(struct test_suite *test __maybe_unused, int subtest __m
>  	return ret;
>  }
>  
> -DEFINE_SUITE("Symbols", symbols);
> +struct kallsyms_sym {
> +	u64 start;
> +	const char *name;
> +};
> +
> +/*
> + * kallsyms from an x86 host where nf_tables was loaded in the page right
> + * after the last symbol of dca, plus a kernel symbol followed closely by a
> + * module and a module far away from the others.
> + */
> +static const struct kallsyms_sym kallsyms_syms[] = {
> +	{ 0xffffffff81000000, "_stext" },
> +	{ 0xffffffff81000ff0, "last_kernel_symbol" },
> +	{ 0xffffffff81001000, "near_module_symbol\t[near]" },
> +	{ 0xffffffffc0c4aa40, "dca_exit\t[dca]" },
> +	{ 0xffffffffc0c4aa50, "dca_sysfs_exit\t[dca]" },
> +	{ 0xffffffffc0c4b000, "__nft_trace_packet\t[nf_tables]" },
> +	{ 0xffffffffc0c4b0b0, "nft_do_chain\t[nf_tables]" },
> +	{ 0xffffffffc0c4b4e0, "nf_tables_core_module_exit\t[nf_tables]" },
> +	{ 0xffffffffc2000000, "far_module_symbol\t[far]" },
> +};
> +
> +static int check_symbol(struct dso *dso, u64 addr, const char *name, u64 end)
> +{
> +	struct symbol *sym = dso__find_symbol_nocache(dso, addr);
> +
> +	if (!sym || strcmp(sym->name, name)) {
> +		pr_debug("%#" PRIx64 ": expected %s, got %s\n", addr, name,
> +			 sym ? sym->name : "no symbol");
> +		return TEST_FAIL;
> +	}
> +	if (sym->end != end) {
> +		pr_debug("%s: expected end %#" PRIx64 ", got %#" PRIx64 "\n",
> +			 name, end, sym->end);
> +		return TEST_FAIL;
> +	}
> +	return TEST_OK;
> +}
> +
> +static int test__kallsyms_fixup_end(struct test_suite *test __maybe_unused,
> +				    int subtest __maybe_unused)
> +{
> +	struct dso *dso = dso__new("[kernel.kallsyms]");
> +	int ret = TEST_FAIL;
> +
> +	if (!dso)
> +		return TEST_FAIL;
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(kallsyms_syms); i++) {
> +		struct symbol *sym = symbol__new(kallsyms_syms[i].start, 0, 0, 0,
> +						 kallsyms_syms[i].name);
> +
> +		if (!sym)
> +			goto out;
> +		symbols__insert(dso__symbols(dso), sym);
> +	}
> +
> +	symbols__fixup_end(dso__symbols(dso), true);
> +
> +	ret = TEST_OK;
> +	/* The next symbol is too close for the end of the page. */
> +	if (check_symbol(dso, 0xffffffff81000ff8, "last_kernel_symbol",
> +			 0xffffffff81001000))
> +		ret = TEST_FAIL;
> +	if (check_symbol(dso, 0xffffffffc0c4aa58, "dca_sysfs_exit\t[dca]",
> +			 0xffffffffc0c4b000))
> +		ret = TEST_FAIL;
> +	if (check_symbol(dso, 0xffffffffc0c4b280, "nft_do_chain\t[nf_tables]",
> +			 0xffffffffc0c4b4e0))
> +		ret = TEST_FAIL;
> +
> +	/* Far from the next module, the end of the page is still used. */
> +	if (check_symbol(dso, 0xffffffffc0c4b4f0,
> +			 "nf_tables_core_module_exit\t[nf_tables]",
> +			 0xffffffffc0c4d000))
> +		ret = TEST_FAIL;
> +	if (check_symbol(dso, 0xffffffff81001008, "near_module_symbol\t[near]",
> +			 0xffffffff81002000))
> +		ret = TEST_FAIL;
> +out:
> +	dso__put(dso);
> +	return ret;
> +}
> +
> +static struct test_case tests__symbols[] = {
> +	TEST_CASE("Symbols", symbols),
> +	TEST_CASE("Kallsyms symbol ends", kallsyms_fixup_end),
> +	{ .name = NULL, }
> +};
> +
> +struct test_suite suite__symbols = {
> +	.desc = "Symbols",
> +	.test_cases = tests__symbols,
> +};
> 
> ---
> base-commit: 0ae6fc78c5ce0dfd18d8712a50f0fd4602eff103
> change-id: 20260928-haghdoost-perf-symbols-fixup-module-end-11ce6858f0c4
> 
> Best regards,
> --  
> Alireza Haghdoost <haghdoost@uber.com>
> 
>