Hi all,
After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
produced these warnings:
lib/strnlen_user.o: warning: objtool: strnlen_user()+0xc9: call to do_strnlen_user() with UACCESS enabled
lib/strncpy_from_user.o: warning: objtool: strncpy_from_user()+0x129: call to do_strncpy_from_user() with UACCESS enabled
vmlinux.o: warning: objtool: mce_start()+0x5c: call to __kasan_check_write() leaves .noinstr.text section
vmlinux.o: warning: objtool: mce_gather_info()+0x5f: call to v8086_mode.constprop.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: mce_read_aux()+0x8a: call to mca_msr_reg() leaves .noinstr.text section
vmlinux.o: warning: objtool: do_machine_check()+0x192: call to mce_no_way_out() leaves .noinstr.text section
vmlinux.o: warning: objtool: mce_severity_amd.constprop.0()+0xca: call to mce_severity_amd_smca() leaves .noinstr.text section
I am not sure which changes caused the above.
--
Cheers,
Stephen Rothwell
Hi all,
On Fri, 17 Dec 2021 14:40:04 +1100 Stephen Rothwell <[email protected]> wrote:
>
> After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
> produced these warnings:
>
> lib/strnlen_user.o: warning: objtool: strnlen_user()+0xc9: call to do_strnlen_user() with UACCESS enabled
> lib/strncpy_from_user.o: warning: objtool: strncpy_from_user()+0x129: call to do_strncpy_from_user() with UACCESS enabled
> vmlinux.o: warning: objtool: mce_start()+0x5c: call to __kasan_check_write() leaves .noinstr.text section
> vmlinux.o: warning: objtool: mce_gather_info()+0x5f: call to v8086_mode.constprop.0() leaves .noinstr.text section
> vmlinux.o: warning: objtool: mce_read_aux()+0x8a: call to mca_msr_reg() leaves .noinstr.text section
> vmlinux.o: warning: objtool: do_machine_check()+0x192: call to mce_no_way_out() leaves .noinstr.text section
> vmlinux.o: warning: objtool: mce_severity_amd.constprop.0()+0xca: call to mce_severity_amd_smca() leaves .noinstr.text section
>
> I am not sure which changes caused the above.
I currently still get the following warnings from an x86_64
allmodconfig build fo Linus' tree:
vmlinux.o: warning: objtool: mce_start()+0x5c: call to __kasan_check_write() leaves .noinstr.text section
vmlinux.o: warning: objtool: mce_gather_info()+0x5f: call to v8086_mode.constprop.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: mce_read_aux()+0x8a: call to mca_msr_reg() leaves .noinstr.text section
vmlinux.o: warning: objtool: do_machine_check()+0x192: call to mce_no_way_out() leaves .noinstr.text section
vmlinux.o: warning: objtool: mce_severity_amd.constprop.0()+0xca: call to mce_severity_amd_smca() leaves .noinstr.text section
$ x86_64-linux-gnu-gcc --version
x86_64-linux-gnu-gcc (Debian 11.2.0-9) 11.2.0
$ x86_64-linux-gnu-ld --version
GNU ld (GNU Binutils for Debian) 2.37
--
Cheers,
Stephen Rothwell
Hi all,
On Sat, 22 Jan 2022 10:58:06 +1100 Stephen Rothwell <[email protected]> wrote:
>
> On Fri, 17 Dec 2021 14:40:04 +1100 Stephen Rothwell <[email protected]> wrote:
> >
> > After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
> > produced these warnings:
> >
> > lib/strnlen_user.o: warning: objtool: strnlen_user()+0xc9: call to do_strnlen_user() with UACCESS enabled
> > lib/strncpy_from_user.o: warning: objtool: strncpy_from_user()+0x129: call to do_strncpy_from_user() with UACCESS enabled
> > vmlinux.o: warning: objtool: mce_start()+0x5c: call to __kasan_check_write() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: mce_gather_info()+0x5f: call to v8086_mode.constprop.0() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: mce_read_aux()+0x8a: call to mca_msr_reg() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: do_machine_check()+0x192: call to mce_no_way_out() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: mce_severity_amd.constprop.0()+0xca: call to mce_severity_amd_smca() leaves .noinstr.text section
> >
> > I am not sure which changes caused the above.
>
> I currently still get the following warnings from an x86_64
> allmodconfig build fo Linus' tree:
>
> vmlinux.o: warning: objtool: mce_start()+0x5c: call to __kasan_check_write() leaves .noinstr.text section
> vmlinux.o: warning: objtool: mce_gather_info()+0x5f: call to v8086_mode.constprop.0() leaves .noinstr.text section
> vmlinux.o: warning: objtool: mce_read_aux()+0x8a: call to mca_msr_reg() leaves .noinstr.text section
> vmlinux.o: warning: objtool: do_machine_check()+0x192: call to mce_no_way_out() leaves .noinstr.text section
> vmlinux.o: warning: objtool: mce_severity_amd.constprop.0()+0xca: call to mce_severity_amd_smca() leaves .noinstr.text section
>
> $ x86_64-linux-gnu-gcc --version
> x86_64-linux-gnu-gcc (Debian 11.2.0-9) 11.2.0
> $ x86_64-linux-gnu-ld --version
> GNU ld (GNU Binutils for Debian) 2.37
I gained these new ones after merging today's tip tree:
arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_2block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_4block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx() falls through to next function poly1305_blocks_x86_64()
arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_emit_avx() falls through to next function poly1305_emit_x86_64()
arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx2() falls through to next function poly1305_blocks_x86_64()
arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx512() falls through to next function poly1305_blocks_x86_64()
--
Cheers,
Stephen Rothwell
On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the tip tree, today's linux-next build (x864 allmodconfig)
> produced these new warnings:
>
> vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
Hurmph, lemme go figure out where that code comes from, I've not seen
those.
> [ Note I was already getting these:
> arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_2block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
> arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_4block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx() falls through to next function poly1305_blocks_x86_64()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_emit_avx() falls through to next function poly1305_emit_x86_64()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx2() falls through to next function poly1305_blocks_x86_64()
Yes, those are somewhere on the todo list, lemme bump them.
On Mon, 21 Mar 2022 17:50:50 +0100
Peter Zijlstra <[email protected]> wrote:
> > This also assumes that we need to trace everything that is marked. I
> > mentioned in another email, what do we do if we only trace funcA?
>
> Like I said later on; if we inhibit tail-calls to notrace, this goes
> away.
Please no. The number of "notrace" functions is increasing to the point
that it's starting to make function tracing useless in a lot of
circumstances. I've already lost my ability to see when user space goes
into the kernel (which I have to hack up custom coding to enable again).
-- Steve
On Mon, Mar 21, 2022 at 9:45 AM Peter Zijlstra <[email protected]> wrote:
> >
> > It's presumably not in any of the pull requests I already have
> > pending, but it would be nice if I saw some details of _what_ you are
> > complaining about, and not just the complaint itself ;)
>
> Duh, right. It's this series:
>
> https://lore.kernel.org/bpf/164757541675.26179.17727138330733641017.git-patchwork-notify@kernel.org/
>
> That went into bpf-next last Friday. I just checked but haven't found a
> pull for it yet.
Thanks. I can confirm it's not in any of the pull requests I have
pending, so I'll just start doing my normal work and try to remember
to look out for this issue later.
Linus
On Mon, Mar 21, 2022 at 12:15:49PM -0400, Steven Rostedt wrote:
> On Mon, 21 Mar 2022 12:12:09 -0400
> Steven Rostedt <[email protected]> wrote:
>
> > > > funcB:
> > > > call __fentry__
> > > push funcB on trace-stack
> > > >
> > > > [..]
> > > call __fexit__
> > > pop trace-stack until empty
> > > 'exit funcB'
> > > 'exit funcA'
> >
> > And what happens if funcC called funcA and it too was on the stack. We pop
> > that too? But it's not done yet, because calling of funcA was not a tail
> > call.
Hmm, yeah, how about we have __ftail__ mark the left function.
func_B()
{
...
}
func_A()
{
...
return func_B();
}
func_C()
{
func_A();
...
return;
}
func_B:
call __fentry__ /* push func_B */
...
call __fexit__ /* pop 1 + tails */
ret
func_A:
call __fentry__ /* push func_A */
...
call __ftail__ /* mark func_A tail */
jmp func_B
func_C:
call __fentry__ /* push func_C */
call func_A;
...
call __fexit__ /* pop 1 + tails */
ret;
Then the stack at the end of func_B looks something like:
func_C
func_A (tail)
func_B
And it will pop func_B plus all tails (func_A).
> And I just thought of another issue, where even my solution wont fix it.
> What happens if we trace funcA but not funcB? How do we get to trace the
> end of funcA?
Disallow tail calls to notrace?
On Mon, Mar 21, 2022 at 02:45:16PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 21, 2022 at 02:08:23PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > > Hi all,
> > > > >
> > > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > > produced these new warnings:
> > > > >
> > > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > >
> > > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > > those.
> > >
> > > Ahh, something tracing. I'll go do some patches on top of it.
> >
> > Also, that x86 patch has never his [email protected] and doesn't have an
> > ACK from any x86 person :-(((
>
> Worse, it adds a 3rd return trampoline without replacing any of the
> existing two :-(
>
> Why was this merged?
It additionally gets ret wrong:
vmlinux.o: warning: objtool: arch_rethook_trampoline()+0x4a: missing int3 after ret
and afaict regs->ss is garbage (much like kretprobes it appears).
Can we please unmerge this stuff and try again later?
On Mon, Mar 21, 2022 at 04:28:06PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 21, 2022 at 02:45:16PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 02:08:23PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > > > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > > > produced these new warnings:
> > > > > >
> > > > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > >
> > > > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > > > those.
> > > >
> > > > Ahh, something tracing. I'll go do some patches on top of it.
> > >
> > > Also, that x86 patch has never his [email protected] and doesn't have an
> > > ACK from any x86 person :-(((
> >
> > Worse, it adds a 3rd return trampoline without replacing any of the
> > existing two :-(
> >
> > Why was this merged?
>
> It additionally gets ret wrong:
>
> vmlinux.o: warning: objtool: arch_rethook_trampoline()+0x4a: missing int3 after ret
>
> and afaict regs->ss is garbage (much like kretprobes it appears).
>
> Can we please unmerge this stuff and try again later?
This landing in -next only today (after having been committed last
friday night) is another clue it should probably go next round.
On Mon, 21 Mar 2022 12:12:09 -0400
Steven Rostedt <[email protected]> wrote:
> > > funcB:
> > > call __fentry__
> > push funcB on trace-stack
> > >
> > > [..]
> > call __fexit__
> > pop trace-stack until empty
> > 'exit funcB'
> > 'exit funcA'
>
> And what happens if funcC called funcA and it too was on the stack. We pop
> that too? But it's not done yet, because calling of funcA was not a tail
> call.
And I just thought of another issue, where even my solution wont fix it.
What happens if we trace funcA but not funcB? How do we get to trace the
end of funcA?
-- Steve
On Mon, 21 Mar 2022 17:40:32 +0100
Peter Zijlstra <[email protected]> wrote:
> func_B:
> call __fentry__ /* push func_B */
> ...
> call __fexit__ /* pop 1 + tails */
> ret
>
> func_A:
> call __fentry__ /* push func_A */
> ...
> call __ftail__ /* mark func_A tail */
> jmp func_B
>
> func_C:
> call __fentry__ /* push func_C */
> call func_A;
> ...
> call __fexit__ /* pop 1 + tails */
> ret;
This also assumes that we need to trace everything that is marked. I
mentioned in another email, what do we do if we only trace funcA?
-- Steve
On Mon, 21 Mar 2022 12:22:59 -0400
Steven Rostedt <[email protected]> wrote:
> Or maybe another solution is:
>
> funcA:
> [..]
> jmp funcB
> call __fexit__
> ret
>
> And if funcA is being traced, we change jmp to a call.
>
> [..]
> call funcB
> call __fexit__
We could also make __fexit__ a tail call:
> ret
funcA:
[..]
call funcB
jmp __fexit__
We would also need a way to know that funcA has a tail call at the end. So
more help from either the compiler or objtool.
-- Steve
On Mon, 21 Mar 2022 14:04:05 +0100
Peter Zijlstra <[email protected]> wrote:
> Ahh, something tracing. I'll go do some patches on top of it.
>
> Also, folks, I'm thinking we should start to move to __fexit__, if CET
> SHSTK ever wants to come to kernel land return trampolines will
> insta-stop working.
>
> Hjl, do you think we could get -mfexit to go along with -mfentry ?
If we do every add a -mfexit, we will need to add a __ftail__ call.
Because, the current function exit tracing works for functions, even with
tail calls.
int funcA () {
[..]
return funcB();
}
Can turn into:
[..]
pop all stack from funcA
load reg params to funcB
jmp funcB
Then when funcB does does it's
[..]
ret
It will pop the call site of funcA (not the call site of funcB) and return
to wherever called funcA with the proper return values.
This currently works with function graph and kretprobe tracing because of
the shadow stack. Let's say we traced both funcA and funcB
funcA:
call __fentry__
Replace caller address with graph_trampoline and
store the return caller into the shadow stack.
[..]
jmp funcB
funcB:
call __fentry__
Replace caller address with graph_trampoline and
store the return caller (which is the
graph_trampoline that was switched earlier) in the
shadow stack.
[..]
ret
Returns to the graph_trampoline and we trace the
return of funcB. Then we pop off the shadow stack
and jump to that. But the shadow stack had a call
to the graph_trampoline, which gets called again.
Returns to the graph_trampoline and we trace the
return of funcA. Then we pop off the shadow stack
and jump to that, which is the original caller to
funcA.
That is, the current algorithm traces the end of both funcA and funcB
without issue, because of how the shadow stack works.
Now if we add a __fexit__, we will need a way to tell the tracers how to
record this scenario. That is why I'm thinking of a jmp to __ftail__.
Perhaps something like:
funcA:
call __fentry__
[..]
push address of funcB
jmp __ftail__
jmp funcB
Where, __ftail__ would do at the end:
ret
To jump to funcB and we skip the jmp to funcB anyway.
And to "nop" it out, we would have to convert it to.
funcA:
call __fentry__
[..]
jmp 1
jmp __ftail__
1: jmp funcB
This is one way I can think of if we include a __fexit__. But to maintain
backward compatibility to function graph tracing (which is a requirement),
we need to be able to handle such cases.
Perhaps this is a good topic to bring up at Plumbers? :-)
Do I need to submit a tracing MC, or can we have this conversation at a
compiler / toolchain MC?
-- Steve
On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > produced these new warnings:
> >
> > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
>
> Hurmph, lemme go figure out where that code comes from, I've not seen
> those.
Ahh, something tracing. I'll go do some patches on top of it.
Also, folks, I'm thinking we should start to move to __fexit__, if CET
SHSTK ever wants to come to kernel land return trampolines will
insta-stop working.
Hjl, do you think we could get -mfexit to go along with -mfentry ?
On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > produced these new warnings:
> > >
> > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> >
> > Hurmph, lemme go figure out where that code comes from, I've not seen
> > those.
>
> Ahh, something tracing. I'll go do some patches on top of it.
The below gets rid of the objtool warnings.
But I still think it's fairly terrible to get a (flawed) carbon copy of
the kretprobe code. Also, I think both should fix regs->ss.
---
diff --git a/arch/x86/kernel/rethook.c b/arch/x86/kernel/rethook.c
index f0f2f0608282..227a1890a984 100644
--- a/arch/x86/kernel/rethook.c
+++ b/arch/x86/kernel/rethook.c
@@ -20,6 +20,7 @@ asm(
".type arch_rethook_trampoline, @function\n"
"arch_rethook_trampoline:\n"
#ifdef CONFIG_X86_64
+ ANNOTATE_NOENDBR
/* Push a fake return address to tell the unwinder it's a kretprobe. */
" pushq $arch_rethook_trampoline\n"
UNWIND_HINT_FUNC
@@ -48,7 +49,7 @@ asm(
" addl $4, %esp\n"
" popfl\n"
#endif
- " ret\n"
+ ASM_RET
".size arch_rethook_trampoline, .-arch_rethook_trampoline\n"
);
NOKPROBE_SYMBOL(arch_rethook_trampoline);
On Mon, 21 Mar 2022 12:15:49 -0400
Steven Rostedt <[email protected]> wrote:
> And I just thought of another issue, where even my solution wont fix it.
> What happens if we trace funcA but not funcB? How do we get to trace the
> end of funcA?
The only solution I can think of to handle all these cases is if you enable
-mfexit, you have to disable tail calls completely. That's going to cause
a performance impact.
Perhaps we need need compiler help to give us a way to hijack the return
address. But is there a way to do this and still not give up the security
that CET SHSTK gives us?
Or maybe another solution is:
funcA:
[..]
jmp funcB
call __fexit__
ret
And if funcA is being traced, we change jmp to a call.
[..]
call funcB
call __fexit__
ret
Such that we only remove the tail calls if we enable tracing on the
function with the tail call.
-- Steve
On Mon, Mar 21, 2022 at 8:46 AM Peter Zijlstra <[email protected]> wrote:
>
> This landing in -next only today (after having been committed last
> friday night) is another clue it should probably go next round.
I went and looked at lore to get the context that I didn't get in this
email that I was added to the participants for.
I didn't find the context there either.
Sure, I found the thread, but the whole "that x86 patch" that you
refer to was never actually specified even in the full thread as far
as I can tell. I see that there is an arm64 equivalent too of what you
are complaining about, and I have no idea about that one either..
Mind actually giving the full details so that we don't have to go
re-do the work you already did?
Because right now I know something is wrong, I know the warnings, but
I don't actually have any handle on the actual patches to look out
for.
It's presumably not in any of the pull requests I already have
pending, but it would be nice if I saw some details of _what_ you are
complaining about, and not just the complaint itself ;)
Linus
On Mon, Mar 21, 2022 at 12:45:51PM -0400, Steven Rostedt wrote:
> On Mon, 21 Mar 2022 17:40:32 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > func_B:
> > call __fentry__ /* push func_B */
> > ...
> > call __fexit__ /* pop 1 + tails */
> > ret
> >
> > func_A:
> > call __fentry__ /* push func_A */
> > ...
> > call __ftail__ /* mark func_A tail */
> > jmp func_B
> >
> > func_C:
> > call __fentry__ /* push func_C */
> > call func_A;
> > ...
> > call __fexit__ /* pop 1 + tails */
> > ret;
>
> This also assumes that we need to trace everything that is marked. I
> mentioned in another email, what do we do if we only trace funcA?
Like I said later on; if we inhibit tail-calls to notrace, this goes
away.
On Mon, Mar 21, 2022 at 09:37:35AM -0700, Linus Torvalds wrote:
> On Mon, Mar 21, 2022 at 8:46 AM Peter Zijlstra <[email protected]> wrote:
> >
> > This landing in -next only today (after having been committed last
> > friday night) is another clue it should probably go next round.
>
> I went and looked at lore to get the context that I didn't get in this
> email that I was added to the participants for.
>
> I didn't find the context there either.
>
> Sure, I found the thread, but the whole "that x86 patch" that you
> refer to was never actually specified even in the full thread as far
> as I can tell. I see that there is an arm64 equivalent too of what you
> are complaining about, and I have no idea about that one either..
>
> Mind actually giving the full details so that we don't have to go
> re-do the work you already did?
>
> Because right now I know something is wrong, I know the warnings, but
> I don't actually have any handle on the actual patches to look out
> for.
>
> It's presumably not in any of the pull requests I already have
> pending, but it would be nice if I saw some details of _what_ you are
> complaining about, and not just the complaint itself ;)
Duh, right. It's this series:
https://lore.kernel.org/bpf/164757541675.26179.17727138330733641017.git-patchwork-notify@kernel.org/
That went into bpf-next last Friday. I just checked but haven't found a
pull for it yet.
On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > produced these new warnings:
> > >
> > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> >
> > Hurmph, lemme go figure out where that code comes from, I've not seen
> > those.
>
> Ahh, something tracing. I'll go do some patches on top of it.
Also, that x86 patch has never his [email protected] and doesn't have an
ACK from any x86 person :-(((
On Mon, Mar 21, 2022 at 11:28:05AM -0400, Steven Rostedt wrote:
> On Mon, 21 Mar 2022 14:04:05 +0100
> Peter Zijlstra <[email protected]> wrote:
> > Also, folks, I'm thinking we should start to move to __fexit__, if CET
> > SHSTK ever wants to come to kernel land return trampolines will
> > insta-stop working.
> >
> > Hjl, do you think we could get -mfexit to go along with -mfentry ?
> int funcA () {
> [..]
> return funcB();
> }
> This currently works with function graph and kretprobe tracing because of
> the shadow stack. Let's say we traced both funcA and funcB
>
> funcA:
> call __fentry__
push funcA on trace-stack
>
> [..]
> jmp funcB
>
> funcB:
> call __fentry__
push funcB on trace-stack
>
> [..]
call __fexit__
pop trace-stack until empty
'exit funcB'
'exit funcA'
> ret
>
> That is, the current algorithm traces the end of both funcA and funcB
> without issue, because of how the shadow stack works.
And it all works, no? Or what am I missing?
On Mon, Mar 21, 2022 at 02:08:23PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > produced these new warnings:
> > > >
> > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > >
> > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > those.
> >
> > Ahh, something tracing. I'll go do some patches on top of it.
>
> Also, that x86 patch has never his [email protected] and doesn't have an
> ACK from any x86 person :-(((
Worse, it adds a 3rd return trampoline without replacing any of the
existing two :-(
Why was this merged?
Hi all,
On Mon, 21 Mar 2022 09:52:58 -0700 Linus Torvalds <[email protected]> wrote:
>
> On Mon, Mar 21, 2022 at 9:45 AM Peter Zijlstra <[email protected]> wrote:
> > >
> > > It's presumably not in any of the pull requests I already have
> > > pending, but it would be nice if I saw some details of _what_ you are
> > > complaining about, and not just the complaint itself ;)
> >
> > Duh, right. It's this series:
> >
> > https://lore.kernel.org/bpf/164757541675.26179.17727138330733641017.git-patchwork-notify@kernel.org/
> >
> > That went into bpf-next last Friday. I just checked but haven't found a
> > pull for it yet.
>
> Thanks. I can confirm it's not in any of the pull requests I have
> pending, so I'll just start doing my normal work and try to remember
> to look out for this issue later.
The normal path for bpf-next code is via the net-next tree. But the
above series has not yet been merged into the net-next tree so is only
in the bpf-next tree.
So, what am I to do? Drop the bpf-next tree from linux-next until this
is resolved? Some input from the BPF people would be useful.
Dave, Jakub, please do not merge the bpf-bext tree into the net-next
tree for now.
--
Cheers,
Stephen Rothwell
On Mon, Mar 21, 2022 at 3:05 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> On Mon, 21 Mar 2022 09:52:58 -0700 Linus Torvalds <[email protected]> wrote:
> >
> > On Mon, Mar 21, 2022 at 9:45 AM Peter Zijlstra <[email protected]> wrote:
> > > >
> > > > It's presumably not in any of the pull requests I already have
> > > > pending, but it would be nice if I saw some details of _what_ you are
> > > > complaining about, and not just the complaint itself ;)
> > >
> > > Duh, right. It's this series:
> > >
> > > https://lore.kernel.org/bpf/164757541675.26179.17727138330733641017.git-patchwork-notify@kernel.org/
> > >
> > > That went into bpf-next last Friday. I just checked but haven't found a
> > > pull for it yet.
> >
> > Thanks. I can confirm it's not in any of the pull requests I have
> > pending, so I'll just start doing my normal work and try to remember
> > to look out for this issue later.
>
> The normal path for bpf-next code is via the net-next tree. But the
> above series has not yet been merged into the net-next tree so is only
> in the bpf-next tree.
>
> So, what am I to do? Drop the bpf-next tree from linux-next until this
> is resolved? Some input from the BPF people would be useful.
>
> Dave, Jakub, please do not merge the bpf-bext tree into the net-next
> tree for now.
That makes little sense. It's not an unusual merge conflict.
Peter's endbr series conflict with Masami's fprobe.
Peter has a trivial patch that fixes objtool warning.
The question is how to land that patch.
I think the best is for Linus to apply it after bpf-next->net-next gets
merged.
We're preparing bpf-next PR right now.
On Mon, 21 Mar 2022 14:45:16 +0100
Peter Zijlstra <[email protected]> wrote:
> On Mon, Mar 21, 2022 at 02:08:23PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > > Hi all,
> > > > >
> > > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > > produced these new warnings:
> > > > >
> > > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > >
> > > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > > those.
> > >
> > > Ahh, something tracing. I'll go do some patches on top of it.
> >
> > Also, that x86 patch has never his [email protected] and doesn't have an
> > ACK from any x86 person :-(((
>
> Worse, it adds a 3rd return trampoline without replacing any of the
> existing two :-(
Sorry about this. I missed to add Ccing to arch maintainers.
Let me check how I can fix those errors.
Thanks.
>
> Why was this merged?
--
Masami Hiramatsu <[email protected]>
On Mon, 21 Mar 2022 17:48:54 +0100
Peter Zijlstra <[email protected]> wrote:
> On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > produced these new warnings:
> > > >
> > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > >
> > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > those.
> >
> > Ahh, something tracing. I'll go do some patches on top of it.
>
> The below gets rid of the objtool warnings.
Yes, I confirmed that.
>
> But I still think it's fairly terrible to get a (flawed) carbon copy of
> the kretprobe code.
Indeed. I would like to replace the trampoline code of kretprobe with
rethook, eventually. There is no reason why we keep the clone.
(But I need more arch maintainers help for that, there are too many
archs implemented kretprobes)
> Also, I think both should fix regs->ss.
I'm not sure this part. Since the return trampoline should run in the same
context of the called function, isn't ss same there too?
Thank you,
>
> ---
> diff --git a/arch/x86/kernel/rethook.c b/arch/x86/kernel/rethook.c
> index f0f2f0608282..227a1890a984 100644
> --- a/arch/x86/kernel/rethook.c
> +++ b/arch/x86/kernel/rethook.c
> @@ -20,6 +20,7 @@ asm(
> ".type arch_rethook_trampoline, @function\n"
> "arch_rethook_trampoline:\n"
> #ifdef CONFIG_X86_64
> + ANNOTATE_NOENDBR
> /* Push a fake return address to tell the unwinder it's a kretprobe. */
> " pushq $arch_rethook_trampoline\n"
> UNWIND_HINT_FUNC
> @@ -48,7 +49,7 @@ asm(
> " addl $4, %esp\n"
> " popfl\n"
> #endif
> - " ret\n"
> + ASM_RET
> ".size arch_rethook_trampoline, .-arch_rethook_trampoline\n"
> );
> NOKPROBE_SYMBOL(arch_rethook_trampoline);
--
Masami Hiramatsu <[email protected]>
On Mon, Mar 21, 2022 at 03:12:05PM -0700, Alexei Starovoitov wrote:
>
> That makes little sense. It's not an unusual merge conflict.
It is not only that; it is adding code that with an x86 arch maintainer
hat on I've never seen and don't agree with. Same for arm64 apparently.
On Mon, Mar 21, 2022 at 12:54:19PM -0400, Steven Rostedt wrote:
> On Mon, 21 Mar 2022 17:50:50 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > > This also assumes that we need to trace everything that is marked. I
> > > mentioned in another email, what do we do if we only trace funcA?
> >
> > Like I said later on; if we inhibit tail-calls to notrace, this goes
> > away.
>
> Please no. The number of "notrace" functions is increasing to the point
> that it's starting to make function tracing useless in a lot of
> circumstances. I've already lost my ability to see when user space goes
> into the kernel (which I have to hack up custom coding to enable again).
I really can't follow the argument there, nor on IRC.
Suppose:
notrace func_B()
{
...
}
func_A()
{
...
return func_B();
}
then inhibiting tail calls would end up looking like:
func_A:
call __fentry__
...
call func_B
call __fexit__
ret
Then A is fully traced, B is invisible, as per spec. What is the
problem?
The problem you initially had, of doing a tail-call into a notrace, was
that the __fexit__ call went missing, because notrace will obviously not
have that. But that's avoided by inhibiting all tail-calls between
notrace and !notrace functions (note that notrace must also not
tail-call !notrace).
Your worry seems to stem about loosing visiblilty of !notrace functions,
but AFAICT that doesn't happen.
On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
> > Also, I think both should fix regs->ss.
>
> I'm not sure this part. Since the return trampoline should run in the same
> context of the called function, isn't ss same there too?
It creates pt_regs on the stack, so the trampolines do:
push $arch_rethook_trampoline
push %rsp
pushf
sub $24, %rsp /* cs, ip, orig_ax */
push %rdi
...
push %r15
That means that if anybody looks at regs->ss, it'll find
$arch_rethook_trampoline, which isn't a valid segment descriptor, or am
I just really bad at counting today?
I'm thinking you want a copy of __KERNEL_DS in that stack slot, not a
function pointer.
On Tue, 22 Mar 2022 09:08:22 +0100
Peter Zijlstra <[email protected]> wrote:
> On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
>
> > > Also, I think both should fix regs->ss.
> >
> > I'm not sure this part. Since the return trampoline should run in the same
> > context of the called function, isn't ss same there too?
>
> It creates pt_regs on the stack, so the trampolines do:
>
> push $arch_rethook_trampoline
> push %rsp
> pushf
> sub $24, %rsp /* cs, ip, orig_ax */
> push %rdi
> ...
> push %r15
>
> That means that if anybody looks at regs->ss, it'll find
> $arch_rethook_trampoline, which isn't a valid segment descriptor, or am
> I just really bad at counting today?
Ah, got it. It seems that the ss was skipped from the beginning, and
no one argued that.
> I'm thinking you want a copy of __KERNEL_DS in that stack slot, not a
> function pointer.
The function pointer is for unwinding stack which involves the kretprobe.
Anyway, I can add a slot for ss if it is neeeded. But if it always be
__KERNEL_DS, is it worth to save it?
Thank you,
--
Masami Hiramatsu <[email protected]>
On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
> > But I still think it's fairly terrible to get a (flawed) carbon copy of
> > the kretprobe code.
>
> Indeed. I would like to replace the trampoline code of kretprobe with
> rethook, eventually. There is no reason why we keep the clone.
> (But I need more arch maintainers help for that, there are too many
> archs implemented kretprobes)
CONFIG_KPROBE_ON_RETHOOK - and then implement archs one by one?
On Tue, Mar 22, 2022 at 06:14:54PM +0900, Masami Hiramatsu wrote:
> On Tue, 22 Mar 2022 09:08:22 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
> >
> > > > Also, I think both should fix regs->ss.
> > >
> > > I'm not sure this part. Since the return trampoline should run in the same
> > > context of the called function, isn't ss same there too?
> >
> > It creates pt_regs on the stack, so the trampolines do:
> >
> > push $arch_rethook_trampoline
> > push %rsp
> > pushf
> > sub $24, %rsp /* cs, ip, orig_ax */
> > push %rdi
> > ...
> > push %r15
> >
> > That means that if anybody looks at regs->ss, it'll find
> > $arch_rethook_trampoline, which isn't a valid segment descriptor, or am
> > I just really bad at counting today?
>
> Ah, got it. It seems that the ss was skipped from the beginning, and
> no one argued that.
Yeah, this is a long-standing issue, but I noticed it when looking at
the code yesterday.
> > I'm thinking you want a copy of __KERNEL_DS in that stack slot, not a
> > function pointer.
>
> The function pointer is for unwinding stack which involves the kretprobe.
> Anyway, I can add a slot for ss if it is neeeded. But if it always be
> __KERNEL_DS, is it worth to save it?
Probably, to save someone future head-aches. The insn-eval.c stuff will
actually look at SS when it tries to decode BP/SP fields, and I've got
vague memories of actually using that a while ago. I think I was playing
around with double-fault and the whole espfix64 mess and hit the ESPFIX
segment.
On Tue, 22 Mar 2022 08:54:55 +0100
Peter Zijlstra <[email protected]> wrote:
> On Mon, Mar 21, 2022 at 12:54:19PM -0400, Steven Rostedt wrote:
> > On Mon, 21 Mar 2022 17:50:50 +0100
> > Peter Zijlstra <[email protected]> wrote:
> >
> > > > This also assumes that we need to trace everything that is marked. I
> > > > mentioned in another email, what do we do if we only trace funcA?
> > >
> > > Like I said later on; if we inhibit tail-calls to notrace, this goes
> > > away.
> >
> > Please no. The number of "notrace" functions is increasing to the point
> > that it's starting to make function tracing useless in a lot of
> > circumstances. I've already lost my ability to see when user space goes
> > into the kernel (which I have to hack up custom coding to enable again).
>
> I really can't follow the argument there, nor on IRC.
>
> Suppose:
>
> notrace func_B()
> {
> ...
> }
>
> func_A()
> {
> ...
> return func_B();
> }
>
> then inhibiting tail calls would end up looking like:
If we inhibit tail calls, then we do not need to make func_B notrace.
>
> func_A:
> call __fentry__
> ...
> call func_B
> call __fexit__
> ret
>
> Then A is fully traced, B is invisible, as per spec. What is the
> problem?
The above is fine, but then func_B is not a tail call and can also be
traced.
>
> The problem you initially had, of doing a tail-call into a notrace, was
> that the __fexit__ call went missing, because notrace will obviously not
> have that. But that's avoided by inhibiting all tail-calls between
> notrace and !notrace functions (note that notrace must also not
> tail-call !notrace).
I'm confused by the above. Why can't a notrace tail call a !notrace?
If we tail call to a
func_B:
call __fentry__
...
call __fexit__
ret
then the fentry and fexit show a perfectly valid trace of func_B.
>
> Your worry seems to stem about loosing visiblilty of !notrace functions,
> but AFAICT that doesn't happen.
My worry is:
func_A:
call __fentry__
...
jmp func_B
Where do we do the call __fexit__ ?
That was the original concern, and I think the proposed solutions have
convoluted our thoughts about what we are trying to fix. So let's go back
to the beginning, and see how to deal with it.
That is, we have:
func_C:
call __fenty__
...
call func_A:
...
call func_B:
...
call __fexit__
ret
func_A:
call __fentry__
...
jmp func_B
func_B:
call __fentry__
...
call __fexit__
ret
Where the above is C calling A and B as normal functions, A calling B as a
tail call and B just being a normal function called by both A and C (and
many other functions).
And note, I do not want to limit function tracing (which does not rely on
__fexit__) just because we can't figure out how to handle __fexit__.
-- Steve
On Tue, 22 Mar 2022 13:17:18 +0100
Peter Zijlstra <[email protected]> wrote:
> On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
>
> > > But I still think it's fairly terrible to get a (flawed) carbon copy of
> > > the kretprobe code.
> >
> > Indeed. I would like to replace the trampoline code of kretprobe with
> > rethook, eventually. There is no reason why we keep the clone.
> > (But I need more arch maintainers help for that, there are too many
> > archs implemented kretprobes)
>
> CONFIG_KPROBE_ON_RETHOOK - and then implement archs one by one?
Sounds good! Maybe we will see different data structure fields
which depends on that config, but those are internal fields, so
user will not access it.
Thank you,
--
Masami Hiramatsu <[email protected]>
On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
> On Mon, 21 Mar 2022 17:48:54 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > > Hi all,
> > > > >
> > > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > > produced these new warnings:
> > > > >
> > > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > >
> > > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > > those.
> > >
> > > Ahh, something tracing. I'll go do some patches on top of it.
> >
> > The below gets rid of the objtool warnings.
>
> Yes, I confirmed that.
>
> > But I still think it's fairly terrible to get a (flawed) carbon copy of
> > the kretprobe code.
>
> Indeed. I would like to replace the trampoline code of kretprobe with
> rethook, eventually. There is no reason why we keep the clone.
> (But I need more arch maintainers help for that, there are too many
> archs implemented kretprobes)
FWIW, I'm more than happy to help on the arm64 side if you could Cc me for
that; I'm aware of other things in this area I'd like to clean up for
backtracing, too.
Thanks,
Mark.
On Tue, 22 Mar 2022 13:15:58 +0000
Mark Rutland <[email protected]> wrote:
> On Tue, Mar 22, 2022 at 02:31:36PM +0900, Masami Hiramatsu wrote:
> > On Mon, 21 Mar 2022 17:48:54 +0100
> > Peter Zijlstra <[email protected]> wrote:
> >
> > > On Mon, Mar 21, 2022 at 02:04:05PM +0100, Peter Zijlstra wrote:
> > > > On Mon, Mar 21, 2022 at 01:55:49PM +0100, Peter Zijlstra wrote:
> > > > > On Mon, Mar 21, 2022 at 02:03:27PM +1100, Stephen Rothwell wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > After merging the tip tree, today's linux-next build (x864 allmodconfig)
> > > > > > produced these new warnings:
> > > > > >
> > > > > > vmlinux.o: warning: objtool: arch_rethook_prepare()+0x55: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: arch_rethook_trampoline_callback()+0x3e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x93e: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x5f2: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: unwind_next_frame()+0x4a7: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x81: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: __rethook_find_ret_addr()+0x90: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x8c: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > > > vmlinux.o: warning: objtool: rethook_trampoline_handler()+0x9b: relocation to !ENDBR: arch_rethook_trampoline+0x0
> > > > >
> > > > > Hurmph, lemme go figure out where that code comes from, I've not seen
> > > > > those.
> > > >
> > > > Ahh, something tracing. I'll go do some patches on top of it.
> > >
> > > The below gets rid of the objtool warnings.
> >
> > Yes, I confirmed that.
> >
> > > But I still think it's fairly terrible to get a (flawed) carbon copy of
> > > the kretprobe code.
> >
> > Indeed. I would like to replace the trampoline code of kretprobe with
> > rethook, eventually. There is no reason why we keep the clone.
> > (But I need more arch maintainers help for that, there are too many
> > archs implemented kretprobes)
>
> FWIW, I'm more than happy to help on the arm64 side if you could Cc me for
> that; I'm aware of other things in this area I'd like to clean up for
> backtracing, too.
Thank you for your warm help. OK, let me update and submit the rethook
for arm64 :-)
Thanks.
--
Masami Hiramatsu <[email protected]>
Hi all,
On Tue, 15 Mar 2022 13:32:49 +1100 Stephen Rothwell <[email protected]> wrote:
>
> I gained these new ones after merging today's tip tree:
>
> arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_2block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
> arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_4block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx() falls through to next function poly1305_blocks_x86_64()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_emit_avx() falls through to next function poly1305_emit_x86_64()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx2() falls through to next function poly1305_blocks_x86_64()
> arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx512() falls through to next function poly1305_blocks_x86_64()
All we have left are the poly1305_ ones. They are in Linus' tree now.
--
Cheers,
Stephen Rothwell