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
[ 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()
]
I have no idea what has caused this ...
--
Cheers,
Stephen Rothwell
On Mon, 21 Mar 2022 17:04:28 +0100
Peter Zijlstra <[email protected]> wrote:
> 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'
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.
-- Steve
>
> > 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: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 :-(
Likewise; I have the same complaints for the arm64 patch.
I haven't had the chance to review/ack that, and I'm actively working on
improving out unwinder and the way it interacts with the various *existing*
trampolines, so adding yat another is *not* good.
> Why was this merged?
Likewise, same question for arm64?
Thanks,
Mark.
Hi Alexei,
On Mon, 21 Mar 2022 15:12:05 -0700 Alexei Starovoitov <[email protected]> wrote:
>
> 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.
Peter has other concerns, please read the thread and consider them.
--
Cheers,
Stephen Rothwell
On Mon, Mar 21, 2022 at 3:46 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Alexei,
>
> On Mon, 21 Mar 2022 15:12:05 -0700 Alexei Starovoitov <[email protected]> wrote:
> >
> > 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.
>
> Peter has other concerns, please read the thread and consider them.
Masami is an expert in kprobe. He copy pasted a bit of kprobe logic
to make it into 'multi kprobe' (he calls it fprobe).
I believe he knows what he's doing.
Steven reviewed and tested that set.
We've tested it as well and don't have any correctness or api concerns.
On Mon, 21 Mar 2022 15:50:17 -0700
Alexei Starovoitov <[email protected]> wrote:
> > Peter has other concerns, please read the thread and consider them.
>
> Masami is an expert in kprobe. He copy pasted a bit of kprobe logic
> to make it into 'multi kprobe' (he calls it fprobe).
> I believe he knows what he's doing.
> Steven reviewed and tested that set.
> We've tested it as well and don't have any correctness or api concerns.
I tested it from a ftrace perspective, not an IBT or other work being done
in the x86 world.
I'm fine with the work being done in kernel/tracing/ but it still requires
the arch maintainer's acks for anything in arch/. I was under the
impression that the arch specific code was Cc'ing the arch maintainers
(which I always do when I touch their code). But I missed that they were
not. That's my fault, I should have caught that.
-- Steve
On Mon, 21 Mar 2022 15:50:17 -0700
Alexei Starovoitov <[email protected]> wrote:
> On Mon, Mar 21, 2022 at 3:46 PM Stephen Rothwell <[email protected]> wrote:
> >
> > Hi Alexei,
> >
> > On Mon, 21 Mar 2022 15:12:05 -0700 Alexei Starovoitov <[email protected]> wrote:
> > >
> > > 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.
> >
> > Peter has other concerns, please read the thread and consider them.
>
> Masami is an expert in kprobe. He copy pasted a bit of kprobe logic
> to make it into 'multi kprobe' (he calls it fprobe).
> I believe he knows what he's doing.
> Steven reviewed and tested that set.
> We've tested it as well and don't have any correctness or api concerns.
Sorry, that's my mistake to not Ccing to arch maintainers for the arch
dependent patches. Let me update and send v13 for this fprobe series.
Thank you,
--
Masami Hiramatsu <[email protected]>
On Mon, Mar 21, 2022 at 9:51 PM Masami Hiramatsu <[email protected]> wrote:
>
> On Mon, 21 Mar 2022 15:50:17 -0700
> Alexei Starovoitov <[email protected]> wrote:
>
> > On Mon, Mar 21, 2022 at 3:46 PM Stephen Rothwell <[email protected]> wrote:
> > >
> > > Hi Alexei,
> > >
> > > On Mon, 21 Mar 2022 15:12:05 -0700 Alexei Starovoitov <[email protected]> wrote:
> > > >
> > > > 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.
> > >
> > > Peter has other concerns, please read the thread and consider them.
> >
> > Masami is an expert in kprobe. He copy pasted a bit of kprobe logic
> > to make it into 'multi kprobe' (he calls it fprobe).
> > I believe he knows what he's doing.
> > Steven reviewed and tested that set.
> > We've tested it as well and don't have any correctness or api concerns.
>
> Sorry, that's my mistake to not Ccing to arch maintainers for the arch
> dependent patches. Let me update and send v13 for this fprobe series.
No. Please read what I've said earlier.
On Mon, 21 Mar 2022 12:12:09 -0400
Steven Rostedt <[email protected]> wrote:
> On Mon, 21 Mar 2022 17:04:28 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > 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
This seems wrong. We don't pop the trace-stack until empty, but we will
record the real stack pointer at funcA.
> > '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.
Thus when the funcC is called, the trace-stack will be poped until funcA,
because we can see the real stack pointer at the 'ret'.
So the funcC is still on the trace-stack after that.
Thank you,
>
> -- Steve
>
>
> >
> > > 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?
>
>
>
--
Masami Hiramatsu <[email protected]>