[PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Zheng Yejian]

Background of this patch set can be found in v1:
https://lore.kernel.org/all/20240613133711.2867745-1-zhengyejian1@huawei.com/

Here add a reproduction to show the impact to livepatch:
1. Add following hack to make livepatch-sample.ko do patch on do_one_initcall()
   which has an overriden weak function behind in vmlinux, then print the
   actually used __fentry__ location:

  diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
  index 90408500e5a3..20b75e7b1771 100644
  --- a/kernel/livepatch/patch.c
  +++ b/kernel/livepatch/patch.c
  @@ -173,6 +173,8 @@ static int klp_patch_func(struct klp_func *func)
                  unsigned long ftrace_loc;

                  ftrace_loc = ftrace_location((unsigned long)func->old_func);
  +               pr_info("%s: old_func=%pS ftrace_loc=%pS (%lx)\n", __func__,
  +                       func->old_func, (void *)ftrace_loc, ftrace_loc);
                  if (!ftrace_loc) {
                          pr_err("failed to find location for function '%s'\n",
                                  func->old_name);
  diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
  index cd76d7ebe598..c6e8e78e23b8 100644
  --- a/samples/livepatch/livepatch-sample.c
  +++ b/samples/livepatch/livepatch-sample.c
  @@ -38,7 +38,7 @@ static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)

   static struct klp_func funcs[] = {
          {
  -               .old_name = "cmdline_proc_show",
  +               .old_name = "do_one_initcall",
                  .new_func = livepatch_cmdline_proc_show,
          }, { }
   };

2. We can see the size of do_one_initcall() is 0x27e:
  $ nm -nS vmlinux | grep do_one_initcall
  ffffffff810021e0 000000000000027e T do_one_initcall

3. Insert the livepatch ko, the expected ftrace_loc is do_one_initcall+0x4/0x27e but
   actually do_one_initcall+0x294/0x2c0 which is invalid, also its size is inaccurate.
   Then this livepatch can not properly work!!!

  # insmod livepatch-sample.ko
  [   17.942451] livepatch: klp_patch_func: old_func=do_one_initcall+0x0/0x2c0 ftrace_loc=do_one_initcall+0x294/0x2c0 (ffffffff81002474)
  #
  # cat /sys/kernel/tracing/available_filter_functions_addrs | grep ffffffff81002474
  ffffffff81002474 __ftrace_invalid_address___660

After this patch, this issue is gone:

  # insmod livepatch-sample.ko
  [   42.408632] livepatch: klp_patch_func: old_func=do_one_initcall+0x0/0x27e ftrace_loc=do_one_initcall+0x4/0x27e (ffffffff810021e4)
  # cat /sys/kernel/tracing/available_filter_functions_addrs | grep ffffffff810021e4
  ffffffff810021e4 do_one_initcall
  # cat /sys/kernel/tracing/enabled_functions
  do_one_initcall (1)   I     M   tramp: 0xffffffffa0020000 (klp_ftrace_handler+0x0/0x258)->klp_ftrace_handler+0x0/0x258

Changes:

v1->v2:
  - Drop patch which titled "kallsyms: Optimize multiple times of realloc() to one time of malloc()"
    as suggested by Masahiro;
    Link: https://lore.kernel.org/all/CAK7LNAQkSnZ1nVXiZH9kg52H-A_=urcsv-W7wGXvunMGhGX8Vw@mail.gmail.com/
  - Changes in PATCH 1/5:
    - Change may_exist_hole_after_symbol() to has_hole() and return bool type;
    - User realloc instead of malloc another table in emit_hole_symbols();
      Link: https://lore.kernel.org/all/CAK7LNAQaLc6aDK85qQtPHoCkQSGyL-TxXjpgJTfehe2Q1=jMSA@mail.gmail.com/
    - Use empty symbol type/name as a special case to represent the hole instead of
      using symbol "t__hole_symbol_XXXXX";
      Link: https://lore.kernel.org/all/CAK7LNARiR5z9hPRG932T7YjRWqkX_qZ7WKmbxx7iTo2w5YJojQ@mail.gmail.com/
  - Changes in PATCH 3/5:
    - Now name of hole symbol is NULL, so if __fentry__ is located in a hole,
      kallsyms_lookup() find an NULL name then will return 0, so drop the
      needless kallsyms_is_hole_symbol().

Zheng Yejian (5):
  kallsyms: Emit symbol at the holes in the text
  module: kallsyms: Determine exact function size
  ftrace: Skip invalid __fentry__ in ftrace_process_locs()
  ftrace: Fix possible out-of-bound issue in ftrace_process_locs()
  ftrace: Revert the FTRACE_MCOUNT_MAX_OFFSET workaround

 arch/powerpc/include/asm/ftrace.h |   7 --
 arch/x86/include/asm/ftrace.h     |   7 --
 include/linux/module.h            |  14 +++
 kernel/module/kallsyms.c          |  42 ++++++--
 kernel/trace/ftrace.c             | 171 ++++++------------------------
 scripts/kallsyms.c                |  95 ++++++++++++++++-
 scripts/link-vmlinux.sh           |   4 +-
 scripts/mksysmap                  |   2 +-
 8 files changed, 173 insertions(+), 169 deletions(-)

-- 
2.25.1

[PATCH v2 1/5] kallsyms: Emit symbol at the holes in the text

[Zheng Yejian]

When a weak type function is overridden, its symbol will be removed
from the symbol table, but its code will not be removed. Besides,
due to lacking of size for kallsyms, kernel compute function size by
substracting its symbol address from its next symbol address (see
kallsyms_lookup_size_offset()). These will cause that size of some
function is computed to be larger than it actually is, just because
symbol of its following weak function is removed.

