Re: [PATCH v2] x86/lib: Do not use local symbols with SYM_CODE_START_LOCAL()

From: Nadav Amit
Date: Thu Jun 01 2023 - 20:53:41 EST




> On May 27, 2023, at 5:29 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> As to what you want to address, I'll talk to toolchain folks first and
> get back to you.

I hope you got the answer you were looking for. I am not sure what is holding
this simple patch back.

To rehash - we are talking about local two symbols that are not exposed. Based
on my search they cover the only region of the kernel text (on x86) that is
not covered by any symbol.

Doing so have two types of impacts. Some tools are affected by the fact the
closest previous symbol is not related, and as a result the symbol they show
when they unwind the stack is unrelated. So instead of seeing
“bad_get_user_clac”, you may see __get_user_nocheck_8 . This is confusing and
misleading users.

This should impact perf, ftrace, /proc/[pid]/stack, dump_stack(), BUG().

The second type of impact occurs since certain code addresses is not covered
by any symbol. This mostly results in reduced functionality of tools.

This includes for instance gdb that cannot “disas” addresses in
bad_get_user_clac (you can x/i for reduced functionality) or crash in
which “dis” only disassembles a single instruction. It might also have impact
on backtraces - I did not try it.

addr2line and llvm-symbolizer also seem to be affected in such a way and they
do not find the symbol that is associated with addresses in
bad_get_user_clac. This means that tools that rely such tools, including
decode_stacktrace.sh are also affected. [*]

There might be other impacts, for instance on kpatch.

Overall, as a general rule, I think it would be best not to have code that is
not covered by any symbol. It can result in misleading output from the kernel
or related tools, and in addition in more limited functionality from tools
such as gdb and crash.

More concretely, I think these two symbols should not be stripped. The fact
that the code of these symbols runs under relatively complicated conditions
(exception tables), makes it even more important to let debug/tracing tools
to see them.

I you wish, I can include the gist of these points in the commit log, although
I think it might be an overkill.



[*] It is worth noting that tools such as addrline and llvm-symbolizer
are able to use DWARF to find the source code location, yet they
do not appear to be able to find the relevant symbol.