Message-ID: <20240613133711.2867745-1-zhengyejian1@huawei.com>
Date: Thu, 13 Jun 2024 21:37:05 +0800
From: Zheng Yejian <zhengyejian1@...wei.com>
To: <rostedt@...dmis.org>, <mhiramat@...nel.org>, <mark.rutland@....com>,
	<mpe@...erman.id.au>, <npiggin@...il.com>, <christophe.leroy@...roup.eu>,
	<naveen.n.rao@...ux.ibm.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<bp@...en8.de>, <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
	<hpa@...or.com>, <mcgrof@...nel.org>, <mathieu.desnoyers@...icios.com>,
	<masahiroy@...nel.org>, <nathan@...nel.org>, <nicolas@...sle.eu>,
	<kees@...nel.org>, <james.clark@....com>, <kent.overstreet@...ux.dev>,
	<yhs@...com>, <jpoimboe@...nel.org>, <peterz@...radead.org>
CC: <zhengyejian1@...wei.com>, <linux-kernel@...r.kernel.org>,
	<linux-trace-kernel@...r.kernel.org>, <linuxppc-dev@...ts.ozlabs.org>,
	<linux-modules@...r.kernel.org>, <linux-kbuild@...r.kernel.org>,
	<bpf@...r.kernel.org>
Subject: [PATCH 0/6] kallsyms: Emit symbol for holes in text and fix weak function issue

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().

Peter suggested to use entry size for FUNC type objects to find holes in
the text and fill them with a symbol, then check the mcount locations
against the symbol table and for every one that falls in a hole [1] [2].

What the patch set does is described as follows:

- Patch 1: Do an optimization for scripts/kallsym.c about memory allocation
  when read symbols from file. This patch has little to do with the above
  issue, but since I changed this script, so it also can be reviewed here;

- Patch 2: Change scripts/kallsyms.c to emit a symbol where there is a hole
  in the text, the symbol name is temporarily named "__hole_symbol_XXXXX";

- Patch 3: When lookup symbols in module, use entry size info to determine
  the exact boundaries of a function symbol;

- Patch 4: Holes in text have been found in previous patches, now check
  __fentry__ in mcount table and skip those locate in the holes;

- Patch 5: Accidentally found a out-of-bound issue when all __fentry__
  are skipped, so fix it;

- Patch 6: Revert Steve's patch about the FTRACE_MCOUNT_MAX_OFFSET
  solution, also two related definition for powerpc.

[1] https://lore.kernel.org/all/20240607150228.GR8774@noisy.programming.kicks-ass.net/
[2] https://lore.kernel.org/all/20240611092157.GU40213@noisy.programming.kicks-ass.net/

Zheng Yejian (6):
  kallsyms: Optimize multiple times of realloc() to one time of malloc()
  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/kallsyms.h          |  13 +++
 include/linux/module.h            |  14 +++
 kernel/module/kallsyms.c          |  42 ++++++--
 kernel/trace/ftrace.c             | 174 ++++++------------------------
 scripts/kallsyms.c                | 134 ++++++++++++++++++++---
 scripts/link-vmlinux.sh           |   4 +-
 scripts/mksysmap                  |   2 +-
 9 files changed, 216 insertions(+), 181 deletions(-)

-- 
2.25.1

Powered by blists - more mailing lists