This issue also causes multiple __fentry__ locations to be counted in
the same function scope, and eventually causes ftrace_location() to find
wrong __fentry__ location. It was reported in
Link: https://lore.kernel.org/all/20240607115211.734845-1-zhengyejian1@huawei.com/

Peter suggested to change scipts/kallsyms.c to emit readily
identifiable symbol names for all the weak junk.

So in this patch:
1. Pass size info to scripts/kallsyms  (see mksysmap());
2. Traverse sorted function symbols, if one function address plus its
   size less than next function address, it means there's a hole, with
   Masahiro's suggestion, then emit a symbol there of which type and
   name are both empty to represent the hole.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Zheng Yejian <zhengyejian@huaweicloud.com>
---
 scripts/kallsyms.c      | 94 +++++++++++++++++++++++++++++++++++++++--
 scripts/link-vmlinux.sh |  4 +-
 scripts/mksysmap        |  2 +-
 3 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 47978efe4797..cf64c20a8292 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -34,6 +34,7 @@
 
 struct sym_entry {
 	unsigned long long addr;
+	unsigned long long size;
 	unsigned int len;
 	unsigned int seq;
 	unsigned int start_pos;
@@ -72,7 +73,6 @@ static int token_profit[0x10000];
 static unsigned char best_table[256][2];
 static unsigned char best_table_len[256];
 
-
 static void usage(void)
 {
 	fprintf(stderr, "Usage: kallsyms [--all-symbols] [--absolute-percpu] "
@@ -128,8 +128,16 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 	size_t len;
 	ssize_t readlen;
 	struct sym_entry *sym;
+	unsigned long long size = 0;
 
 	errno = 0;
+	/*
+	 * Example of expected symbol format:
+	 * 1. symbol with size info:
+	 *    ffffffff81000070 00000000000001d7 T __startup_64
+	 * 2. symbol without size info:
+	 *    0000000002a00000 A text_size
+	 */
 	readlen = getline(buf, buf_len, in);
 	if (readlen < 0) {
 		if (errno) {
@@ -143,9 +151,24 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 		(*buf)[readlen - 1] = 0;
 
 	addr = strtoull(*buf, &p, 16);
+	if (*buf == p || *p++ != ' ') {
+		fprintf(stderr, "line format error: unable to parse address\n");
+		exit(EXIT_FAILURE);
+	}
+
+	if (*p == '0') {
+		char *str = p;
+
+		size = strtoull(str, &p, 16);
+		if (str == p || *p++ != ' ') {
+			fprintf(stderr, "line format error: unable to parse size\n");
+			exit(EXIT_FAILURE);
+		}
+	}
 
-	if (*buf == p || *p++ != ' ' || !isascii((type = *p++)) || *p++ != ' ') {
-		fprintf(stderr, "line format error\n");
+	type = *p++;
+	if (!isascii(type) || *p++ != ' ') {
+		fprintf(stderr, "line format error: unable to parse type\n");
 		exit(EXIT_FAILURE);
 	}
 
@@ -180,6 +203,7 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 		exit(EXIT_FAILURE);
 	}
 	sym->addr = addr;
+	sym->size = size;
 	sym->len = len;
 	sym->sym[0] = type;
 	strcpy(sym_name(sym), name);
@@ -788,6 +812,69 @@ static void sort_symbols(void)
 	qsort(table, table_cnt, sizeof(table[0]), compare_symbols);
 }
 
+static bool has_hole(const struct sym_entry *se1, const struct sym_entry *se2)
+{
+	char type = se1->sym[0];
+
+	/* Only check text symbol or weak symbol */
+	if (type != 't' && type != 'T' &&
+	    type != 'w' && type != 'W')
+		return 0;
+	/* Symbol without size has no hole */
+	if (!se1->size)
+		return 0;
+	return se1->addr + se1->size < se2->addr;
+}
+
+static struct sym_entry *gen_hole_symbol(const struct sym_entry *se)
+{
+	struct sym_entry *sym;
+
+	/* Use empty symbol type/name as a special case to represent the hole */
+	sym = malloc(sizeof(*sym) + 1);
+	if (!sym) {
+		fprintf(stderr, "unable to allocate memory for hole symbol\n");
+		exit(EXIT_FAILURE);
+	}
+	sym->addr = se->addr + se->size;
+	sym->size = 0;
+	sym->len = 1;
+	sym->sym[0] = '\0';
+	sym->percpu_absolute = 0;
+	return sym;
+}
+
+static void emit_hole_symbols(void)
+{
+	unsigned int i, j, nr_emit;
+	unsigned int new_cnt;
+
+	nr_emit = 0;
+	for (i = 0; i < table_cnt - 1; i++) {
+		if (has_hole(table[i], table[i+1]))
+			nr_emit++;
+	}
+	if (!nr_emit)
+		return;
+
+	new_cnt = table_cnt + nr_emit;
+	table = realloc(table, sizeof(*table) * new_cnt);
+	if (!table) {
+		fprintf(stderr, "unable to allocate memory for emitting hole symbols\n");
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = table_cnt - 1, j = new_cnt - 1; i >= 0; i--, j--) {
+		if ((i != table_cnt - 1) && has_hole(table[i], table[i+1]))
+			table[j--] = gen_hole_symbol(table[i]);
+		if (j != i)
+			table[j] = table[i];
+		else
+			break;
+	}
+	table_cnt = new_cnt;
+}
+
 static void make_percpus_absolute(void)
 {
 	unsigned int i;
@@ -847,6 +934,7 @@ int main(int argc, char **argv)
 	if (absolute_percpu)
 		make_percpus_absolute();
 	sort_symbols();
+	emit_hole_symbols();
 	if (base_relative)
 		record_relative_base();
 	optimize_token_table();
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 518c70b8db50..8e1373902bfe 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -189,11 +189,11 @@ kallsyms_step()
 }
 
 # Create map file with all symbols from ${1}
-# See mksymap for additional details
+# See mksysmap for additional details
 mksysmap()
 {
 	info NM ${2}
-	${NM} -n "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}"
+	${NM} -nS "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}"
 }
 
 sorttable()
diff --git a/scripts/mksysmap b/scripts/mksysmap
index c12723a04655..7a4415f21143 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 #
 # sed script to filter out symbols that are not needed for System.map,
-# or not suitable for kallsyms. The input should be 'nm -n <file>'.
+# or not suitable for kallsyms. The input should be 'nm -nS <file>'.
 #
 # System.map is used by module-init tools and some debugging
 # tools to retrieve the actual addresses of symbols in the kernel.
-- 
2.25.1

[PATCH v2 2/5] module: kallsyms: Determine exact function size

[Zheng Yejian]

When a weak type function is overridden, its symbol will be removed
from the symbol table, but its code will not been removed. It will
cause find_kallsyms_symbol() to compute a larger function size than
it actually is, just because symbol of its following weak function is
removed.

To fix this issue, check that given address is within the size of
the function found.

Signed-off-by: Zheng Yejian <zhengyejian@huaweicloud.com>
---
 include/linux/module.h   |  7 +++++++
 kernel/module/kallsyms.c | 19 +++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 4213d8993cd8..0299d79433ae 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -599,6 +599,13 @@ static inline unsigned long kallsyms_symbol_value(const Elf_Sym *sym)
 }
 #endif
 
+#ifndef HAVE_ARCH_KALLSYMS_SYMBOL_TYPE
+static inline unsigned int kallsyms_symbol_type(const Elf_Sym *sym)
+{
+	return ELF_ST_TYPE(sym->st_info);
+}
+#endif
+
 /* FIXME: It'd be nice to isolate modules during init, too, so they
    aren't used before they (may) fail.  But presently too much code
    (IDE & SCSI) require entry into the module during init.*/
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index bf65e0c3c86f..cce4f81b9933 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -262,6 +262,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
 	unsigned long nextval, bestval;
 	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
 	struct module_memory *mod_mem;
+	const Elf_Sym *sym;
 
 	/* At worse, next value is at end of module */
 	if (within_module_init(addr, mod))
@@ -278,9 +279,10 @@ static const char *find_kallsyms_symbol(struct module *mod,
 	 * starts real symbols at 1).
 	 */
 	for (i = 1; i < kallsyms->num_symtab; i++) {
-		const Elf_Sym *sym = &kallsyms->symtab[i];
-		unsigned long thisval = kallsyms_symbol_value(sym);
+		unsigned long thisval;
 
+		sym = &kallsyms->symtab[i];
+		thisval = kallsyms_symbol_value(sym);
 		if (sym->st_shndx == SHN_UNDEF)
 			continue;
 
@@ -292,6 +294,13 @@ static const char *find_kallsyms_symbol(struct module *mod,
 		    is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
 			continue;
 
+		if (kallsyms_symbol_type(sym) == STT_FUNC &&
+		    addr >= thisval && addr < thisval + sym->st_size) {
+			best = i;
+			bestval = thisval;
+			nextval = thisval + sym->st_size;
+			goto found;
+		}
 		if (thisval <= addr && thisval > bestval) {
 			best = i;
 			bestval = thisval;
@@ -303,6 +312,12 @@ static const char *find_kallsyms_symbol(struct module *mod,
 	if (!best)
 		return NULL;
 
+	sym = &kallsyms->symtab[best];
+	if (kallsyms_symbol_type(sym) == STT_FUNC && sym->st_size &&
+	    addr >= kallsyms_symbol_value(sym) + sym->st_size)
+		return NULL;
+
+found:
 	if (size)
 		*size = nextval - bestval;
 	if (offset)
-- 
2.25.1

[PATCH v2 3/5] ftrace: Skip invalid __fentry__ in ftrace_process_locs()

[Zheng Yejian]

ftrace_location() was changed to not only return the __fentry__ location
when called for the __fentry__ location, but also when called for the
sym+0 location after commit aebfd12521d9 ("x86/ibt,ftrace: Search for
__fentry__ location"). That is, if sym+0 location is not __fentry__,
ftrace_location() would find one over the entire size of the sym.

However, there is case that more than one __fentry__ exist in the sym
range (described below) and ftrace_location() would find wrong __fentry__
location by binary searching, which would cause its users like livepatch/
kprobe/bpf to not work properly on this sym!

The case is that, based on current compiler behavior, suppose:
 - function A is followed by weak function B1 in same binary file;
 - weak function B1 is overridden by function B2;
Then in the final binary file:
 - symbol B1 will be removed from symbol table while its instructions are
   not removed;
 - __fentry__ of B1 will be still in __mcount_loc table;
 - function size of A is computed by substracting the symbol address of
   A from its next symbol address (see kallsyms_lookup_size_offset()),
   but because symbol info of B1 is removed, the next symbol of A is
   originally the next symbol of B1. See following example, function
   sizeof A will be (symbol_address_C - symbol_address_A):

     symbol_address_A
     symbol_address_B1 (Not in symbol table)
     symbol_address_C

The weak function issue has been discovered in commit b39181f7c690
("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function")
but it didn't resolve the issue in ftrace_location().

To solve the issue, with Peter's suggestions, in previous patches, all
holes in the text have been found and filled with specail symbols, also
the same case with module weak function has been handled. Then check and
skip __fentry__ that locate in the holes.

Also in this patch, introduce module_kallsyms_find_symbol() to check if
a __fentry__ locate in a valid function of the given module. It is needed
because other symbol lookup functions like module_address_lookup() will
find module of the passed address first, but as ftrace_process_locs() is
called, the module has not been fully loaded, so those lookup functions
can not work.

Fixes: aebfd12521d9 ("x86/ibt,ftrace: Search for __fentry__ location")
Signed-off-by: Zheng Yejian <zhengyejian@huaweicloud.com>
---
 include/linux/module.h   |  7 +++++++
 kernel/module/kallsyms.c | 23 +++++++++++++++++------
 kernel/trace/ftrace.c    | 12 +++++++++++-
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 0299d79433ae..4f5dd018e33d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -962,6 +962,8 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 unsigned long module_kallsyms_lookup_name(const char *name);
 
 unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name);
+int module_kallsyms_find_symbol(struct module *mod, unsigned long addr,
+				unsigned long *size, unsigned long *offset);
 
 #else	/* CONFIG_MODULES && CONFIG_KALLSYMS */
 
@@ -1006,6 +1008,11 @@ static inline unsigned long find_kallsyms_symbol_value(struct module *mod,
 	return 0;
 }
 
+static inline int module_kallsyms_find_symbol(struct module *mod, unsigned long addr,
+					      unsigned long *size, unsigned long *offset)
+{
+	return 0;
+}
 #endif  /* CONFIG_MODULES && CONFIG_KALLSYMS */
 
 #endif /* _LINUX_MODULE_H */
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index cce4f81b9933..71b3ed25cd40 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -253,10 +253,10 @@ static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned
  * Given a module and address, find the corresponding symbol and return its name
  * while providing its size and offset if needed.
  */
-static const char *find_kallsyms_symbol(struct module *mod,
-					unsigned long addr,
-					unsigned long *size,
-					unsigned long *offset)
+static const char *__find_kallsyms_symbol(struct module *mod,
+					  unsigned long addr,
+					  unsigned long *size,
+					  unsigned long *offset)
 {
 	unsigned int i, best = 0;
 	unsigned long nextval, bestval;
@@ -326,6 +326,17 @@ static const char *find_kallsyms_symbol(struct module *mod,
 	return kallsyms_symbol_name(kallsyms, best);
 }
 
+int module_kallsyms_find_symbol(struct module *mod, unsigned long addr,
+				unsigned long *size, unsigned long *offset)
+{
+	const char *ret;
+
+	preempt_disable();
+	ret = __find_kallsyms_symbol(mod, addr, size, offset);
+	preempt_enable();
+	return !!ret;
+}
+
 void * __weak dereference_module_function_descriptor(struct module *mod,
 						     void *ptr)
 {
@@ -360,7 +371,7 @@ int module_address_lookup(unsigned long addr,
 #endif
 		}
 
-		sym = find_kallsyms_symbol(mod, addr, size, offset);
+		sym = __find_kallsyms_symbol(mod, addr, size, offset);
 
 		if (sym)
 			ret = strscpy(namebuf, sym, KSYM_NAME_LEN);
@@ -381,7 +392,7 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)
 		if (within_module(addr, mod)) {
 			const char *sym;
 
-			sym = find_kallsyms_symbol(mod, addr, NULL, NULL);
+			sym = __find_kallsyms_symbol(mod, addr, NULL, NULL);
 			if (!sym)
 				goto out;
 
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0f579430f02a..fff5d3466c41 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6989,6 +6989,16 @@ static void test_is_sorted(unsigned long *start, unsigned long count)
 }
 #endif
 
+static int is_invalid_rec(struct module *mod, unsigned long addr)
+{
+	char str[KSYM_SYMBOL_LEN];
+
+	if (mod)
+		return !module_kallsyms_find_symbol(mod, addr, NULL, NULL);
+
+	return !kallsyms_lookup(addr, NULL, NULL, NULL, str);
+}
+
 static int ftrace_process_locs(struct module *mod,
 			       unsigned long *start,
 			       unsigned long *end)
@@ -7060,7 +7070,7 @@ static int ftrace_process_locs(struct module *mod,
 		 * object files to satisfy alignments.
 		 * Skip any NULL pointers.
 		 */
-		if (!addr) {
+		if (!addr || is_invalid_rec(mod, addr)) {
 			skipped++;
 			continue;
 		}
-- 
2.25.1

[PATCH v2 4/5] ftrace: Fix possible out-of-bound issue in ftrace_process_locs()

[Zheng Yejian]

In ftrace_process_locs(), a series pages are prepared and linked in
start_pg, then fentry records are skipped or added, then unused pages
are freed.

However, assume that all records are skipped, currently the start_pg
will still be in list of ftrace_pages_start but without any record.
Then in ftrace_free_mem() index record by (pg->index - 1) will be out
of bound.

To fix this issue, properly handle with unused start_pg and add
WARN_ON_ONCE() where the records need to be indexed.

Fixes: 26efd79c4624 ("ftrace: Fix possible warning on checking all pages used in ftrace_process_locs()")
Signed-off-by: Zheng Yejian <zhengyejian@huaweicloud.com>
---
 kernel/trace/ftrace.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index fff5d3466c41..6947be8801d9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7087,10 +7087,22 @@ static int ftrace_process_locs(struct module *mod,
 		rec->ip = addr;
 	}
 
-	if (pg->next) {
+	if (pg->index == 0) {
+		/* No record is added on the current page, so it's unused */
+		pg_unuse = pg;
+	} else if (pg->next) {
+		/* Current page has records, so it's next page is unused */
 		pg_unuse = pg->next;
 		pg->next = NULL;
 	}
+	/*
+	 * Even the start_pg hasn't been used, that means, no record has
+	 * been added, so restore state of ftrace_pages and just go out.
+	 */
+	if (pg_unuse == start_pg) {
+		ftrace_pages->next = NULL;
+		goto out;
+	}
 
 	/* Assign the last page to ftrace_pages */
 	ftrace_pages = pg;
@@ -7306,6 +7318,8 @@ void ftrace_release_mod(struct module *mod)
 	 */
 	last_pg = &ftrace_pages_start;
 	for (pg = ftrace_pages_start; pg; pg = *last_pg) {
+		/* The page should have at lease one record */
+		WARN_ON_ONCE(!pg->index);
 		rec = &pg->records[0];
 		if (within_module(rec->ip, mod)) {
 			/*
@@ -7685,6 +7699,8 @@ void ftrace_free_mem(struct module *mod, void *start_ptr, void *end_ptr)
 		mod_map = allocate_ftrace_mod_map(mod, start, end);
 
 	for (pg = ftrace_pages_start; pg; last_pg = &pg->next, pg = *last_pg) {
+		/* The page should have at lease one record */
+		WARN_ON_ONCE(!pg->index);
 		if (end < pg->records[0].ip ||
 		    start >= (pg->records[pg->index - 1].ip + MCOUNT_INSN_SIZE))
 			continue;
-- 
2.25.1

[PATCH v2 5/5] ftrace: Revert the FTRACE_MCOUNT_MAX_OFFSET workaround

[Zheng Yejian]

After patch titled "ftrace: Skip invalid __fentry__ in
ftrace_process_locs()", __fentry__ locations in overridden weak function
have been checked and skipped, then all records in ftrace_pages are
valid, the FTRACE_MCOUNT_MAX_OFFSET workaround can be reverted, include:
 1. commit b39181f7c690 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid
    adding weak function")
 2. commit 7af82ff90a2b ("powerpc/ftrace: Ignore weak functions")
 3. commit f6834c8c59a8 ("powerpc/ftrace: Fix dropping weak symbols with
    older toolchains")

Signed-off-by: Zheng Yejian <zhengyejian@huaweicloud.com>
---
 arch/powerpc/include/asm/ftrace.h |   7 --
 arch/x86/include/asm/ftrace.h     |   7 --
 kernel/trace/ftrace.c             | 141 +-----------------------------
 3 files changed, 2 insertions(+), 153 deletions(-)

diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 559560286e6d..328cf55acfb7 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -8,13 +8,6 @@
 #define MCOUNT_ADDR		((unsigned long)(_mcount))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
 
-/* Ignore unused weak functions which will have larger offsets */
-#if defined(CONFIG_MPROFILE_KERNEL) || defined(CONFIG_ARCH_USING_PATCHABLE_FUNCTION_ENTRY)
-#define FTRACE_MCOUNT_MAX_OFFSET	16
-#elif defined(CONFIG_PPC32)
-#define FTRACE_MCOUNT_MAX_OFFSET	8
-#endif
-
 #ifndef __ASSEMBLY__
 extern void _mcount(void);
 
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 0152a81d9b4a..6a3a4a8830dc 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -9,13 +9,6 @@
 # define MCOUNT_ADDR		((unsigned long)(__fentry__))
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
-/* Ignore unused weak functions which will have non zero offsets */
-#ifdef CONFIG_HAVE_FENTRY
-# include <asm/ibt.h>
-/* Add offset for endbr64 if IBT enabled */
-# define FTRACE_MCOUNT_MAX_OFFSET	ENDBR_INSN_SIZE
-#endif
-
 #ifdef CONFIG_DYNAMIC_FTRACE
 #define ARCH_SUPPORTS_FTRACE_OPS 1
 #endif
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6947be8801d9..37510c591498 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -49,8 +49,6 @@
 #define FTRACE_NOCLEAR_FLAGS	(FTRACE_FL_DISABLED | FTRACE_FL_TOUCHED | \
 				 FTRACE_FL_MODIFIED)
 
-#define FTRACE_INVALID_FUNCTION		"__ftrace_invalid_address__"
-
 #define FTRACE_WARN_ON(cond)			\
 	({					\
 		int ___r = cond;		\
@@ -4208,105 +4206,6 @@ static void add_trampoline_func(struct seq_file *m, struct ftrace_ops *ops,
 		seq_printf(m, " ->%pS", ptr);
 }
 
-#ifdef FTRACE_MCOUNT_MAX_OFFSET
-/*
- * Weak functions can still have an mcount/fentry that is saved in
- * the __mcount_loc section. These can be detected by having a
- * symbol offset of greater than FTRACE_MCOUNT_MAX_OFFSET, as the
- * symbol found by kallsyms is not the function that the mcount/fentry
- * is part of. The offset is much greater in these cases.
- *
- * Test the record to make sure that the ip points to a valid kallsyms
- * and if not, mark it disabled.
- */
-static int test_for_valid_rec(struct dyn_ftrace *rec)
-{
-	char str[KSYM_SYMBOL_LEN];
-	unsigned long offset;
-	const char *ret;
-
-	ret = kallsyms_lookup(rec->ip, NULL, &offset, NULL, str);
-
-	/* Weak functions can cause invalid addresses */
-	if (!ret || offset > FTRACE_MCOUNT_MAX_OFFSET) {
-		rec->flags |= FTRACE_FL_DISABLED;
-		return 0;
-	}
-	return 1;
-}
-
-static struct workqueue_struct *ftrace_check_wq __initdata;
-static struct work_struct ftrace_check_work __initdata;
-
-/*
- * Scan all the mcount/fentry entries to make sure they are valid.
- */
-static __init void ftrace_check_work_func(struct work_struct *work)
-{
-	struct ftrace_page *pg;
-	struct dyn_ftrace *rec;
-
-	mutex_lock(&ftrace_lock);
-	do_for_each_ftrace_rec(pg, rec) {
-		test_for_valid_rec(rec);
-	} while_for_each_ftrace_rec();
-	mutex_unlock(&ftrace_lock);
-}
-
-static int __init ftrace_check_for_weak_functions(void)
-{
-	INIT_WORK(&ftrace_check_work, ftrace_check_work_func);
-
-	ftrace_check_wq = alloc_workqueue("ftrace_check_wq", WQ_UNBOUND, 0);
-
-	queue_work(ftrace_check_wq, &ftrace_check_work);
-	return 0;
-}
-
-static int __init ftrace_check_sync(void)
-{
-	/* Make sure the ftrace_check updates are finished */
-	if (ftrace_check_wq)
-		destroy_workqueue(ftrace_check_wq);
-	return 0;
-}
-
-late_initcall_sync(ftrace_check_sync);
-subsys_initcall(ftrace_check_for_weak_functions);
-
-static int print_rec(struct seq_file *m, unsigned long ip)
-{
-	unsigned long offset;
-	char str[KSYM_SYMBOL_LEN];
-	char *modname;
-	const char *ret;
-
-	ret = kallsyms_lookup(ip, NULL, &offset, &modname, str);
-	/* Weak functions can cause invalid addresses */
-	if (!ret || offset > FTRACE_MCOUNT_MAX_OFFSET) {
-		snprintf(str, KSYM_SYMBOL_LEN, "%s_%ld",
-			 FTRACE_INVALID_FUNCTION, offset);
-		ret = NULL;
-	}
-
-	seq_puts(m, str);
-	if (modname)
-		seq_printf(m, " [%s]", modname);
-	return ret == NULL ? -1 : 0;
-}
-#else
-static inline int test_for_valid_rec(struct dyn_ftrace *rec)
-{
-	return 1;
-}
-
-static inline int print_rec(struct seq_file *m, unsigned long ip)
-{
-	seq_printf(m, "%ps", (void *)ip);
-	return 0;
-}
-#endif
-
 static int t_show(struct seq_file *m, void *v)
 {
 	struct ftrace_iterator *iter = m->private;
@@ -4334,13 +4233,7 @@ static int t_show(struct seq_file *m, void *v)
 	if (iter->flags & FTRACE_ITER_ADDRS)
 		seq_printf(m, "%lx ", rec->ip);
 
-	if (print_rec(m, rec->ip)) {
-		/* This should only happen when a rec is disabled */
-		WARN_ON_ONCE(!(rec->flags & FTRACE_FL_DISABLED));
-		seq_putc(m, '\n');
-		return 0;
-	}
-
+	seq_printf(m, "%ps", (void *)rec->ip);
 	if (iter->flags & (FTRACE_ITER_ENABLED | FTRACE_ITER_TOUCHED)) {
 		struct ftrace_ops *ops;
 
@@ -4720,24 +4613,6 @@ add_rec_by_index(struct ftrace_hash *hash, struct ftrace_glob *func_g,
 	return 0;
 }
 
-#ifdef FTRACE_MCOUNT_MAX_OFFSET
-static int lookup_ip(unsigned long ip, char **modname, char *str)
-{
-	unsigned long offset;
-
-	kallsyms_lookup(ip, NULL, &offset, modname, str);
-	if (offset > FTRACE_MCOUNT_MAX_OFFSET)
-		return -1;
-	return 0;
-}
-#else
-static int lookup_ip(unsigned long ip, char **modname, char *str)
-{
-	kallsyms_lookup(ip, NULL, NULL, modname, str);
-	return 0;
-}
-#endif
-
 static int
 ftrace_match_record(struct dyn_ftrace *rec, struct ftrace_glob *func_g,
 		struct ftrace_glob *mod_g, int exclude_mod)
@@ -4745,12 +4620,7 @@ ftrace_match_record(struct dyn_ftrace *rec, struct ftrace_glob *func_g,
 	char str[KSYM_SYMBOL_LEN];
 	char *modname;
 
-	if (lookup_ip(rec->ip, &modname, str)) {
-		/* This should only happen when a rec is disabled */
-		WARN_ON_ONCE(system_state == SYSTEM_RUNNING &&
-			     !(rec->flags & FTRACE_FL_DISABLED));
-		return 0;
-	}
+	kallsyms_lookup(rec->ip, NULL, NULL, &modname, str);
 
 	if (mod_g) {
 		int mod_matches = (modname) ? ftrace_match(modname, mod_g) : 0;
@@ -7399,13 +7269,6 @@ void ftrace_module_enable(struct module *mod)
 		if (!within_module(rec->ip, mod))
 			break;
 
-		/* Weak functions should still be ignored */
-		if (!test_for_valid_rec(rec)) {
-			/* Clear all other flags. Should not be enabled anyway */
-			rec->flags = FTRACE_FL_DISABLED;
-			continue;
-		}
-
 		cnt = 0;
 
 		/*
-- 
2.25.1

Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Martin Kelly]

On Tue, 2024-07-23 at 14:32 +0800, Zheng Yejian wrote:
> Background of this patch set can be found in v1:
> 
> https://lore.kernel.org/all/20240613133711.2867745-1-zhengyejian1@huawei.com/
>  
> 
> Here add a reproduction to show the impact to livepatch:
> 1. Add following hack to make livepatch-sample.ko do patch on
> do_one_initcall()
>    which has an overriden weak function behind in vmlinux, then print
> the
>    actually used __fentry__ location:
> 

Hi all, what is the status of this patch series? I'd really like to see
it or some other fix to this issue merged. The underlying bug is a
significant one that can cause ftrace/livepatch/BPF fentry to fail
silently. I've noticed this bug in another context[1] and realized
they're the same issue.

I'm happy to help with this patch series to address any issues as
needed.

[1]
https://lore.kernel.org/bpf/7136605d24de9b1fc62d02a355ef11c950a94153.camel@crowdstrike.com/T/#mb7e6f84ac90fa78989e9e2c3cd8d29f65a78845b

Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Christophe Leroy]

Hi,

Le 10/12/2024 à 20:15, Martin Kelly a écrit :
> [Vous ne recevez pas souvent de courriers de martin.kelly@crowdstrike.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Tue, 2024-07-23 at 14:32 +0800, Zheng Yejian wrote:
>> Background of this patch set can be found in v1:
>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20240613133711.2867745-1-zhengyejian1%40huawei.com%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cbc4f27151ef04b74fba608dd194f0034%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638694550404456289%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C80000%7C%7C%7C&sdata=a5XFKy9qxVrM5yXuvJuilJ%2FsUxU4j326MOmEz7dBViY%3D&reserved=0
>>
>>
>> Here add a reproduction to show the impact to livepatch:
>> 1. Add following hack to make livepatch-sample.ko do patch on
>> do_one_initcall()
>>     which has an overriden weak function behind in vmlinux, then print
>> the
>>     actually used __fentry__ location:
>>
> 
> Hi all, what is the status of this patch series? I'd really like to see
> it or some other fix to this issue merged. The underlying bug is a
> significant one that can cause ftrace/livepatch/BPF fentry to fail
> silently. I've noticed this bug in another context[1] and realized
> they're the same issue.
> 
> I'm happy to help with this patch series to address any issues as
> needed.

As far as I can see there are problems on build with patch 1, see 
https://patchwork.kernel.org/project/linux-modules/patch/20240723063258.2240610-2-zhengyejian@huaweicloud.com/

> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fbpf%2F7136605d24de9b1fc62d02a355ef11c950a94153.camel%40crowdstrike.com%2FT%2F%23mb7e6f84ac90fa78989e9e2c3cd8d29f65a78845b&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cbc4f27151ef04b74fba608dd194f0034%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638694550404477455%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C80000%7C%7C%7C&sdata=v9qPnj%2FDDWAuSdB6dP19nyxUWijxveymI6mQb63KxbY%3D&reserved=0

Christophe

Re: Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Martin Kelly]

On Tue, 2024-12-10 at 21:01 +0100, Christophe Leroy wrote:
> > 
> > Hi all, what is the status of this patch series? I'd really like to
> > see
> > it or some other fix to this issue merged. The underlying bug is a
> > significant one that can cause ftrace/livepatch/BPF fentry to fail
> > silently. I've noticed this bug in another context[1] and realized
> > they're the same issue.
> > 
> > I'm happy to help with this patch series to address any issues as
> > needed.
> 
> As far as I can see there are problems on build with patch 1, see 
> https://patchwork.kernel.org/project/linux-modules/patch/20240723063258.2240610-2-zhengyejian@huaweicloud.com/
>  
> 
> 

Yeah, I see those. Additionally, this patch no longer applies cleanly
to current master, though fixing it up to do so is pretty easy. Having
done that, this series appears to resolve the issues I saw in the other
linked thread.

Zheng, do you plan to send a v3? I'd be happy to help out with this
patch series if you'd like, as I'm hoping to get this issue resolved
(though I am not an ftrace expert).

Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Zheng Yejian]

On 2024/12/11 04:49, Martin Kelly wrote:
> On Tue, 2024-12-10 at 21:01 +0100, Christophe Leroy wrote:
>>>
>>> Hi all, what is the status of this patch series? I'd really like to
>>> see
>>> it or some other fix to this issue merged. The underlying bug is a
>>> significant one that can cause ftrace/livepatch/BPF fentry to fail
>>> silently. I've noticed this bug in another context[1] and realized
>>> they're the same issue.
>>>
>>> I'm happy to help with this patch series to address any issues as
>>> needed.
>>
>> As far as I can see there are problems on build with patch 1, see
>> https://patchwork.kernel.org/project/linux-modules/patch/20240723063258.2240610-2-zhengyejian@huaweicloud.com/
>>   
>>
>>
> 
> Yeah, I see those. Additionally, this patch no longer applies cleanly
> to current master, though fixing it up to do so is pretty easy. Having
> done that, this series appears to resolve the issues I saw in the other
> linked thread.
> 
> Zheng, do you plan to send a v3? I'd be happy to help out with this
> patch series if you'd like, as I'm hoping to get this issue resolved
> (though I am not an ftrace expert).

Sorry to rely so late. Thanks for your feedback!

Steve recently started a discussion of the issue in:
https://lore.kernel.org/all/20241014210841.5a4764de@gandalf.local.home/
but there's no conclusion.
  
I can rebase this patch series and send a new version first, and
I'm also hoping to get more feedbacks and help to resolve the issue.

-- 
Thanks,
Zheng Yejian

Re: Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Martin Kelly]

On Thu, 2024-12-12 at 17:52 +0800, Zheng Yejian wrote:
> On 2024/12/11 04:49, Martin Kelly wrote:
> > 
> > 
> > Zheng, do you plan to send a v3? I'd be happy to help out with this
> > patch series if you'd like, as I'm hoping to get this issue
> > resolved
> > (though I am not an ftrace expert).
> 
> Sorry to rely so late. Thanks for your feedback!
> 
> Steve recently started a discussion of the issue in:
> 
> https://lore.kernel.org/all/20241015100111.37adfb28@gandalf.local.home/
> but there's no conclusion.
>   
> I can rebase this patch series and send a new version first, and
> I'm also hoping to get more feedbacks and help to resolve the issue.
> 

Hi Zheng,

I may have misunderstood, but based on the final message from Steven, I
got the sense that the idea listed in that thread didn't work out and
we should proceed with your current approach.

Please consider me an interested party for this patch series, and let
me know if there's anything I can do to help speed it along (co-
develop, test, anything else). And of course, thanks very much for your
work thus far!

Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Zheng Yejian]

On 2024/12/14 03:31, Martin Kelly wrote:
> On Thu, 2024-12-12 at 17:52 +0800, Zheng Yejian wrote:
>> On 2024/12/11 04:49, Martin Kelly wrote:
>>>
>>>
>>> Zheng, do you plan to send a v3? I'd be happy to help out with this
>>> patch series if you'd like, as I'm hoping to get this issue
>>> resolved
>>> (though I am not an ftrace expert).
>>
>> Sorry to rely so late. Thanks for your feedback!
>>
>> Steve recently started a discussion of the issue in:
>>
>> https://lore.kernel.org/all/20241015100111.37adfb28@gandalf.local.home/
>> but there's no conclusion.
>>    
>> I can rebase this patch series and send a new version first, and
>> I'm also hoping to get more feedbacks and help to resolve the issue.
>>
> 
> Hi Zheng,
> 
> I may have misunderstood, but based on the final message from Steven, I
> got the sense that the idea listed in that thread didn't work out and
> we should proceed with your current approach.

This patch set attempts to add length information of symbols to kallsyms,
which can then ensure that there is only one valid fentry within the range
of function length.

However, I think this patch set actually has some performance implications,
including：
   1. Adding hole symbols may cause the symbol table to grow significantly;
   2. When parsing fentry tables, excluding invalid fentries through kallsyms lookups
      may also increase boot or module load times slightly.

The direct cause of this issue is the wrong fentry being founded by ftrace_location(),
following the approach of "FTRACE_MCOUNT_MAX_OFFSET", narrowing down the search range
and re-finding may also solve this problem, demo patch like below (not
fully tested):

     diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
     index 9b17efb1a87d..7d34320ca9d1 100644
     --- a/kernel/trace/ftrace.c
     +++ b/kernel/trace/ftrace.c
     @@ -1678,8 +1678,11 @@ unsigned long ftrace_location(unsigned long ip)
                             goto out;
     
                     /* map sym+0 to __fentry__ */
     -               if (!offset)
     +               if (!offset) {
                             loc = ftrace_location_range(ip, ip + size - 1);
     +                       while (loc > ip && loc - ip > FTRACE_MCOUNT_MAX_OFFSET)
     +                               loc = ftrace_location_range(ip, loc - 1);
     +               }
             }

Steve, Peter, what do you think?

> 
> Please consider me an interested party for this patch series, and let
> me know if there's anything I can do to help speed it along (co-
> develop, test, anything else). And of course, thanks very much for your
> work thus far!

-- 
Thanks,
Zheng Yejian

Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Steven Rostedt]

Sorry for the late reply. Forgot about this as I was focused on other end-of-year issues.

On Sat, 14 Dec 2024 16:37:59 +0800
Zheng Yejian <zhengyejian1@huawei.com> wrote:

> The direct cause of this issue is the wrong fentry being founded by ftrace_location(),
> following the approach of "FTRACE_MCOUNT_MAX_OFFSET", narrowing down the search range
> and re-finding may also solve this problem, demo patch like below (not
> fully tested):
> 
>      diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>      index 9b17efb1a87d..7d34320ca9d1 100644
>      --- a/kernel/trace/ftrace.c
>      +++ b/kernel/trace/ftrace.c
>      @@ -1678,8 +1678,11 @@ unsigned long ftrace_location(unsigned long ip)
>                              goto out;
>      
>                      /* map sym+0 to __fentry__ */
>      -               if (!offset)
>      +               if (!offset) {
>                              loc = ftrace_location_range(ip, ip + size - 1);
>      +                       while (loc > ip && loc - ip > FTRACE_MCOUNT_MAX_OFFSET)
>      +                               loc = ftrace_location_range(ip, loc - 1);
>      +               }
>              }
> 
> Steve, Peter, what do you think?

Hmm, removing the weak functions from the __mcount_loc location should also
solve this, as the ftrace_location_range() will not return a weak function
if it's not part of the __mcount_loc table.

That is, would this patchset work?

  https://lore.kernel.org/all/20250102232609.529842248@goodmis.org/

-- Steve

Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Zheng Yejian]

On 2025/1/22 01:48, Steven Rostedt wrote:
> 
> Sorry for the late reply. Forgot about this as I was focused on other end-of-year issues.
> 
> On Sat, 14 Dec 2024 16:37:59 +0800
> Zheng Yejian <zhengyejian1@huawei.com> wrote:
> 
>> The direct cause of this issue is the wrong fentry being founded by ftrace_location(),
>> following the approach of "FTRACE_MCOUNT_MAX_OFFSET", narrowing down the search range
>> and re-finding may also solve this problem, demo patch like below (not
>> fully tested):
>>
>>       diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>>       index 9b17efb1a87d..7d34320ca9d1 100644
>>       --- a/kernel/trace/ftrace.c
>>       +++ b/kernel/trace/ftrace.c
>>       @@ -1678,8 +1678,11 @@ unsigned long ftrace_location(unsigned long ip)
>>                               goto out;
>>       
>>                       /* map sym+0 to __fentry__ */
>>       -               if (!offset)
>>       +               if (!offset) {
>>                               loc = ftrace_location_range(ip, ip + size - 1);
>>       +                       while (loc > ip && loc - ip > FTRACE_MCOUNT_MAX_OFFSET)
>>       +                               loc = ftrace_location_range(ip, loc - 1);
>>       +               }
>>               }
>>
>> Steve, Peter, what do you think?
> 
> Hmm, removing the weak functions from the __mcount_loc location should also
> solve this, as the ftrace_location_range() will not return a weak function
> if it's not part of the __mcount_loc table.
> 
> That is, would this patchset work?
> 
>    https://lore.kernel.org/all/20250102232609.529842248@goodmis.org/

I only pick patch15 and patch16 into v6.14-rc1, since most of patches in that patches
have already merged, and the issue seems gone, thanks!

> 
> -- Steve

-- 
Thanks,
Zheng Yejian