| 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
| ||
|
Message-ID: <Yyxf3eIF5hY7DHUh@hirez.programming.kicks-ass.net>
Date: Thu, 22 Sep 2022 15:15:09 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
x86@...nel.org, Linus Torvalds <torvalds@...ux-foundation.org>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Andrew Cooper <Andrew.Cooper3@...rix.com>,
Johannes Wikner <kwikner@...z.ch>,
Alyssa Milburn <alyssa.milburn@...ux.intel.com>,
Jann Horn <jannh@...gle.com>, "H.J. Lu" <hjl.tools@...il.com>,
Joao Moreira <joao.moreira@...el.com>,
Joseph Nuzman <joseph.nuzman@...el.com>,
Steven Rostedt <rostedt@...dmis.org>,
Juergen Gross <jgross@...e.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
K Prateek Nayak <kprateek.nayak@....com>,
Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH v3 32/59] objtool: Allow STT_NOTYPE -> STT_FUNC+0
tail-calls
On Thu, Sep 22, 2022 at 12:47:26PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 22, 2022 at 12:29:58PM +0200, Peter Zijlstra wrote:
> > I managed to reproduce with 12.2.0 -- my 12.1.0 compiler didn't
> > reproduce.
> >
> > $ ./scripts/objdump-func vmlinux.o ftrace_replace_code
>
> > 0000 0000000000012a83 <ftrace_replace_code.cold>:
> > 0000 12a83: 48 89 de mov %rbx,%rsi
> > 0003 12a86: 89 c7 mov %eax,%edi
> > 0005 12a88: 5b pop %rbx
> > 0006 12a89: 5d pop %rbp
> > 0007 12a8a: 41 5c pop %r12
> > 0009 12a8c: 41 5d pop %r13
> > 000b 12a8e: 41 5e pop %r14
> > 000d 12a90: e9 62 fd ff ff jmp 127f7 <ftrace_bug>
> >
> >
> > Seems to suggest objtool is actually right; I cannot find a reference to
> > that cold symbol.
>
> Ohhhh, ftrace_replace_cold is a weak function, so it could be the
> original weak symbol had a reference to the cold thing.
>
> We have some code to deal with crap like that, lemme try and figure out
> what went wrong.
The original weak function:
$ ./scripts/objdump-func kernel/trace/ftrace.o ftrace_replace_code
0000 0000000000003bc0 <ftrace_replace_code>:
0000 3bc0: f3 0f 1e fa endbr64
0004 3bc4: 8b 15 00 00 00 00 mov 0x0(%rip),%edx # 3bca <ftrace_replace_code+0xa> 3bc6: R_X86_64_PC32 .data..read_mostly+0xc
000a 3bca: 41 56 push %r14
000c 3bcc: 41 55 push %r13
000e 3bce: 41 89 fd mov %edi,%r13d
0011 3bd1: 41 54 push %r12
0013 3bd3: 41 83 e5 02 and $0x2,%r13d
0017 3bd7: 55 push %rbp
0018 3bd8: 53 push %rbx
0019 3bd9: 85 d2 test %edx,%edx
001b 3bdb: 75 67 jne 3c44 <ftrace_replace_code+0x84>
001d 3bdd: 4c 8b 35 00 00 00 00 mov 0x0(%rip),%r14 # 3be4 <ftrace_replace_code+0x24> 3be0: R_X86_64_PC32 .bss+0x54
0024 3be4: 41 89 fc mov %edi,%r12d
0027 3be7: 41 83 e4 01 and $0x1,%r12d
002b 3beb: 4d 85 f6 test %r14,%r14
002e 3bee: 74 54 je 3c44 <ftrace_replace_code+0x84>
0030 3bf0: 41 8b 46 10 mov 0x10(%r14),%eax
0034 3bf4: 31 ed xor %ebp,%ebp
0036 3bf6: 85 c0 test %eax,%eax
0038 3bf8: 7f 0b jg 3c05 <ftrace_replace_code+0x45>
003a 3bfa: eb 40 jmp 3c3c <ftrace_replace_code+0x7c>
003c 3bfc: 83 c5 01 add $0x1,%ebp
003f 3bff: 41 39 6e 10 cmp %ebp,0x10(%r14)
0043 3c03: 7e 37 jle 3c3c <ftrace_replace_code+0x7c>
0045 3c05: 48 63 dd movslq %ebp,%rbx
0048 3c08: 48 c1 e3 04 shl $0x4,%rbx
004c 3c0c: 49 03 5e 08 add 0x8(%r14),%rbx
0050 3c10: f6 43 0b 02 testb $0x2,0xb(%rbx)
0054 3c14: 75 e6 jne 3bfc <ftrace_replace_code+0x3c>
0056 3c16: 44 89 e6 mov %r12d,%esi
0059 3c19: 48 89 df mov %rbx,%rdi
005c 3c1c: e8 3f fb ff ff call 3760 <__ftrace_replace_code>
0061 3c21: 85 c0 test %eax,%eax
0063 3c23: 0f 85 00 00 00 00 jne 3c29 <ftrace_replace_code+0x69> 3c25: R_X86_64_PC32 .text.unlikely+0x368
Jumps to the cold subfunction right here ^
0069 3c29: 45 85 ed test %r13d,%r13d
006c 3c2c: 74 ce je 3bfc <ftrace_replace_code+0x3c>
006e 3c2e: e8 00 00 00 00 call 3c33 <ftrace_replace_code+0x73> 3c2f: R_X86_64_PLT32 __SCT__cond_resched-0x4
0073 3c33: 83 c5 01 add $0x1,%ebp
0076 3c36: 41 39 6e 10 cmp %ebp,0x10(%r14)
007a 3c3a: 7f c9 jg 3c05 <ftrace_replace_code+0x45>
007c 3c3c: 4d 8b 36 mov (%r14),%r14
007f 3c3f: 4d 85 f6 test %r14,%r14
0082 3c42: 75 ac jne 3bf0 <ftrace_replace_code+0x30>
0084 3c44: 5b pop %rbx
0085 3c45: 5d pop %rbp
0086 3c46: 41 5c pop %r12
0088 3c48: 41 5d pop %r13
008a 3c4a: 41 5e pop %r14
008c 3c4c: e9 00 00 00 00 jmp 3c51 <ftrace_replace_code+0x91> 3c4d: R_X86_64_PLT32 __x86_return_thunk-0x4
0091 3c51: 66 66 2e 0f 1f 84 00 00 00 00 00 data16 cs nopw 0x0(%rax,%rax,1)
009c 3c5c: 0f 1f 40 00 nopl 0x0(%rax)
00a0 3c60: 90 nop
00a1 3c61: 90 nop
00a2 3c62: 90 nop
00a3 3c63: 90 nop
00a4 3c64: 90 nop
00a5 3c65: 90 nop
00a6 3c66: 90 nop
00a7 3c67: 90 nop
00a8 3c68: 90 nop
00a9 3c69: 90 nop
00aa 3c6a: 90 nop
00ab 3c6b: 90 nop
00ac 3c6c: 90 nop
00ad 3c6d: 90 nop
00ae 3c6e: 90 nop
00af 3c6f: 90 nop
0000 000000000000036c <ftrace_replace_code.cold>:
0000 36c: 48 89 de mov %rbx,%rsi
0003 36f: 89 c7 mov %eax,%edi
0005 371: 5b pop %rbx
0006 372: 5d pop %rbp
0007 373: 41 5c pop %r12
0009 375: 41 5d pop %r13
000b 377: 41 5e pop %r14
000d 379: e9 62 fd ff ff jmp e0 <ftrace_bug>
And yes, you're right this patch broke it.
Because the weak function symbol gets deleted by the linker, these
instructions will belong to no function and insn->func will be NULL.
Then the new: STT_NOTYPE -> STT_FUNC+0 logic kicks in and turns this
into a tail-call.
That means insn->jump_dest is unset in favour of insn->call_dest, and
then the unreachable logic falls on it's face because that doesn't deal
with call_dest.
Ho-Humm.. let me find a non hacky solution.
Powered by blists - more mailing lists