Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\))
Subject: Re: [PATCH v2] x86/lib: Do not use local symbols with
 SYM_CODE_START_LOCAL()
From:   Nadav Amit <nadav.amit@gmail.com>
In-Reply-To: <20230527122916.GAZHH3nHk7kkUA7CeM@nazgul.local>
Date:   Thu, 1 Jun 2023 17:53:13 -0700
Cc:     Dave Hansen <dave.hansen@intel.com>,
        Jiri Slaby <jirislaby@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <F5C390E4-8CF7-49EE-A94C-205DB537E1AB@gmail.com>
References: <38e24fd4-9213-229d-9919-7ae3bfb113bb@intel.com>
 <24E47178-C177-425F-A8EF-CFFAE22597D4@gmail.com>
 <20230526155336.GAZHDWAFi1FRqq83TP@nazgul.local>
 <0F07EEDB-8A3F-4224-9FF1-43A5300B1B8B@gmail.com>
 <20230526204559.GAZHEahxxnQaHhSUul@nazgul.local>
 <D63AB9E6-BA52-4E24-B8EF-C7B9DB1595CC@gmail.com>
 <e6cd1909-2776-28d2-ccc0-4b3d2d09e9ce@intel.com>
 <49861038-B8CA-4CDD-BD44-73066FF453F3@gmail.com>
 <20230527072338.GAZHGv+no2LZASyLWM@nazgul.local>
 <9A302EB1-308A-4904-801C-DC70D9908E11@gmail.com>
 <20230527122916.GAZHH3nHk7kkUA7CeM@nazgul.local>
To:     Borislav Petkov <bp@alien8.de>
Precedence: bulk


> On May 27, 2023, at 5:29 AM, Borislav Petkov <bp@alien8.de> wrote:
>=20
> 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
=E2=80=9Cbad_get_user_clac=E2=80=9D, you may see __get_user_nocheck_8 . =
This is confusing and
 misleading users.

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


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 =E2=80=9Cdisas=E2=80=9D =
addresses in
bad_get_user_clac (you can x/i for reduced functionality) or crash in
which =E2=80=9Cdis=E2=80=9D 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.
=20
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.