linux-kernel - Have compiler remove __fentry locations from overwritten weak functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241014210841.5a4764de@gandalf.local.home>
Date: Mon, 14 Oct 2024 21:08:41 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: "Jose E. Marchesi" <jemarch@....org>, Indu Bhagat
 <indu.bhagat@...cle.com>, Nick Desaulniers <ndesaulniers@...gle.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux Trace Kernel
 <linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>, Mathieu
 Desnoyers <mathieu.desnoyers@...icios.com>, Peter Zijlstra
 <peterz@...radead.org>, Zheng Yejian <zhengyejian@...weicloud.com>, Andrii
 Nakryiko <andrii.nakryiko@...il.com>
Subject: Have compiler remove __fentry locations from overwritten weak
 functions

There's a long standing issue with having fentry in weak functions that
were overwritten. This was first caught when a "notrace" function was
showing up in the /sys/kernel/tracing/available_filter_functions list.

   https://lore.kernel.org/all/20220412094923.0abe90955e5db486b7bca279@kernel.org/

What was happening is that ftrace uses kallsyms (what is basically listed
by nm) to map the location of __fentry to the previous symbol ahead of it.
Then it says that function is the name of the function for that __fentry.
But when you have weak functions, you can have this:

  int notrace do_not_watch_me(void)
  {
	return whatever;
  }

  void __weak define_me_in_arch(void)
  {
	return;
  }

The define_me_in_arch() gets a __fentry, but because it is overwritten, it
becomes attached to the end of do_not_watch_me(). When ftrace asks kallsyms
for the symbol name of that __fentry location, it returns
do_not_watch_me(), and ftrace will simply list that.

I currently have a hack in the kernel that has:

	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;
	}

Which checks if the __fentry is too far away from the start of the symbol
returned and if so, it uses the name: __ftrace_invalid_address__

Then when you cat /sys/kernel/tracing/available_filter_functions you get:

   [..]
   do_one_initcall
   __ftrace_invalid_address___884
   rootfs_init_fs_context
   wait_for_initramfs
   __ftrace_invalid_address___84
   calibration_delay_done
   calibrate_delay
   [..]

The order of available_filter_functions matter as you can enable the filter
via the index into that file.

   seq 50 100 > /sys/kernel/tracing/set_ftrace_filter

will enable all the functions in available_filter_functions from index of
50 through 100 (it does it in n*log(n) time, where as echoing the names in
takes n^2 time). It also allows you to differentiate between different
functions with the same name.

That means the weak functions found still need to be listed, otherwise it
will make the index lookup not match what is shown.

But I was thinking, since gcc (and perhaps clang?) now has:

  -mrecord-mcount

that creates the __mcount_loc section that ftrace uses to find where the
__fentry's are. Perhaps the compiler can simply remove any of those
__fentry's that happen to exist in a weak function that was overridden.

Would this be something that the compiler could do?

-- Steve