linux-kernel - Re: [PATCH v2] x86/lib: Do not use local symbols with SYM_CODE_START

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230527072338.GAZHGv+no2LZASyLWM@nazgul.local>
Date:   Sat, 27 May 2023 09:23:38 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     Dave Hansen <dave.hansen@...el.com>,
        Jiri Slaby <jirislaby@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] x86/lib: Do not use local symbols with
 SYM_CODE_START_LOCAL()

On Fri, May 26, 2023 at 02:55:21PM -0700, Nadav Amit wrote:
> So my tool takes a branch trace and then simulates the code execution.
> As a preparatory step I need to disassemble the code, yet as I do not
> know where the symbol starts and its size, I can only disassemble one
> instruction at a time. [ I prefer to disassemble the whole symbol at once

So in this particular case, the exception handling ends up being part of
__get_user_nocheck_8, see the end of this mail.

However, the symbol table says it is of size 19:

123630: ffffffff81b89310    19 FUNC    GLOBAL DEFAULT    1 __get_user_nocheck_8

which means the trailing exception handling is not part of that symbol
when looking at the size. And that's where your tool fails.

Close?

And if so, your tool could simply look at the next symbol's address and
attribute the trailing bytes to the previous symbol.

If you look at the disassembly at the end, some other option has added
INT3 padding to the end of the symbol so that the next one is aligned.
But you can simply skip over those 0xcc insn bytes.

And skipping over those 0xcc bytes is something your tool needs to
handle anyway because they're not part of the symbol either.

> These are only 2 things that break to one extent or another. I can
> have workarounds for them (I already do). I just see no reason to
> treat these two symbols differently.

Right, the kernel should not dance just because some tool says so. And
every time a new tool pops up, there are patches to "fix" the kernel.

> I seriously see no downside

The downside is polluting the symbol table unnecessarily. If it doesn't
have to be there then it shouldn't be.

And yeah yeah, this particular case can be fixed easily but the bigger
issue remains: we have a lot of local symbols which get discarded around
the tree. Does that mean that because your tool cannot handle that, we
have to stop using local symbols?

What happens if we do something else weird in the future and your tool
breaks again?

Also, why can't your tool handle such cases gracefully instead of having
to "fix" the kernel each time?

Thx.

ffffffff81b89310 <__get_user_nocheck_8>:
ffffffff81b89310:       90                      nop
ffffffff81b89311:       90                      nop
ffffffff81b89312:       90                      nop
ffffffff81b89313:       90                      nop
ffffffff81b89314:       90                      nop
ffffffff81b89315:       90                      nop
ffffffff81b89316:       48 8b 10                mov    (%rax),%rdx
ffffffff81b89319:       31 c0                   xor    %eax,%eax
ffffffff81b8931b:       90                      nop
ffffffff81b8931c:       90                      nop
ffffffff81b8931d:       90                      nop
ffffffff81b8931e:       e9 9d a3 01 00          jmp    ffffffff81ba36c0 <__x86_return_thunk>
ffffffff81b89323:       66 66 2e 0f 1f 84 00    data16 cs nopw 0x0(%rax,%rax,1)
ffffffff81b8932a:       00 00 00 00
ffffffff81b8932e:       66 90                   xchg   %ax,%ax
ffffffff81b89330:       90                      nop
ffffffff81b89331:       90                      nop
ffffffff81b89332:       90                      nop
ffffffff81b89333:       31 d2                   xor    %edx,%edx
ffffffff81b89335:       48 c7 c0 f2 ff ff ff    mov    $0xfffffffffffffff2,%rax
ffffffff81b8933c:       e9 7f a3 01 00          jmp    ffffffff81ba36c0 <__x86_return_thunk>
ffffffff81b89341:       cc                      int3
ffffffff81b89342:       cc                      int3
ffffffff81b89343:       cc                      int3
ffffffff81b89344:       cc                      int3
ffffffff81b89345:       cc                      int3
ffffffff81b89346:       cc                      int3
ffffffff81b89347:       cc                      int3
ffffffff81b89348:       cc                      int3
ffffffff81b89349:       cc                      int3
ffffffff81b8934a:       cc                      int3
ffffffff81b8934b:       cc                      int3
ffffffff81b8934c:       cc                      int3
ffffffff81b8934d:       cc                      int3
ffffffff81b8934e:       cc                      int3
ffffffff81b8934f:       cc                      int3

ffffffff81b89350 <__pfx_inat_get_opcode_attribute>

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette