Message-Id: <20240723063258.2240610-1-zhengyejian@huaweicloud.com>
Date: Tue, 23 Jul 2024 14:32:53 +0800
From: Zheng Yejian <zhengyejian@...weicloud.com>
To: masahiroy@...nel.org,
	peterz@...radead.org,
	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,
	hpa@...or.com,
	mcgrof@...nel.org,
	mathieu.desnoyers@...icios.com,
	nathan@...nel.org,
	nicolas@...sle.eu,
	ojeda@...nel.org,
	akpm@...ux-foundation.org,
	surenb@...gle.com,
	pasha.tatashin@...een.com,
	kent.overstreet@...ux.dev,
	james.clark@....com,
	jpoimboe@...nel.org
Cc: x86@...nel.org,
	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,
	zhengyejian@...weicloud.com
Subject: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

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

Powered by blists - more mailing lists