Hopefully last posting...
Since last time:
- updated the ftrace_location() patch (naveen, rostedt)
- added a few comments and clarifications (bpetkov)
- disable jump-tables (joao)
- verified clang-14-rc2 works
- fixed a whole bunch of objtool unreachable insn issue
- picked up a few more tags
Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
Enjoy!
On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> Hopefully last posting...
>
> Since last time:
>
> - updated the ftrace_location() patch (naveen, rostedt)
> - added a few comments and clarifications (bpetkov)
> - disable jump-tables (joao)
> - verified clang-14-rc2 works
> - fixed a whole bunch of objtool unreachable insn issue
> - picked up a few more tags
>
> Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
I've tried to test it.
Applied the first 23 patches, since patch 24 failed to apply to bpf and bpf-next trees.
selftest/bpf/test_progs
shows that all bpf trampoline tests are failing and
eventually the kernel is crashing:
[ 53.040582] RIP: 0010:do_init_module+0x9/0x6f0
[ 53.052044] Call Trace:
[ 53.052319] <TASK>
[ 53.052559] bpf_trampoline_6442471381_0+0x32/0x1000
[ 53.053117] do_init_module+0x5/0x6f0
[ 53.053550] load_module+0x77c0/0x9c00
I havne't had time to debug what's going on.
On Tue, Mar 08, 2022 at 11:01:04PM +0100, Peter Zijlstra wrote:
> On Tue, Mar 08, 2022 at 12:00:52PM -0800, Alexei Starovoitov wrote:
> > On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> > > Hopefully last posting...
> > >
> > > Since last time:
> > >
> > > - updated the ftrace_location() patch (naveen, rostedt)
> > > - added a few comments and clarifications (bpetkov)
> > > - disable jump-tables (joao)
> > > - verified clang-14-rc2 works
> > > - fixed a whole bunch of objtool unreachable insn issue
> > > - picked up a few more tags
> > >
> > > Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
> >
> > I've tried to test it.
>
> I could cleanly do:
>
> git checkout tip/master
> git merge bpf-next/master
> git merge queue/x86/wip.ibt
>
> You want me to push out that result somewhere?
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
includes bpf-next/master.
On Tue, Mar 08, 2022 at 12:00:52PM -0800, Alexei Starovoitov wrote:
> On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> > Hopefully last posting...
> >
> > Since last time:
> >
> > - updated the ftrace_location() patch (naveen, rostedt)
> > - added a few comments and clarifications (bpetkov)
> > - disable jump-tables (joao)
> > - verified clang-14-rc2 works
> > - fixed a whole bunch of objtool unreachable insn issue
> > - picked up a few more tags
> >
> > Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
>
> I've tried to test it.
I could cleanly do:
git checkout tip/master
git merge bpf-next/master
git merge queue/x86/wip.ibt
You want me to push out that result somewhere?
On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> Hopefully last posting...
>
> Since last time:
>
> - updated the ftrace_location() patch (naveen, rostedt)
> - added a few comments and clarifications (bpetkov)
> - disable jump-tables (joao)
> - verified clang-14-rc2 works
> - fixed a whole bunch of objtool unreachable insn issue
> - picked up a few more tags
>
> Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
<applause> Nice work!!! kernel shadow stacks next? ;-)
Acked-by: Josh Poimboeuf <[email protected]>
As talked about on IRC there are still a few outstanding issues, that
I'm fine with fixing after the merge window during the upcoming -next
cycle:
- xen hypercall page functions need 'ret' - (I think you already fixed)
- why don't unreachables need to fill up the entire sym hole?
- get rid of the 'c_file' hack
- improve cmdline option intuitive-ness
- properly integrate the retpoline "demotion" with the new Spectre BHI
related patches - probably still needs more discussion - for example
we might instead want to disable IBT and warn
--
Josh
On Tue, Mar 08, 2022 at 11:32:37PM +0100, Peter Zijlstra wrote:
> On Tue, Mar 08, 2022 at 11:01:04PM +0100, Peter Zijlstra wrote:
> > On Tue, Mar 08, 2022 at 12:00:52PM -0800, Alexei Starovoitov wrote:
> > > On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> > > > Hopefully last posting...
> > > >
> > > > Since last time:
> > > >
> > > > - updated the ftrace_location() patch (naveen, rostedt)
> > > > - added a few comments and clarifications (bpetkov)
> > > > - disable jump-tables (joao)
> > > > - verified clang-14-rc2 works
> > > > - fixed a whole bunch of objtool unreachable insn issue
> > > > - picked up a few more tags
> > > >
> > > > Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
> > > >
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
> > >
> > > I've tried to test it.
> >
> > I could cleanly do:
> >
> > git checkout tip/master
> > git merge bpf-next/master
> > git merge queue/x86/wip.ibt
> >
> > You want me to push out that result somewhere?
>
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
>
> includes bpf-next/master.
I just managed to run bpf selftests with that kernel on a tigerlake
platform. Seems to still work.
On Tue, Mar 08, 2022 at 12:06:18PM -0800, Josh Poimboeuf wrote:
> As talked about on IRC there are still a few outstanding issues, that
> I'm fine with fixing after the merge window during the upcoming -next
> cycle:
>
> - xen hypercall page functions need 'ret' - (I think you already fixed)
>
> - why don't unreachables need to fill up the entire sym hole?
>
> - get rid of the 'c_file' hack
>
> - improve cmdline option intuitive-ness
>
> - properly integrate the retpoline "demotion" with the new Spectre BHI
> related patches - probably still needs more discussion - for example
> we might instead want to disable IBT and warn
One more:
- Changing objtool should force a vmlinux re-link.
--
Josh
On Tue, 8 Mar 2022, Peter Zijlstra wrote:
> Hopefully last posting...
>
> Since last time:
>
> - updated the ftrace_location() patch (naveen, rostedt)
> - added a few comments and clarifications (bpetkov)
> - disable jump-tables (joao)
> - verified clang-14-rc2 works
> - fixed a whole bunch of objtool unreachable insn issue
> - picked up a few more tags
>
> Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
FWIW objtool changes look good to me.
I only came across
arch/x86/kernel/head_64.o: warning: objtool: .noinstr.text: unexpected end of section
with CC_HAS_IBT=n, which you already know about.
CC_HAS_IBT=y compilation gives
vmlinux.o: warning: objtool: xen_vcpu_setup()+0xa3: unreachable instruction
Miroslav
From: Josh Poimboeuf
> Sent: 09 March 2022 06:57
>
> On Tue, Mar 08, 2022 at 12:06:18PM -0800, Josh Poimboeuf wrote:
> > As talked about on IRC there are still a few outstanding issues, that
> > I'm fine with fixing after the merge window during the upcoming -next
> > cycle:
> >
> > - xen hypercall page functions need 'ret' - (I think you already fixed)
> >
> > - why don't unreachables need to fill up the entire sym hole?
> >
> > - get rid of the 'c_file' hack
> >
> > - improve cmdline option intuitive-ness
> >
> > - properly integrate the retpoline "demotion" with the new Spectre BHI
> > related patches - probably still needs more discussion - for example
> > we might instead want to disable IBT and warn
>
> One more:
>
> - Changing objtool should force a vmlinux re-link.
I'm wondering what actually happens to loadable modules?
Especially those built 'out of tree',
potentially with a different compiler,
and maybe containing binary 'blobs'.
The requirement to run programs on old distributions means
that things get compiled with quite old versions of gcc.
For instance RHEL7 is gcc 4.8.5.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, Mar 8, 2022 at 2:32 PM Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Mar 08, 2022 at 11:01:04PM +0100, Peter Zijlstra wrote:
> > On Tue, Mar 08, 2022 at 12:00:52PM -0800, Alexei Starovoitov wrote:
> > > On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> > > > Hopefully last posting...
> > > >
> > > > Since last time:
> > > > - verified clang-14-rc2 works
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
I observed the following error when building with
CONFIG_LTO_CLANG_FULL=y enabled:
ld.lld: error: ld-temp.o <inline asm>:7:2: symbol 'ibt_selftest_ip' is
already defined
ibt_selftest_ip:
^
Seems to come from
commit a802350ba65a ("x86/ibt: Add IBT feature, MSR and #CP handling")
Commenting out the label in the inline asm, I then observed:
vmlinux.o: warning: objtool: identify_cpu()+0x6d0: sibling call from
callable instruction with modified stack frame
vmlinux.o: warning: objtool: identify_cpu()+0x6e0: stack state
mismatch: cfa1=4+64 cfa2=4+8
These seemed to disappear when I kept CONFIG_LTO_CLANG_FULL=y but then
disabled CONFIG_X86_KERNEL_IBT. (perhaps due to the way I hacked out
the ibt_selftest_ip label).
Otherwise defconfig and CONFIG_LTO_CLANG_THIN=y both built and booted
in a vm WITHOUT IBT support.
Any idea what's the status of IBT emulation in QEMU, and if it exists,
what's the necessary `-cpu` flag to enable it?
--
Thanks,
~Nick Desaulniers
On Wed, Mar 09, 2022 at 02:02:20AM +0100, Peter Zijlstra wrote:
> On Tue, Mar 08, 2022 at 11:32:37PM +0100, Peter Zijlstra wrote:
> > On Tue, Mar 08, 2022 at 11:01:04PM +0100, Peter Zijlstra wrote:
> > > On Tue, Mar 08, 2022 at 12:00:52PM -0800, Alexei Starovoitov wrote:
> > > > On Tue, Mar 08, 2022 at 04:30:11PM +0100, Peter Zijlstra wrote:
> > > > > Hopefully last posting...
> > > > >
> > > > > Since last time:
> > > > >
> > > > > - updated the ftrace_location() patch (naveen, rostedt)
> > > > > - added a few comments and clarifications (bpetkov)
> > > > > - disable jump-tables (joao)
> > > > > - verified clang-14-rc2 works
> > > > > - fixed a whole bunch of objtool unreachable insn issue
> > > > > - picked up a few more tags
> > > > >
> > > > > Patches go on top of tip/master + arm64/for-next/linkage. Also available here:
> > > > >
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/wip.ibt
> > > >
> > > > I've tried to test it.
> > >
> > > I could cleanly do:
> > >
> > > git checkout tip/master
> > > git merge bpf-next/master
> > > git merge queue/x86/wip.ibt
> > >
> > > You want me to push out that result somewhere?
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
> >
> > includes bpf-next/master.
>
> I just managed to run bpf selftests with that kernel on a tigerlake
> platform. Seems to still work.
Pulled above and it got even worse.
With kasan and lockdep during qemu boot I see:
[ 1.147498] rcu_scheduler_active = 1, debug_locks = 1
[ 1.147498] 2 locks held by kthreadd/2:
[ 1.147498] #0: ffff888100362b80 (&p->pi_lock){....}-{2:2}, at: task_rq_lock+0x71/0x380
[ 1.147498] #1: ffff8881f6a3a218 (&rq->__lock){-...}-{2:2}, at: raw_spin_rq_lock_nested+0x2b/0x40
[ 1.147498]
[ 1.147498] stack backtrace:
[ 1.147498] CPU: 0 PID: 2 Comm: kthreadd Not tainted 5.17.0-rc7-02289-gc958c6aae879 #1
[ 1.147498] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
[ 1.147498] Call Trace:
[ 1.147498] <TASK>
[ 1.147498] dump_stack_lvl+0x48/0x5b
[ 1.147498] cpuacct_charge+0x2b3/0x390
[ 1.147498] update_curr+0x33e/0x7d0
[ 1.147498] dequeue_entity+0x28/0xdf0
[ 1.147498] ? rcu_read_lock_bh_held+0xa0/0xa0
[ 1.147498] dequeue_task_fair+0x1fa/0xd60
[ 1.147498] __do_set_cpus_allowed+0x253/0x620
[ 1.147498] __set_cpus_allowed_ptr_locked+0x25f/0x450
[ 1.147498] __set_cpus_allowed_ptr+0x7c/0xa0
[ 1.147498] ? __set_cpus_allowed_ptr_locked+0x450/0x450
[ 1.147498] ? _raw_spin_unlock_irqrestore+0x34/0x60
[ 1.147498] ? lockdep_hardirqs_on+0x7d/0x100
[ 1.147498] kthreadd+0x48/0x610
[ 1.147498] ? _raw_spin_unlock_irq+0x28/0x50
[ 1.147498] ? kthread_is_per_cpu+0xc0/0xc0
[ 1.147498] ret_from_fork+0x1f/0x30
[ 6.698206] ======================================================
[ 6.698209] WARNING: possible circular locking dependency detected
[ 6.698211] 5.17.0-rc7-02289-gc958c6aae879 #1 Not tainted
[ 6.698213] ------------------------------------------------------
[ 6.698214] scsi_eh_1/401 is trying to acquire lock:
[ 6.698216] ffff888100360900 (&p->pi_lock){-.-.}-{2:2}, at: try_to_wake_up+0xb6/0x1570
[ 6.698241]
[ 6.698241] but task is already holding lock:
[ 6.698241] ffffffff854439f8 ((console_sem).lock){-...}-{2:2}, at: up+0x1b/0xe0
[ 6.698253]
[ 6.698253] which lock already depends on the new lock.
[ 6.698253]
[ 6.698254]
[ 6.698254] the existing dependency chain (in reverse order) is:
[ 6.698255]
[ 6.698255] -> #2 ((console_sem).lock){-...}-{2:2}:
[ 6.698259] _raw_spin_lock_irqsave+0x3c/0x60
[ 6.698270] down_trylock+0x17/0x70
[ 6.698274] __down_trylock_console_sem+0x23/0x90
[ 6.698278] console_trylock+0x17/0x70
[ 6.698281] vprintk_emit+0x72/0x290
[ 6.698288] _printk+0x9a/0xb4
[ 6.698296] lockdep_rcu_suspicious+0x60/0x158
[ 6.698299] cpuacct_charge+0x2b3/0x390
[ 6.698302] update_curr+0x33e/0x7d0
[ 6.698306] dequeue_entity+0x28/0xdf0
[ 6.698308] dequeue_task_fair+0x1fa/0xd60
Most of the time it hangs during the boot.
I'm using gcc 8.5 and qemu -smp 8
With qemu -smp 1 it luckly boots.
Then I run test_progs and see:
Summary: 215/1115 PASSED, 4 SKIPPED, 18 FAILED
All trampoline tests fail.
Here is one:
$ test_progs -t fentry
test_fentry_fexit:PASS:fentry_skel_load 0 nsec
test_fentry_fexit:PASS:fexit_skel_load 0 nsec
test_fentry_fexit:PASS:fentry_attach 0 nsec
test_fentry_fexit:FAIL:fexit_attach unexpected error: -1 (errno 19)
#54 fentry_fexit:FAIL
or
./test_progs -t xdp_bpf
test_xdp_bpf2bpf:PASS:test_xdp__open_and_load 0 nsec
test_xdp_bpf2bpf:PASS:test_xdp_bpf2bpf__open 0 nsec
test_xdp_bpf2bpf:PASS:test_xdp_bpf2bpf__load 0 nsec
libbpf: prog 'trace_on_entry': failed to attach: Device or resource busy
libbpf: prog 'trace_on_entry': failed to auto-attach: -16
test_xdp_bpf2bpf:FAIL:test_xdp_bpf2bpf__attach unexpected error: -16 (errno 16)
#225 xdp_bpf2bpf:FAIL
On Thu, Mar 10, 2022 at 09:22:59AM +0000, David Laight wrote:
> From: Peter Zijlstra
> > Sent: 10 March 2022 09:05
> >
> > On Wed, Mar 09, 2022 at 04:30:28PM -0800, Nick Desaulniers wrote:
> >
> > > I observed the following error when building with
> > > CONFIG_LTO_CLANG_FULL=y enabled:
> > >
> > > ld.lld: error: ld-temp.o <inline asm>:7:2: symbol 'ibt_selftest_ip' is
> > > already defined
> > > ibt_selftest_ip:
> > > ^
> > >
> > > Seems to come from
> > > commit a802350ba65a ("x86/ibt: Add IBT feature, MSR and #CP handling")
> > >
> > > Commenting out the label in the inline asm, I then observed:
> > > vmlinux.o: warning: objtool: identify_cpu()+0x6d0: sibling call from
> > > callable instruction with modified stack frame
> > > vmlinux.o: warning: objtool: identify_cpu()+0x6e0: stack state
> > > mismatch: cfa1=4+64 cfa2=4+8
> > > These seemed to disappear when I kept CONFIG_LTO_CLANG_FULL=y but then
> > > disabled CONFIG_X86_KERNEL_IBT. (perhaps due to the way I hacked out
> > > the ibt_selftest_ip label).
> >
> > Urgh.. I'm thikning this is a clang bug :/
> >
> > The code in question is:
> >
> >
> > void ibt_selftest_ip(void); /* code label defined in asm below */
> >
> > DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
> > {
> > /* ... */
> >
> > if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
> > regs->ax = 0;
> > return;
> > }
> >
> > /* ... */
> > }
> >
> > bool ibt_selftest(void)
> > {
> > unsigned long ret;
> >
> > asm (" lea ibt_selftest_ip(%%rip), %%rax\n\t"
> > ANNOTATE_RETPOLINE_SAFE
> > " jmp *%%rax\n\t"
> > "ibt_selftest_ip:\n\t"
> > UNWIND_HINT_FUNC
> > ANNOTATE_NOENDBR
> > " nop\n\t"
> >
> > : "=a" (ret) : : "memory");
> >
> > return !ret;
> > }
> >
> > There is only a single definition of that symbol, the one in the asm.
> > The other is a declaration, which is used in the exception handler to
> > compare against regs->ip.
>
> LTO has probably inlined it twice.
Indeed, adding noinline to ibt_selftest() makes it work.
---
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index d8bbc705efe5..0c737cc31ee5 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -781,7 +781,8 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
return NOTIFY_STOP;
}
-static void __init int3_selftest(void)
+/* Must be noinline to ensure uniqueness of int3_selftest_ip. */
+static noinline void __init int3_selftest(void)
{
static __initdata struct notifier_block int3_exception_nb = {
.notifier_call = int3_exception_notify,
@@ -794,9 +795,8 @@ static void __init int3_selftest(void)
/*
* Basically: int3_magic(&val); but really complicated :-)
*
- * Stick the address of the INT3 instruction into int3_selftest_ip,
- * then trigger the INT3, padded with NOPs to match a CALL instruction
- * length.
+ * INT3 padded with NOP to CALL_INSN_SIZE. The int3_exception_nb
+ * notifier above will emulate CALL for us.
*/
asm volatile ("int3_selftest_ip:\n\t"
ANNOTATE_NOENDBR
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 837cc3c7d4f4..fb89a2f1011f 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -214,7 +214,7 @@ DEFINE_IDTENTRY(exc_overflow)
static __ro_after_init bool ibt_fatal = true;
-void ibt_selftest_ip(void); /* code label defined in asm below */
+extern void ibt_selftest_ip(void); /* code label defined in asm below */
enum cp_error_code {
CP_EC = (1 << 15) - 1,
@@ -238,7 +238,7 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
if (WARN_ON_ONCE(user_mode(regs) || (error_code & CP_EC) != CP_ENDBR))
return;
- if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
+ if (unlikely(regs->ip == (unsigned long)&ibt_selftest_ip)) {
regs->ax = 0;
return;
}
@@ -252,7 +252,8 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
BUG();
}
-bool ibt_selftest(void)
+/* Must be noinline to ensure uniqueness of ibt_selftest_ip. */
+noinline bool ibt_selftest(void)
{
unsigned long ret;
On Wed, Mar 09, 2022 at 11:09:17AM -0800, Alexei Starovoitov wrote:
> Pulled above and it got even worse.
> With kasan and lockdep during qemu boot I see:
> [ 1.147498] rcu_scheduler_active = 1, debug_locks = 1
> [ 1.147498] 2 locks held by kthreadd/2:
> [ 1.147498] #0: ffff888100362b80 (&p->pi_lock){....}-{2:2}, at: task_rq_lock+0x71/0x380
> [ 1.147498] #1: ffff8881f6a3a218 (&rq->__lock){-...}-{2:2}, at: raw_spin_rq_lock_nested+0x2b/0x40
> [ 1.147498]
> [ 1.147498] stack backtrace:
> [ 1.147498] CPU: 0 PID: 2 Comm: kthreadd Not tainted 5.17.0-rc7-02289-gc958c6aae879 #1
> [ 1.147498] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> [ 1.147498] Call Trace:
> [ 1.147498] <TASK>
> [ 1.147498] dump_stack_lvl+0x48/0x5b
> [ 1.147498] cpuacct_charge+0x2b3/0x390
> [ 1.147498] update_curr+0x33e/0x7d0
> [ 1.147498] dequeue_entity+0x28/0xdf0
> [ 1.147498] ? rcu_read_lock_bh_held+0xa0/0xa0
> [ 1.147498] dequeue_task_fair+0x1fa/0xd60
> [ 1.147498] __do_set_cpus_allowed+0x253/0x620
> [ 1.147498] __set_cpus_allowed_ptr_locked+0x25f/0x450
> [ 1.147498] __set_cpus_allowed_ptr+0x7c/0xa0
> [ 1.147498] ? __set_cpus_allowed_ptr_locked+0x450/0x450
> [ 1.147498] ? _raw_spin_unlock_irqrestore+0x34/0x60
> [ 1.147498] ? lockdep_hardirqs_on+0x7d/0x100
> [ 1.147498] kthreadd+0x48/0x610
> [ 1.147498] ? _raw_spin_unlock_irq+0x28/0x50
> [ 1.147498] ? kthread_is_per_cpu+0xc0/0xc0
> [ 1.147498] ret_from_fork+0x1f/0x30
Yeah, sorry about that, currently arguing with Paul about that one.
Should go away if you disable the RCU lockdep thing. The warning itself
is a false positive and more harmful than anything else due to it
generating a possible printk deadlock.
> Most of the time it hangs during the boot.
> I'm using gcc 8.5 and qemu -smp 8
> With qemu -smp 1 it luckly boots.
> Then I run test_progs and see:
> Summary: 215/1115 PASSED, 4 SKIPPED, 18 FAILED
> All trampoline tests fail.
> Here is one:
> $ test_progs -t fentry
> test_fentry_fexit:PASS:fentry_skel_load 0 nsec
> test_fentry_fexit:PASS:fexit_skel_load 0 nsec
> test_fentry_fexit:PASS:fentry_attach 0 nsec
> test_fentry_fexit:FAIL:fexit_attach unexpected error: -1 (errno 19)
> #54 fentry_fexit:FAIL
>
> or
>
> ./test_progs -t xdp_bpf
> test_xdp_bpf2bpf:PASS:test_xdp__open_and_load 0 nsec
> test_xdp_bpf2bpf:PASS:test_xdp_bpf2bpf__open 0 nsec
> test_xdp_bpf2bpf:PASS:test_xdp_bpf2bpf__load 0 nsec
> libbpf: prog 'trace_on_entry': failed to attach: Device or resource busy
> libbpf: prog 'trace_on_entry': failed to auto-attach: -16
> test_xdp_bpf2bpf:FAIL:test_xdp_bpf2bpf__attach unexpected error: -16 (errno 16)
> #225 xdp_bpf2bpf:FAIL
Urgh.. I totally missed that in flood of output. Let me go try and
figure out what's happening there.
On Wed, Mar 09, 2022 at 04:30:28PM -0800, Nick Desaulniers wrote:
> I observed the following error when building with
> CONFIG_LTO_CLANG_FULL=y enabled:
>
> ld.lld: error: ld-temp.o <inline asm>:7:2: symbol 'ibt_selftest_ip' is
> already defined
> ibt_selftest_ip:
> ^
>
> Seems to come from
> commit a802350ba65a ("x86/ibt: Add IBT feature, MSR and #CP handling")
>
> Commenting out the label in the inline asm, I then observed:
> vmlinux.o: warning: objtool: identify_cpu()+0x6d0: sibling call from
> callable instruction with modified stack frame
> vmlinux.o: warning: objtool: identify_cpu()+0x6e0: stack state
> mismatch: cfa1=4+64 cfa2=4+8
> These seemed to disappear when I kept CONFIG_LTO_CLANG_FULL=y but then
> disabled CONFIG_X86_KERNEL_IBT. (perhaps due to the way I hacked out
> the ibt_selftest_ip label).
Urgh.. I'm thikning this is a clang bug :/
The code in question is:
void ibt_selftest_ip(void); /* code label defined in asm below */
DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
{
/* ... */
if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
regs->ax = 0;
return;
}
/* ... */
}
bool ibt_selftest(void)
{
unsigned long ret;
asm (" lea ibt_selftest_ip(%%rip), %%rax\n\t"
ANNOTATE_RETPOLINE_SAFE
" jmp *%%rax\n\t"
"ibt_selftest_ip:\n\t"
UNWIND_HINT_FUNC
ANNOTATE_NOENDBR
" nop\n\t"
: "=a" (ret) : : "memory");
return !ret;
}
There is only a single definition of that symbol, the one in the asm.
The other is a declaration, which is used in the exception handler to
compare against regs->ip.
So what this code does is trigger an explicit #CP and special case that
in the handler. For that the handler needs to know the special IP that
will trigger the failure, this is cummunicated with that symbol.
> Otherwise defconfig and CONFIG_LTO_CLANG_THIN=y both built and booted
> in a vm WITHOUT IBT support.
>
> Any idea what's the status of IBT emulation in QEMU, and if it exists,
> what's the necessary `-cpu` flag to enable it?
I have a very ugly kvm patch that goes with a very ugly qemu patch to
make it work. I would very much not recommend those getting merged.
Someone with some actual kvm/qemu foo should do one. The complicating
factor is that IA32_S_CET also contains SHSTK enable bits, so a straight
passthrough like I use relies on the guest never setting those bits or
keeping the pieces. It either needs to filter the MSR or implement the
full CET mess.
On Wed, Mar 09, 2022 at 01:37:50PM +0000, David Laight wrote:
> I'm wondering what actually happens to loadable modules?
They work just fine..
> Especially those built 'out of tree',
> potentially with a different compiler,
> and maybe containing binary 'blobs'.
-EDONTCARE, you're on your own and get to keep whatever pieces.
In fact, the more pieces I can get you, the better I feel about it.
> The requirement to run programs on old distributions means
> that things get compiled with quite old versions of gcc.
> For instance RHEL7 is gcc 4.8.5.
Min GCC is 5.1. If you want Kernel IBT you get to use GCC-9 (or 8 with
backports) but I'd recommend using GCC-10 or later since before that the
IBT code-gen is pretty stupid.
If you build modules using another compiler than the main kernel, you
again get to keep the pieces.
From: Peter Zijlstra
> Sent: 10 March 2022 09:05
>
> On Wed, Mar 09, 2022 at 04:30:28PM -0800, Nick Desaulniers wrote:
>
> > I observed the following error when building with
> > CONFIG_LTO_CLANG_FULL=y enabled:
> >
> > ld.lld: error: ld-temp.o <inline asm>:7:2: symbol 'ibt_selftest_ip' is
> > already defined
> > ibt_selftest_ip:
> > ^
> >
> > Seems to come from
> > commit a802350ba65a ("x86/ibt: Add IBT feature, MSR and #CP handling")
> >
> > Commenting out the label in the inline asm, I then observed:
> > vmlinux.o: warning: objtool: identify_cpu()+0x6d0: sibling call from
> > callable instruction with modified stack frame
> > vmlinux.o: warning: objtool: identify_cpu()+0x6e0: stack state
> > mismatch: cfa1=4+64 cfa2=4+8
> > These seemed to disappear when I kept CONFIG_LTO_CLANG_FULL=y but then
> > disabled CONFIG_X86_KERNEL_IBT. (perhaps due to the way I hacked out
> > the ibt_selftest_ip label).
>
> Urgh.. I'm thikning this is a clang bug :/
>
> The code in question is:
>
>
> void ibt_selftest_ip(void); /* code label defined in asm below */
>
> DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
> {
> /* ... */
>
> if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
> regs->ax = 0;
> return;
> }
>
> /* ... */
> }
>
> bool ibt_selftest(void)
> {
> unsigned long ret;
>
> asm (" lea ibt_selftest_ip(%%rip), %%rax\n\t"
> ANNOTATE_RETPOLINE_SAFE
> " jmp *%%rax\n\t"
> "ibt_selftest_ip:\n\t"
> UNWIND_HINT_FUNC
> ANNOTATE_NOENDBR
> " nop\n\t"
>
> : "=a" (ret) : : "memory");
>
> return !ret;
> }
>
> There is only a single definition of that symbol, the one in the asm.
> The other is a declaration, which is used in the exception handler to
> compare against regs->ip.
LTO has probably inlined it twice.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Thu, Mar 10, 2022 at 02:47:18PM +0100, Peter Zijlstra wrote:
> On Thu, Mar 10, 2022 at 10:35:32AM +0100, Peter Zijlstra wrote:
>
> > > $ test_progs -t fentry
> > > test_fentry_fexit:PASS:fentry_skel_load 0 nsec
> > > test_fentry_fexit:PASS:fexit_skel_load 0 nsec
> > > test_fentry_fexit:PASS:fentry_attach 0 nsec
> > > test_fentry_fexit:FAIL:fexit_attach unexpected error: -1 (errno 19)
> > > #54 fentry_fexit:FAIL
>
> This seems to cure the above. fexit_bpf2bpf is still failing, I'll dig
> into that after lunch.
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index acb50fb7ed2d..2d86d3c09d64 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5354,6 +5381,11 @@ int modify_ftrace_direct(unsigned long ip,
> mutex_lock(&direct_mutex);
>
> mutex_lock(&ftrace_lock);
> +
> + ip = ftrace_location(ip);
> + if (!ip)
> + goto out_unlock;
> +
> entry = find_direct_entry(&ip, &rec);
> if (!entry)
> goto out_unlock;
This seems to cure most of the rest. I'm still seeing one failure:
libbpf: prog 'connect_v4_prog': BPF program load failed: Invalid argument
libbpf: failed to load program 'connect_v4_prog'
libbpf: failed to load object './connect4_prog.o'
test_fexit_bpf2bpf_common:FAIL:tgt_prog_load unexpected error: -22 (errno 22)
#48/4 fexit_bpf2bpf/func_replace_verify:FAIL
but the rest seems to be good again.
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 2b1e266ff95c..3fecfe8c4429 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -384,6 +395,13 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
/* BPF poking in modules is not supported */
return -EINVAL;
+ /*
+ * See emit_prologue(), for IBT builds the trampoline hook is preceded
+ * with an ENDBR instruction.
+ */
+ if (is_endbr(*(u32 *)ip))
+ ip += ENDBR_INSN_SIZE;
+
return __bpf_arch_text_poke(ip, t, old_addr, new_addr, true);
}
On Thu, Mar 10, 2022 at 10:35:32AM +0100, Peter Zijlstra wrote:
> > $ test_progs -t fentry
> > test_fentry_fexit:PASS:fentry_skel_load 0 nsec
> > test_fentry_fexit:PASS:fexit_skel_load 0 nsec
> > test_fentry_fexit:PASS:fentry_attach 0 nsec
> > test_fentry_fexit:FAIL:fexit_attach unexpected error: -1 (errno 19)
> > #54 fentry_fexit:FAIL
This seems to cure the above. fexit_bpf2bpf is still failing, I'll dig
into that after lunch.
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index acb50fb7ed2d..2d86d3c09d64 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5354,6 +5381,11 @@ int modify_ftrace_direct(unsigned long ip,
mutex_lock(&direct_mutex);
mutex_lock(&ftrace_lock);
+
+ ip = ftrace_location(ip);
+ if (!ip)
+ goto out_unlock;
+
entry = find_direct_entry(&ip, &rec);
if (!entry)
goto out_unlock;
On Thu, Mar 10, 2022 at 2:16 AM Peter Zijlstra <[email protected]> wrote:
>
> On Thu, Mar 10, 2022 at 09:22:59AM +0000, David Laight wrote:
> > From: Peter Zijlstra
> > > Sent: 10 March 2022 09:05
> > >
> > > On Wed, Mar 09, 2022 at 04:30:28PM -0800, Nick Desaulniers wrote:
> > >
> > > > I observed the following error when building with
> > > > CONFIG_LTO_CLANG_FULL=y enabled:
> > > >
> > > > ld.lld: error: ld-temp.o <inline asm>:7:2: symbol 'ibt_selftest_ip' is
> > > > already defined
> > > > ibt_selftest_ip:
> > > > ^
> > > >
> > > > Seems to come from
> > > > commit a802350ba65a ("x86/ibt: Add IBT feature, MSR and #CP handling")
> > > >
> > > > Commenting out the label in the inline asm, I then observed:
> > > > vmlinux.o: warning: objtool: identify_cpu()+0x6d0: sibling call from
> > > > callable instruction with modified stack frame
> > > > vmlinux.o: warning: objtool: identify_cpu()+0x6e0: stack state
> > > > mismatch: cfa1=4+64 cfa2=4+8
> > > > These seemed to disappear when I kept CONFIG_LTO_CLANG_FULL=y but then
> > > > disabled CONFIG_X86_KERNEL_IBT. (perhaps due to the way I hacked out
> > > > the ibt_selftest_ip label).
> > >
> > LTO has probably inlined it twice.
>
> Indeed, adding noinline to ibt_selftest() makes it work.
Yep, that LGTM. If you end up sticking that as a patch on top:
Reported-by: Nick Desaulniers <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
For the kernel IBT series @ v4 plus this diff:
Tested-by: Nick Desaulniers <[email protected]> # llvm build, non-IBT boot
>
>
> ---
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index d8bbc705efe5..0c737cc31ee5 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -781,7 +781,8 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
> return NOTIFY_STOP;
> }
>
> -static void __init int3_selftest(void)
> +/* Must be noinline to ensure uniqueness of int3_selftest_ip. */
> +static noinline void __init int3_selftest(void)
> {
> static __initdata struct notifier_block int3_exception_nb = {
> .notifier_call = int3_exception_notify,
> @@ -794,9 +795,8 @@ static void __init int3_selftest(void)
> /*
> * Basically: int3_magic(&val); but really complicated :-)
> *
> - * Stick the address of the INT3 instruction into int3_selftest_ip,
> - * then trigger the INT3, padded with NOPs to match a CALL instruction
> - * length.
> + * INT3 padded with NOP to CALL_INSN_SIZE. The int3_exception_nb
> + * notifier above will emulate CALL for us.
> */
> asm volatile ("int3_selftest_ip:\n\t"
> ANNOTATE_NOENDBR
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 837cc3c7d4f4..fb89a2f1011f 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -214,7 +214,7 @@ DEFINE_IDTENTRY(exc_overflow)
>
> static __ro_after_init bool ibt_fatal = true;
>
> -void ibt_selftest_ip(void); /* code label defined in asm below */
> +extern void ibt_selftest_ip(void); /* code label defined in asm below */
>
> enum cp_error_code {
> CP_EC = (1 << 15) - 1,
> @@ -238,7 +238,7 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
> if (WARN_ON_ONCE(user_mode(regs) || (error_code & CP_EC) != CP_ENDBR))
> return;
>
> - if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
> + if (unlikely(regs->ip == (unsigned long)&ibt_selftest_ip)) {
(Though adding the address of operator & to the function name in the
comparisons isn't strictly necessary; functions used in expressions
"decay" into function pointers; I guess the standard calls these
"function designators." I see that's been added to be consistent
between the two...See 6.3.2.1.4 of
http://open-std.org/jtc1/sc22/wg14/www/docs/n2731.pdf pdf page
62/printed page 46.)
> regs->ax = 0;
> return;
> }
> @@ -252,7 +252,8 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
> BUG();
> }
>
> -bool ibt_selftest(void)
> +/* Must be noinline to ensure uniqueness of ibt_selftest_ip. */
> +noinline bool ibt_selftest(void)
> {
> unsigned long ret;
>
--
Thanks,
~Nick Desaulniers
On Thu, 10 Mar 2022 14:47:18 +0100
Peter Zijlstra <[email protected]> wrote:
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index acb50fb7ed2d..2d86d3c09d64 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5354,6 +5381,11 @@ int modify_ftrace_direct(unsigned long ip,
> mutex_lock(&direct_mutex);
>
> mutex_lock(&ftrace_lock);
> +
> + ip = ftrace_location(ip);
> + if (!ip)
> + goto out_unlock;
> +
Perhaps this should go into find_direct_entry() instead, as I think you are
adding it before all the find_direct_entry() callers.
And find_direct_entry will update the ip.
-- Steve
> entry = find_direct_entry(&ip, &rec);
> if (!entry)
> goto out_unlock;
On Thu, Mar 10, 2022 at 05:29:11PM +0100, Peter Zijlstra wrote:
> This seems to cure most of the rest. I'm still seeing one failure:
>
> libbpf: prog 'connect_v4_prog': BPF program load failed: Invalid argument
> libbpf: failed to load program 'connect_v4_prog'
> libbpf: failed to load object './connect4_prog.o'
> test_fexit_bpf2bpf_common:FAIL:tgt_prog_load unexpected error: -22 (errno 22)
> #48/4 fexit_bpf2bpf/func_replace_verify:FAIL
Hmm, with those two patches on I get:
root@tigerlake:/usr/src/linux-2.6/tgl-build# ./test_progs -t fexit
#46 fentry_fexit:OK
#48 fexit_bpf2bpf:OK
#49 fexit_sleep:OK
#50 fexit_stress:OK
#51 fexit_test:OK
Summary: 5/9 PASSED, 0 SKIPPED, 0 FAILED
On the tigerlake, I suppose I'm doing something wrong on the other
machine because there it's even failing on the pre-ibt kernel image.
I'll go write up changelogs and stick these on.
The following commit has been merged into the x86/core branch of tip:
Commit-ID: c7d90e15b8950009d0d4e8f3503b09b2ea6d527c
Gitweb: https://git.kernel.org/tip/c7d90e15b8950009d0d4e8f3503b09b2ea6d527c
Author: Peter Zijlstra <[email protected]>
AuthorDate: Thu, 10 Mar 2022 11:16:03 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 11 Mar 2022 13:05:08 +01:00
x86: Fix {int3,ibt}_selftest() vs LTO
Both these selftests define a symbol in inline asm which goes
side-ways if the asm gets duplicated due to inlining. Nick actually
saw this happen with a Clang LTO build.
Mark the two selftests noinline to ensure this cannot happen, as
suggestd by David.
While there, update the comment for int3_selftest() and increase coding
style consistency between the two.
Fixes: 103c0093ceb6 ("x86/ibt: Add IBT feature, MSR and #CP handling")
Reported-by: Nick Desaulniers <[email protected]>
Suggested-by: David Laight <[email protected]>,
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]> # llvm build, non-IBT boot
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/alternative.c | 8 ++++----
arch/x86/kernel/traps.c | 7 ++++---
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index d6c41f8..820c43a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -781,7 +781,8 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
return NOTIFY_STOP;
}
-static void __init int3_selftest(void)
+/* Must be noinline to ensure uniqueness of int3_selftest_ip. */
+static noinline void __init int3_selftest(void)
{
static __initdata struct notifier_block int3_exception_nb = {
.notifier_call = int3_exception_notify,
@@ -794,9 +795,8 @@ static void __init int3_selftest(void)
/*
* Basically: int3_magic(&val); but really complicated :-)
*
- * Stick the address of the INT3 instruction into int3_selftest_ip,
- * then trigger the INT3, padded with NOPs to match a CALL instruction
- * length.
+ * INT3 padded with NOP to CALL_INSN_SIZE. The int3_exception_nb
+ * notifier above will emulate CALL for us.
*/
asm volatile ("int3_selftest_ip:\n\t"
ANNOTATE_NOENDBR
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 89fb299..755c23b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -213,7 +213,7 @@ DEFINE_IDTENTRY(exc_overflow)
static __ro_after_init bool ibt_fatal = true;
-void ibt_selftest_ip(void); /* code label defined in asm below */
+extern void ibt_selftest_ip(void); /* code label defined in asm below */
enum cp_error_code {
CP_EC = (1 << 15) - 1,
@@ -237,7 +237,7 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
if (WARN_ON_ONCE(user_mode(regs) || (error_code & CP_EC) != CP_ENDBR))
return;
- if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
+ if (unlikely(regs->ip == (unsigned long)&ibt_selftest_ip)) {
regs->ax = 0;
return;
}
@@ -251,7 +251,8 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
BUG();
}
-bool ibt_selftest(void)
+/* Must be noinline to ensure uniqueness of ibt_selftest_ip. */
+noinline bool ibt_selftest(void)
{
unsigned long ret;
On Fri, Mar 11, 2022 at 2:40 AM Peter Zijlstra <[email protected]> wrote:
>
> On Thu, Mar 10, 2022 at 05:29:11PM +0100, Peter Zijlstra wrote:
>
> > This seems to cure most of the rest. I'm still seeing one failure:
> >
> > libbpf: prog 'connect_v4_prog': BPF program load failed: Invalid argument
> > libbpf: failed to load program 'connect_v4_prog'
> > libbpf: failed to load object './connect4_prog.o'
> > test_fexit_bpf2bpf_common:FAIL:tgt_prog_load unexpected error: -22 (errno 22)
> > #48/4 fexit_bpf2bpf/func_replace_verify:FAIL
>
>
> Hmm, with those two patches on I get:
>
> root@tigerlake:/usr/src/linux-2.6/tgl-build# ./test_progs -t fexit
> #46 fentry_fexit:OK
> #48 fexit_bpf2bpf:OK
> #49 fexit_sleep:OK
> #50 fexit_stress:OK
> #51 fexit_test:OK
> Summary: 5/9 PASSED, 0 SKIPPED, 0 FAILED
>
> On the tigerlake, I suppose I'm doing something wrong on the other
> machine because there it's even failing on the pre-ibt kernel image.
>
> I'll go write up changelogs and stick these on.
What is the latest branch I can use to test it?
On Thu, Mar 10, 2022 at 09:37:31AM -0500, Steven Rostedt wrote:
> On Thu, 10 Mar 2022 14:47:18 +0100
> Peter Zijlstra <[email protected]> wrote:
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index acb50fb7ed2d..2d86d3c09d64 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -5354,6 +5381,11 @@ int modify_ftrace_direct(unsigned long ip,
> > mutex_lock(&direct_mutex);
> >
> > mutex_lock(&ftrace_lock);
> > +
> > + ip = ftrace_location(ip);
> > + if (!ip)
> > + goto out_unlock;
> > +
>
> Perhaps this should go into find_direct_entry() instead, as I think you are
> adding it before all the find_direct_entry() callers.
Something like so then?
Index: linux-2.6/kernel/trace/ftrace.c
===================================================================
--- linux-2.6.orig/kernel/trace/ftrace.c
+++ linux-2.6/kernel/trace/ftrace.c
@@ -1575,7 +1575,7 @@ unsigned long ftrace_location_range(unsi
* If @ip matches sym+0, return sym's ftrace location.
* Otherwise, return 0.
*/
-unsigned long ftrace_location(unsigned long ip)
+unsigned long __ftrace_location(unsigned long ip, struct dyn_ftrace **recp)
{
struct dyn_ftrace *rec;
unsigned long offset;
@@ -1591,13 +1591,22 @@ unsigned long ftrace_location(unsigned l
rec = lookup_rec(ip, ip + size - 1);
}
- if (rec)
+ if (rec) {
+ if (recp)
+ *recp = rec;
+
return rec->ip;
+ }
out:
return 0;
}
+unsigned long ftrace_location(unsigned long ip)
+{
+ return __ftrace_location(ip, NULL);
+}
+
/**
* ftrace_text_reserved - return true if range contains an ftrace location
* @start: start of range to search
@@ -2392,6 +2401,30 @@ static struct ftrace_hash *direct_functi
static DEFINE_MUTEX(direct_mutex);
int ftrace_direct_func_count;
+static struct ftrace_func_entry *
+find_direct_entry(unsigned long *ip, struct dyn_ftrace **recp, bool warn)
+{
+ struct ftrace_func_entry *entry;
+ struct dyn_ftrace *rec = NULL;
+
+ *ip = __ftrace_location(*ip, &rec);
+ if (!*ip)
+ return NULL;
+
+ if (recp)
+ *recp = rec;
+
+ entry = __ftrace_lookup_ip(direct_functions, *ip);
+ if (!entry) {
+ WARN_ON(rec->flags & FTRACE_FL_DIRECT);
+ return NULL;
+ }
+
+ WARN_ON(warn && !(rec->flags & FTRACE_FL_DIRECT));
+
+ return entry;
+}
+
/*
* Search the direct_functions hash to see if the given instruction pointer
* has a direct caller attached to it.
@@ -2400,7 +2433,7 @@ unsigned long ftrace_find_rec_direct(uns
{
struct ftrace_func_entry *entry;
- entry = __ftrace_lookup_ip(direct_functions, ip);
+ entry = find_direct_entry(&ip, NULL, false);
if (!entry)
return 0;
@@ -5127,40 +5160,19 @@ int register_ftrace_direct(unsigned long
struct ftrace_direct_func *direct;
struct ftrace_func_entry *entry;
struct ftrace_hash *free_hash = NULL;
- struct dyn_ftrace *rec;
+ struct dyn_ftrace *rec = NULL;
int ret = -ENODEV;
mutex_lock(&direct_mutex);
- ip = ftrace_location(ip);
- if (!ip)
+ entry = find_direct_entry(&ip, &rec, true);
+ if (!ip || !rec)
goto out_unlock;
- /* See if there's a direct function at @ip already */
ret = -EBUSY;
- if (ftrace_find_rec_direct(ip))
- goto out_unlock;
-
- ret = -ENODEV;
- rec = lookup_rec(ip, ip);
- if (!rec)
+ if (entry && entry->direct)
goto out_unlock;
- /*
- * Check if the rec says it has a direct call but we didn't
- * find one earlier?
- */
- if (WARN_ON(rec->flags & FTRACE_FL_DIRECT))
- goto out_unlock;
-
- /* Make sure the ip points to the exact record */
- if (ip != rec->ip) {
- ip = rec->ip;
- /* Need to check this ip for a direct. */
- if (ftrace_find_rec_direct(ip))
- goto out_unlock;
- }
-
ret = -ENOMEM;
direct = ftrace_find_direct_func(addr);
if (!direct) {
@@ -5209,33 +5221,6 @@ int register_ftrace_direct(unsigned long
}
EXPORT_SYMBOL_GPL(register_ftrace_direct);
-static struct ftrace_func_entry *find_direct_entry(unsigned long *ip,
- struct dyn_ftrace **recp)
-{
- struct ftrace_func_entry *entry;
- struct dyn_ftrace *rec;
-
- rec = lookup_rec(*ip, *ip);
- if (!rec)
- return NULL;
-
- entry = __ftrace_lookup_ip(direct_functions, rec->ip);
- if (!entry) {
- WARN_ON(rec->flags & FTRACE_FL_DIRECT);
- return NULL;
- }
-
- WARN_ON(!(rec->flags & FTRACE_FL_DIRECT));
-
- /* Passed in ip just needs to be on the call site */
- *ip = rec->ip;
-
- if (recp)
- *recp = rec;
-
- return entry;
-}
-
int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
{
struct ftrace_direct_func *direct;
@@ -5245,11 +5230,7 @@ int unregister_ftrace_direct(unsigned lo
mutex_lock(&direct_mutex);
- ip = ftrace_location(ip);
- if (!ip)
- goto out_unlock;
-
- entry = find_direct_entry(&ip, NULL);
+ entry = find_direct_entry(&ip, NULL, true);
if (!entry)
goto out_unlock;
@@ -5382,11 +5363,7 @@ int modify_ftrace_direct(unsigned long i
mutex_lock(&ftrace_lock);
- ip = ftrace_location(ip);
- if (!ip)
- goto out_unlock;
-
- entry = find_direct_entry(&ip, &rec);
+ entry = find_direct_entry(&ip, &rec, true);
if (!entry)
goto out_unlock;
The following commit has been merged into the x86/core branch of tip:
Commit-ID: c841668784cc609e7ae103d91c3e03bf8939418d
Gitweb: https://git.kernel.org/tip/c841668784cc609e7ae103d91c3e03bf8939418d
Author: Peter Zijlstra <[email protected]>
AuthorDate: Fri, 11 Mar 2022 10:40:40 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 11 Mar 2022 13:05:08 +01:00
x86,ftrace: Fix modify_ftrace_direct()
Alexei reported that BPF direct trampolines are no longer working with
IBT=y builds.
Make modify_ftrace_direct() consistent vs
{,un}register_ftrace_direct(), such that they all agree on where the
__fentry__ site lives.
Fixes: ee1a8cf8dd0f ("x86/ibt,ftrace: Search for __fentry__ location")
Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/trace/ftrace.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ae0d9f6..8e0509e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5381,6 +5381,11 @@ int modify_ftrace_direct(unsigned long ip,
mutex_lock(&direct_mutex);
mutex_lock(&ftrace_lock);
+
+ ip = ftrace_location(ip);
+ if (!ip)
+ goto out_unlock;
+
entry = find_direct_entry(&ip, &rec);
if (!entry)
goto out_unlock;
On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> During the build with gcc 8.5 I see:
>
> arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> .ibt_endbr_seal, skipping
> arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> .orc_unwind section, skipping
> LD [M] crypto/async_tx/async_xor.ko
> LD [M] crypto/authenc.ko
> make[3]: *** [../scripts/Makefile.modfinal:61:
> arch/x86/crypto/crc32c-intel.ko] Error 255
> make[3]: *** Waiting for unfinished jobs....
>
> but make clean cures it.
> I suspect it's some missing makefile dependency.
Yes, I recently ran into it; I've been trying to kick Makefile into
submission but have not had success yet. Will try again on Monday.
Problem appears to be that it will re-link .ko even though .o hasn't
changed, resulting in duplicate objtool runs. I've been trying to have
makefile generate .o.objtool empty file to serve as dependency marker to
avoid doing second objtool run, but like said, no luck yet.
> and:
> vmlinux.o: warning: objtool: ksys_unshare()+0x626: unreachable instruction
> which stays even after make clean.
Humm, I shall have to dig out gcc-8.5 then.
> The rcu "false positive" is still there that causes
> sporadic hangs during the boot.
I've merged fix for that yesterday, shall respin this ibt tree to
include that.
> The test_progs shows:
> Summary: 228/1122 PASSED, 4 SKIPPED, 6 FAILED
> (when I remove one test)
>
> That test is actually crashing the kernel:
> ./test_progs -t mod_race
Argh, I wasn't seeing crashing, I'll prod with sharp stick.
On Sat, Mar 12, 2022 at 7:44 AM Peter Zijlstra <[email protected]> wrote:
>
> On Fri, Mar 11, 2022 at 09:09:38AM -0800, Alexei Starovoitov wrote:
> > On Fri, Mar 11, 2022 at 2:40 AM Peter Zijlstra <[email protected]> wrote:
> > >
> > > On Thu, Mar 10, 2022 at 05:29:11PM +0100, Peter Zijlstra wrote:
> > >
> > > > This seems to cure most of the rest. I'm still seeing one failure:
> > > >
> > > > libbpf: prog 'connect_v4_prog': BPF program load failed: Invalid argument
> > > > libbpf: failed to load program 'connect_v4_prog'
> > > > libbpf: failed to load object './connect4_prog.o'
> > > > test_fexit_bpf2bpf_common:FAIL:tgt_prog_load unexpected error: -22 (errno 22)
> > > > #48/4 fexit_bpf2bpf/func_replace_verify:FAIL
> > >
> > >
> > > Hmm, with those two patches on I get:
> > >
> > > root@tigerlake:/usr/src/linux-2.6/tgl-build# ./test_progs -t fexit
> > > #46 fentry_fexit:OK
> > > #48 fexit_bpf2bpf:OK
> > > #49 fexit_sleep:OK
> > > #50 fexit_stress:OK
> > > #51 fexit_test:OK
> > > Summary: 5/9 PASSED, 0 SKIPPED, 0 FAILED
> > >
> > > On the tigerlake, I suppose I'm doing something wrong on the other
> > > machine because there it's even failing on the pre-ibt kernel image.
> > >
> > > I'll go write up changelogs and stick these on.
> >
> > What is the latest branch I can use to test it?
>
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
>
> that also include bpf-next. Thanks!
Looks better.
During the build with gcc 8.5 I see:
arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
.ibt_endbr_seal, skipping
arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
.orc_unwind section, skipping
LD [M] crypto/async_tx/async_xor.ko
LD [M] crypto/authenc.ko
make[3]: *** [../scripts/Makefile.modfinal:61:
arch/x86/crypto/crc32c-intel.ko] Error 255
make[3]: *** Waiting for unfinished jobs....
but make clean cures it.
I suspect it's some missing makefile dependency.
and:
vmlinux.o: warning: objtool: ksys_unshare()+0x626: unreachable instruction
which stays even after make clean.
The rcu "false positive" is still there that causes
sporadic hangs during the boot.
The test_progs shows:
Summary: 228/1122 PASSED, 4 SKIPPED, 6 FAILED
(when I remove one test)
That test is actually crashing the kernel:
./test_progs -t mod_race
[ 39.202593] bpf_testmod: loading out-of-tree module taints kernel.
[ 39.303142] general protection fault, probably for non-canonical
address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN PTI
[ 39.304610] KASAN: null-ptr-deref in range
[0x0000000000000000-0x0000000000000007]
[ 39.305514] CPU: 9 PID: 1599 Comm: test_progs Tainted: G
O 5.17.0-rc7-02525-g5dd5efb53cf1 #4
[ 39.306675] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
[ 39.308036] RIP: 0010:do_init_module+0x9/0x6f0
[ 39.308583] Code: fe ff ff e8 59 13 46 00 e9 7f fe ff ff e8 4f 13
46 00 e9 49 fe ff ff 66 2e 0f 1f 84 00 00 00 00 00 e8 cb 8d eb 1e 48
b8 00 00 <00> 00 00 fc ff df 41 57 49 89 ff 48 c7 c7 20 f6 5c 84 48 89
fa 41
[ 39.310815] RSP: 0018:ffff88810f7e7aa0 EFLAGS: 00010282
[ 39.311450] RAX: dffffc0000000000 RBX: 0000000000000009 RCX: ffffffff81283b16
[ 39.312253] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffffffffa0224c00
[ 39.313031] RBP: ffff88810f7e7ac8 R08: 0000000000000000 R09: fffffbfff0b4d557
[ 39.313813] R10: ffffffff85a6aab7 R11: fffffbfff0b4d556 R12: ffff88811171f518
[ 39.314591] R13: dffffc0000000000 R14: ffffffffa0224c00 R15: ffff88810f7e7e50
[ 39.315374] FS: 00007f8e1b981700(0000) GS:ffff8881f6a80000(0000)
knlGS:0000000000000000
[ 39.316293] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 39.316984] CR2: 00007fdf39350ff0 CR3: 000000011952e006 CR4: 00000000003706e0
[ 39.317860] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 39.318680] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 39.319467] Call Trace:
[ 39.319744] <TASK>
[ 39.319982] bpf_trampoline_6442471603_0+0x32/0x1000
[ 39.320537] do_init_module+0x5/0x6f0
[ 39.320945] load_module+0x77c0/0x9c00
[ 39.321376] ? module_frob_arch_sections+0x20/0x20
[ 39.321892] ? ima_post_read_file+0x161/0x180
[ 39.322392] ? ima_read_file+0x140/0x140
[ 39.322827] ? security_kernel_post_read_file+0x55/0xb0
[ 39.323406] ? __x64_sys_fsconfig+0x630/0x630
[ 39.323889] ? fput_many+0x1e/0x120
[ 39.324285] ? __do_sys_finit_module+0xf3/0x150
[ 39.324822] __do_sys_finit_module+0xf3/0x150
[ 39.325311] ? __ia32_sys_init_module+0xb0/0xb0
[ 39.325826] ? rcu_read_lock_held_common+0xe/0xa0
[ 39.326349] ? rcu_read_lock_sched_held+0x5a/0xc0
[ 39.326869] ? rcu_read_lock_bh_held+0xa0/0xa0
[ 39.327362] ? file_open_root+0x1f0/0x1f0
[ 39.327812] ? syscall_trace_enter.isra.17+0x184/0x250
[ 39.328411] do_syscall_64+0x38/0x80
[ 39.328812] entry_SYSCALL_64_after_hwframe+0x44/0xae
The test was designed to check whether the kernel bug is fixed.
If not it would crash the kernel.
Kumar,
you've added that test.
Could you please take a look at why it is crashing in Peter's tree?
On Fri, Mar 11, 2022 at 09:09:38AM -0800, Alexei Starovoitov wrote:
> On Fri, Mar 11, 2022 at 2:40 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Thu, Mar 10, 2022 at 05:29:11PM +0100, Peter Zijlstra wrote:
> >
> > > This seems to cure most of the rest. I'm still seeing one failure:
> > >
> > > libbpf: prog 'connect_v4_prog': BPF program load failed: Invalid argument
> > > libbpf: failed to load program 'connect_v4_prog'
> > > libbpf: failed to load object './connect4_prog.o'
> > > test_fexit_bpf2bpf_common:FAIL:tgt_prog_load unexpected error: -22 (errno 22)
> > > #48/4 fexit_bpf2bpf/func_replace_verify:FAIL
> >
> >
> > Hmm, with those two patches on I get:
> >
> > root@tigerlake:/usr/src/linux-2.6/tgl-build# ./test_progs -t fexit
> > #46 fentry_fexit:OK
> > #48 fexit_bpf2bpf:OK
> > #49 fexit_sleep:OK
> > #50 fexit_stress:OK
> > #51 fexit_test:OK
> > Summary: 5/9 PASSED, 0 SKIPPED, 0 FAILED
> >
> > On the tigerlake, I suppose I'm doing something wrong on the other
> > machine because there it's even failing on the pre-ibt kernel image.
> >
> > I'll go write up changelogs and stick these on.
>
> What is the latest branch I can use to test it?
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
that also include bpf-next. Thanks!
On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > During the build with gcc 8.5 I see:
> >
> > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > .ibt_endbr_seal, skipping
> > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > .orc_unwind section, skipping
> > LD [M] crypto/async_tx/async_xor.ko
> > LD [M] crypto/authenc.ko
> > make[3]: *** [../scripts/Makefile.modfinal:61:
> > arch/x86/crypto/crc32c-intel.ko] Error 255
> > make[3]: *** Waiting for unfinished jobs....
> >
> > but make clean cures it.
> > I suspect it's some missing makefile dependency.
>
> Yes, I recently ran into it; I've been trying to kick Makefile into
> submission but have not had success yet. Will try again on Monday.
>
> Problem appears to be that it will re-link .ko even though .o hasn't
> changed, resulting in duplicate objtool runs. I've been trying to have
> makefile generate .o.objtool empty file to serve as dependency marker to
> avoid doing second objtool run, but like said, no luck yet.
Masahiro-san, I'm trying the below, but afaict it's not working because
the rule for the .o file itself:
$(multi-obj-m): FORCE
$(call if_changed,link_multi-m)
will in fact update the timestamp of the .o file, even if if_changed
nops out the cmd. Concequently all rules that try to use if_changed with
this .o file as a dependency will find it newer and run anyway.
remake -x output of a fs/f2fs/ module (re)build:
Prerequisite 'FORCE' of target 'fs/f2fs/f2fs.o' does not exist.
Must remake target 'fs/f2fs/f2fs.o'.
../scripts/Makefile.build:454: target 'fs/f2fs/f2fs.o' does not exist
##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
:
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Successfully remade target file 'fs/f2fs/f2fs.o'.
Prerequisite 'fs/f2fs/f2fs.o' is newer than target 'fs/f2fs/f2fs.mod'.
Prerequisite 'FORCE' of target 'fs/f2fs/f2fs.mod' does not exist.
Must remake target 'fs/f2fs/f2fs.mod'.
../scripts/Makefile.build:281: update target 'fs/f2fs/f2fs.mod' due to: fs/f2fs/f2fs.o
##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
set -e; { echo fs/f2fs/dir.o fs/f2fs/file.o fs/f2fs/inode.o fs/f2fs/namei.o fs/f2fs/hash.o fs/f2fs/super.o fs/f2fs/inline.o fs/f2fs/checkpoint.o fs/f2fs/gc.o fs/f2fs/data.o fs/f2fs/node.o fs/f2fs/segment.o fs/f2fs/recovery.o fs/f2fs/shrinker.o fs/f2fs/extent_cache.o fs/f2fs/sysfs.o fs/f2fs/debug.o fs/f2fs/xattr.o fs/f2fs/acl.o fs/f2fs/iostat.o; echo; } > fs/f2fs/f2fs.mod; printf '%s\n' 'cmd_fs/f2fs/f2fs.mod := { echo fs/f2fs/dir.o fs/f2fs/file.o fs/f2fs/inode.o fs/f2fs/namei.o fs/f2fs/hash.o fs/f2fs/super.o fs/f2fs/inline.o fs/f2fs/checkpoint.o fs/f2fs/gc.o fs/f2fs/data.o fs/f2fs/node.o fs/f2fs/segment.o fs/f2fs/recovery.o fs/f2fs/shrinker.o fs/f2fs/extent_cache.o fs/f2fs/sysfs.o fs/f2fs/debug.o fs/f2fs/xattr.o fs/f2fs/acl.o fs/f2fs/iostat.o; echo; } > fs/f2fs/f2fs.mod' > fs/f2fs/.f2fs.mod.cmd
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Successfully remade target file 'fs/f2fs/f2fs.mod'.
Prerequisite 'fs/f2fs/f2fs.o' is newer than target 'fs/f2fs/f2fs.objtool'.
Prerequisite 'FORCE' of target 'fs/f2fs/f2fs.objtool' does not exist.
Must remake target 'fs/f2fs/f2fs.objtool'.
../scripts/Makefile.build:287: update target 'fs/f2fs/f2fs.objtool' due to: fs/f2fs/f2fs.o
##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
set -e; { echo fs/f2fs/f2fs.o ; ./tools/objtool/objtool orc generate --module --lto --ibt --no-fp --uaccess fs/f2fs/f2fs.o ; } > fs/f2fs/f2fs.objtool; printf '%s\n' 'cmd_fs/f2fs/f2fs.objtool := { echo fs/f2fs/f2fs.o ; ./tools/objtool/objtool orc generate --module --lto --ibt --no-fp --uaccess fs/f2fs/f2fs.o ; } > fs/f2fs/f2fs.objtool' > fs/f2fs/.f2fs.objtool.cmd
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
fs/f2fs/f2fs.o: warning: objtool: file already has .static_call_sites section, skipping
fs/f2fs/f2fs.o: warning: objtool: file already has .ibt_endbr_seal, skipping
fs/f2fs/f2fs.o: warning: objtool: file already has .orc_unwind section, skipping
../scripts/Makefile.build:286: *** [fs/f2fs/f2fs.objtool] error 255
Where we can see that we don't re-generate f2fs.o (empty command), but
then we do re-generate f2fs.mod because f2fs.o is newer and the same
happens for the new f2fs.objtool.
Help?
---
Index: linux-2.6/scripts/Makefile.build
===================================================================
--- linux-2.6.orig/scripts/Makefile.build
+++ linux-2.6/scripts/Makefile.build
@@ -92,6 +92,10 @@ ifdef CONFIG_LTO_CLANG
targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
endif
+ifdef CONFIG_X86_KERNEL_IBT
+targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
+endif
+
ifdef need-modorder
targets-for-modules += $(obj)/modules.order
endif
@@ -276,6 +280,12 @@ cmd_mod = { \
$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
$(call if_changed,mod)
+cmd_objtool_mod = \
+ { echo $< $(cmd_objtool) ; } > $@
+
+$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
+ $(call if_changed,objtool_mod)
+
quiet_cmd_cc_lst_c = MKLST $@
cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
$(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
Index: linux-2.6/scripts/Makefile.lib
===================================================================
--- linux-2.6.orig/scripts/Makefile.lib
+++ linux-2.6/scripts/Makefile.lib
@@ -552,9 +552,8 @@ objtool_args = \
$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
$(if $(CONFIG_SLS), --sls)
-cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
-cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
-cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
endif # CONFIG_STACK_VALIDATION
@@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
# instead run objtool on the module as a whole, right before
# the final link pass with the linker script.
-%.ko: objtool-enabled = y
-%.ko: part-of-module := y
+$(obj)/%.objtool: objtool-enabled = y
+$(obj)/%.objtool: part-of-module := y
else
Index: linux-2.6/scripts/Makefile.modfinal
===================================================================
--- linux-2.6.orig/scripts/Makefile.modfinal
+++ linux-2.6/scripts/Makefile.modfinal
@@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
quiet_cmd_ld_ko_o = LD [M] $@
cmd_ld_ko_o += \
- $(cmd_objtool_mod) \
$(LD) -r $(KBUILD_LDFLAGS) \
$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \
-T scripts/module.lds -o $@ $(filter %.o, $^); \
On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > and:
> > vmlinux.o: warning: objtool: ksys_unshare()+0x626: unreachable instruction
> > which stays even after make clean.
>
> Humm, I shall have to dig out gcc-8.5 then.
Ha!, I could reproduce using the bpf-selftest .config for x86_64. Fixing
that (the __noreturn on __invalid_creds) immediately yields another one
on that .config in asm_exc_double_fault. And a missing ENDBR if you
don't build 32bit compat.
I'll go clean up ...
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -551,6 +551,14 @@ SYM_CODE_START(\asmsym)
movq $-1, ORIG_RAX(%rsp) /* no syscall to restart */
call \cfunc
+ /*
+ * For some configurations exc_double_fault() is a noreturn
+ */
+1:
+ .pushsection .discard.reachable
+ .long 1b - .
+ .popsection
+
jmp paranoid_exit
_ASM_NOKPROBE(\asmsym)
@@ -1440,6 +1448,7 @@ SYM_CODE_END(asm_exc_nmi)
*/
SYM_CODE_START(ignore_sysret)
UNWIND_HINT_EMPTY
+ ENDBR
mov $-ENOSYS, %eax
sysretl
SYM_CODE_END(ignore_sysret)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index fcbc6885cc09..9ed9232af934 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -176,7 +176,7 @@ extern int set_cred_ucounts(struct cred *);
* check for validity of credentials
*/
#ifdef CONFIG_DEBUG_CREDENTIALS
-extern void __invalid_creds(const struct cred *, const char *, unsigned);
+extern void __noreturn __invalid_creds(const struct cred *, const char *, unsigned);
extern void __validate_process_creds(struct task_struct *,
const char *, unsigned);
diff --git a/kernel/cred.c b/kernel/cred.c
index 933155c96922..e10c15f51c1f 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -870,7 +870,7 @@ static void dump_invalid_creds(const struct cred *cred, const char *label,
/*
* report use of invalid credentials
*/
-void __invalid_creds(const struct cred *cred, const char *file, unsigned line)
+void __noreturn __invalid_creds(const struct cred *cred, const char *file, unsigned line)
{
printk(KERN_ERR "CRED: Invalid credentials\n");
printk(KERN_ERR "CRED: At %s:%u\n", file, line);
On Tue, Mar 15, 2022 at 10:00:43AM +0100, Peter Zijlstra wrote:
> I don't seem able to run this mod_race test, it keeps saying:
>
> tgl-build# ./test_progs -v -t mod_race
> bpf_testmod.ko is already unloaded.
> Loading bpf_testmod.ko...
> Successfully loaded bpf_testmod.ko.
> Summary: 0/0 PASSED, 0 SKIPPED, 0 FAILED
> Successfully unloaded bpf_testmod.ko.
>
> Which I'm taking to mean I'm doing it wrong...
That.. I was building the wrong tree, mod_race comes from bpf-next and I
was building a tree without that merged in... let me to see what it does
now.
The following commit has been merged into the x86/core branch of tip:
Commit-ID: 3515899bef545fc5b5f6b865e080bfe4c9a92a41
Gitweb: https://git.kernel.org/tip/3515899bef545fc5b5f6b865e080bfe4c9a92a41
Author: Peter Zijlstra <[email protected]>
AuthorDate: Mon, 14 Mar 2022 18:07:30 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 15 Mar 2022 10:32:45 +01:00
x86: Annotate idtentry_df()
Without CONFIG_X86_ESPFIX64 exc_double_fault() is noreturn and objtool
is clever enough to figure that out.
vmlinux.o: warning: objtool: asm_exc_double_fault()+0x22: unreachable instruction
0000000000001260 <asm_exc_double_fault>:
1260: f3 0f 1e fa endbr64
1264: 90 nop
1265: 90 nop
1266: 90 nop
1267: e8 84 03 00 00 call 15f0 <paranoid_entry>
126c: 48 89 e7 mov %rsp,%rdi
126f: 48 8b 74 24 78 mov 0x78(%rsp),%rsi
1274: 48 c7 44 24 78 ff ff ff ff movq $0xffffffffffffffff,0x78(%rsp)
127d: e8 00 00 00 00 call 1282 <asm_exc_double_fault+0x22> 127e: R_X86_64_PLT32 exc_double_fault-0x4
1282: e9 09 04 00 00 jmp 1690 <paranoid_exit>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/entry/entry_64.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 6e53991..4faac48 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -551,6 +551,9 @@ SYM_CODE_START(\asmsym)
movq $-1, ORIG_RAX(%rsp) /* no syscall to restart */
call \cfunc
+ /* For some configurations \cfunc ends up being a noreturn. */
+ REACHABLE
+
jmp paranoid_exit
_ASM_NOKPROBE(\asmsym)
On Mon, Mar 14, 2022 at 03:59:05PM +0100, Peter Zijlstra wrote:
> On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > > During the build with gcc 8.5 I see:
> > >
> > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > .ibt_endbr_seal, skipping
> > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > .orc_unwind section, skipping
> > > LD [M] crypto/async_tx/async_xor.ko
> > > LD [M] crypto/authenc.ko
> > > make[3]: *** [../scripts/Makefile.modfinal:61:
> > > arch/x86/crypto/crc32c-intel.ko] Error 255
> > > make[3]: *** Waiting for unfinished jobs....
> > >
> > > but make clean cures it.
> > > I suspect it's some missing makefile dependency.
> >
> > Yes, I recently ran into it; I've been trying to kick Makefile into
> > submission but have not had success yet. Will try again on Monday.
> >
> > Problem appears to be that it will re-link .ko even though .o hasn't
> > changed, resulting in duplicate objtool runs. I've been trying to have
> > makefile generate .o.objtool empty file to serve as dependency marker to
> > avoid doing second objtool run, but like said, no luck yet.
>
> Masahiro-san, I'm trying the below, but afaict it's not working because
> the rule for the .o file itself:
>
Ha, sleep, it is marvelous!
The below appears to be working as desired.
---
Index: linux-2.6/scripts/Makefile.build
===================================================================
--- linux-2.6.orig/scripts/Makefile.build
+++ linux-2.6/scripts/Makefile.build
@@ -86,12 +86,18 @@ ifdef need-builtin
targets-for-builtin += $(obj)/built-in.a
endif
-targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
+targets-for-modules :=
ifdef CONFIG_LTO_CLANG
targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
endif
+ifdef CONFIG_X86_KERNEL_IBT
+targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
+endif
+
+targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
+
ifdef need-modorder
targets-for-modules += $(obj)/modules.order
endif
@@ -276,6 +282,19 @@ cmd_mod = { \
$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
$(call if_changed,mod)
+#
+# Since objtool will re-write the file it will change the timestamps, therefore
+# it is critical that the %.objtool file gets a timestamp *after* objtool runs.
+#
+# Additionally, care must be had with ordering this rule against the other rules
+# that take %.o as a dependency.
+#
+cmd_objtool_mod = \
+ true $(cmd_objtool) ; touch $@
+
+$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
+ $(call if_changed,objtool_mod)
+
quiet_cmd_cc_lst_c = MKLST $@
cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
$(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
Index: linux-2.6/scripts/Makefile.lib
===================================================================
--- linux-2.6.orig/scripts/Makefile.lib
+++ linux-2.6/scripts/Makefile.lib
@@ -552,9 +552,8 @@ objtool_args = \
$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
$(if $(CONFIG_SLS), --sls)
-cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
-cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
-cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
endif # CONFIG_STACK_VALIDATION
@@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
# instead run objtool on the module as a whole, right before
# the final link pass with the linker script.
-%.ko: objtool-enabled = y
-%.ko: part-of-module := y
+$(obj)/%.objtool: objtool-enabled = y
+$(obj)/%.objtool: part-of-module := y
else
Index: linux-2.6/scripts/Makefile.modfinal
===================================================================
--- linux-2.6.orig/scripts/Makefile.modfinal
+++ linux-2.6/scripts/Makefile.modfinal
@@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
quiet_cmd_ld_ko_o = LD [M] $@
cmd_ld_ko_o += \
- $(cmd_objtool_mod) \
$(LD) -r $(KBUILD_LDFLAGS) \
$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \
-T scripts/module.lds -o $@ $(filter %.o, $^); \
On Tue, Mar 15, 2022 at 02:14:02AM +0530, Kumar Kartikeya Dwivedi wrote:
> [ Note: I have no experience with trampoline code or IBT so what follows might
> be incorrect. ]
>
> In case of fexit and fmod_ret, we call original function (but skip
> X86_PATCH_SIZE bytes), with ENDBR we must also skip those 4 bytes, but in some
> cases like bpf_fentry_test1, for which this test has fmod_ret prog, compiler
> (gcc 11) emits endbr64, but not for do_init_module, for which we do fexit.
>
> This means for do_init_module module, orig_call += X86_PATCH_SIZE +
> ENDBR_INSN_SIZE would skip more bytes than needed to emit call to original
> function, which explains why I was seeing crash in the middle of
> 'mov edx, 0x10' instruction.
>
> The diff below fixes the problem for me, and allows the test to pass.
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index b98e1c95bcc4..760c9a3c075f 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2031,11 +2031,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
>
> ip_off = stack_size;
>
> - if (flags & BPF_TRAMP_F_SKIP_FRAME)
> + if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> /* skip patched call instruction and point orig_call to actual
> * body of the kernel function.
> */
> - orig_call += X86_PATCH_SIZE + ENDBR_INSN_SIZE;
> + if (is_endbr(*(u32 *)orig_call))
> + orig_call += ENDBR_INSN_SIZE;
> + orig_call += X86_PATCH_SIZE;
> + }
>
> prog = image;
Hmm, so I was under the impression that this was targeting the NOP from
emit_prologue(), and that has an unconditional ENDBR. If this is instead
targeting the 'start of random kernel function' then yes, what you
propose will work.
(obviously, once we go do more complicated CFI schemes, all this needs
revisiting yet again).
I don't seem able to run this mod_race test, it keeps saying:
tgl-build# ./test_progs -v -t mod_race
bpf_testmod.ko is already unloaded.
Loading bpf_testmod.ko...
Successfully loaded bpf_testmod.ko.
Summary: 0/0 PASSED, 0 SKIPPED, 0 FAILED
Successfully unloaded bpf_testmod.ko.
Which I'm taking to mean I'm doing it wrong... so I can't immediately
verify, but your proposal looks sane so I'll fold it in.
Thanks!
On Tue, Mar 15, 2022 at 11:07:06AM +0100, Peter Zijlstra wrote:
> On Tue, Mar 15, 2022 at 10:00:43AM +0100, Peter Zijlstra wrote:
> > I don't seem able to run this mod_race test, it keeps saying:
> >
> > tgl-build# ./test_progs -v -t mod_race
> > bpf_testmod.ko is already unloaded.
> > Loading bpf_testmod.ko...
> > Successfully loaded bpf_testmod.ko.
> > Summary: 0/0 PASSED, 0 SKIPPED, 0 FAILED
> > Successfully unloaded bpf_testmod.ko.
> >
> > Which I'm taking to mean I'm doing it wrong...
>
> That.. I was building the wrong tree, mod_race comes from bpf-next and I
> was building a tree without that merged in... let me to see what it does
> now.
I can confirm, crashes without, fixed with. Thanks!
I've forced pushed a cleaned up tip/x86/core. Also:
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt.bpf
is a merge of that and bpf-next
On Wed, Mar 16, 2022 at 03:05:38PM IST, Peter Zijlstra wrote:
> On Tue, Mar 15, 2022 at 10:00:43AM +0100, Peter Zijlstra wrote:
> > On Tue, Mar 15, 2022 at 02:14:02AM +0530, Kumar Kartikeya Dwivedi wrote:
> >
> > > [ Note: I have no experience with trampoline code or IBT so what follows might
> > > be incorrect. ]
> > >
> > > In case of fexit and fmod_ret, we call original function (but skip
> > > X86_PATCH_SIZE bytes), with ENDBR we must also skip those 4 bytes, but in some
> > > cases like bpf_fentry_test1, for which this test has fmod_ret prog, compiler
> > > (gcc 11) emits endbr64, but not for do_init_module, for which we do fexit.
> > >
> > > This means for do_init_module module, orig_call += X86_PATCH_SIZE +
> > > ENDBR_INSN_SIZE would skip more bytes than needed to emit call to original
> > > function, which explains why I was seeing crash in the middle of
> > > 'mov edx, 0x10' instruction.
> > >
> > > The diff below fixes the problem for me, and allows the test to pass.
> > >
> > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > > index b98e1c95bcc4..760c9a3c075f 100644
> > > --- a/arch/x86/net/bpf_jit_comp.c
> > > +++ b/arch/x86/net/bpf_jit_comp.c
> > > @@ -2031,11 +2031,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> > >
> > > ip_off = stack_size;
> > >
> > > - if (flags & BPF_TRAMP_F_SKIP_FRAME)
> > > + if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> > > /* skip patched call instruction and point orig_call to actual
> > > * body of the kernel function.
> > > */
> > > - orig_call += X86_PATCH_SIZE + ENDBR_INSN_SIZE;
> > > + if (is_endbr(*(u32 *)orig_call))
> > > + orig_call += ENDBR_INSN_SIZE;
> > > + orig_call += X86_PATCH_SIZE;
> > > + }
> > >
> > > prog = image;
> >
> > Hmm, so I was under the impression that this was targeting the NOP from
> > emit_prologue(), and that has an unconditional ENDBR. If this is instead
> > targeting the 'start of random kernel function' then yes, what you
> > propose will work.
>
> Can you confirm that orig_call can be any kernel function? Because if
> so, I'm thinking it will still do the wrong thing for a notrace
> function, that will not have a __fentry__ site, so unconditionally
> skipping those 5 bytes will place you somewhere non-sensible.
>
It fails for notrace functions, e.g. considering fentry prog, when
bpf_trampoline_link_prog -> bpf_trampoline_update eventually calls
register_fentry -> bpf_arch_text_poke, old_addr is NULL, so nop_insn is copied
to old_insn, and then that memcmp(ip, old_insn, X86_PATCH_SIZE) should fail, so
it would return -EBUSY. If you consider modify_fentry case, then register_fentry
for earlier prog must have succeeded, so it must not be notrace function.
The orig_call adjustment is only done for fexit and fmod_ret (they set CALL_ORIG
and SKIP_FRAME flags), because in case of just fentry we just continue after
ret, instead of emitting call to original function in trampoline, for those too
the bpf_arch_text_poke should fail, for the same reason as above.
> This would not be a new issue; but perhaps it should be clarified and or
> fixed.
Based on my inspection it looks fine, others can correct me if I'm wrong.
--
Kartikeya
On Mon, Mar 14, 2022 at 1:44 PM Kumar Kartikeya Dwivedi
<[email protected]> wrote:
>
> The crash does not seem to be resurfacing the bug, AFAICT.
>
> [ Note: I have no experience with trampoline code or IBT so what follows might
> be incorrect. ]
>
> In case of fexit and fmod_ret, we call original function (but skip
> X86_PATCH_SIZE bytes), with ENDBR we must also skip those 4 bytes, but in some
> cases like bpf_fentry_test1, for which this test has fmod_ret prog, compiler
> (gcc 11) emits endbr64, but not for do_init_module, for which we do fexit.
>
> This means for do_init_module module, orig_call += X86_PATCH_SIZE +
> ENDBR_INSN_SIZE would skip more bytes than needed to emit call to original
> function, which explains why I was seeing crash in the middle of
> 'mov edx, 0x10' instruction.
>
> The diff below fixes the problem for me, and allows the test to pass.
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index b98e1c95bcc4..760c9a3c075f 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2031,11 +2031,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
>
> ip_off = stack_size;
>
> - if (flags & BPF_TRAMP_F_SKIP_FRAME)
> + if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> /* skip patched call instruction and point orig_call to actual
> * body of the kernel function.
> */
> - orig_call += X86_PATCH_SIZE + ENDBR_INSN_SIZE;
> + if (is_endbr(*(u32 *)orig_call))
> + orig_call += ENDBR_INSN_SIZE;
> + orig_call += X86_PATCH_SIZE;
> + }
>
Thanks Kumar!
The bpf trampoline can attach to both indirect and non-indirect
functions. My understanding is that only indirect targets will have
endbr first insn. So the fix totally makes sense.
On Mon, Mar 14, 2022 at 11:59 PM Peter Zijlstra <[email protected]> wrote:
>
> On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > > During the build with gcc 8.5 I see:
> > >
> > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > .ibt_endbr_seal, skipping
> > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > .orc_unwind section, skipping
> > > LD [M] crypto/async_tx/async_xor.ko
> > > LD [M] crypto/authenc.ko
> > > make[3]: *** [../scripts/Makefile.modfinal:61:
> > > arch/x86/crypto/crc32c-intel.ko] Error 255
> > > make[3]: *** Waiting for unfinished jobs....
> > >
> > > but make clean cures it.
> > > I suspect it's some missing makefile dependency.
> >
> > Yes, I recently ran into it; I've been trying to kick Makefile into
> > submission but have not had success yet. Will try again on Monday.
> >
> > Problem appears to be that it will re-link .ko even though .o hasn't
> > changed, resulting in duplicate objtool runs. I've been trying to have
> > makefile generate .o.objtool empty file to serve as dependency marker to
> > avoid doing second objtool run, but like said, no luck yet.
>
> Masahiro-san, I'm trying the below, but afaict it's not working because
> the rule for the .o file itself:
>
> $(multi-obj-m): FORCE
> $(call if_changed,link_multi-m)
>
> will in fact update the timestamp of the .o file, even if if_changed
> nops out the cmd. Concequently all rules that try to use if_changed with
> this .o file as a dependency will find it newer and run anyway.
>
>
> remake -x output of a fs/f2fs/ module (re)build:
>
> Prerequisite 'FORCE' of target 'fs/f2fs/f2fs.o' does not exist.
> Must remake target 'fs/f2fs/f2fs.o'.
> ../scripts/Makefile.build:454: target 'fs/f2fs/f2fs.o' does not exist
> ##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> :
> ##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> Successfully remade target file 'fs/f2fs/f2fs.o'.
> Prerequisite 'fs/f2fs/f2fs.o' is newer than target 'fs/f2fs/f2fs.mod'.
> Prerequisite 'FORCE' of target 'fs/f2fs/f2fs.mod' does not exist.
> Must remake target 'fs/f2fs/f2fs.mod'.
> ../scripts/Makefile.build:281: update target 'fs/f2fs/f2fs.mod' due to: fs/f2fs/f2fs.o
> ##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> set -e; { echo fs/f2fs/dir.o fs/f2fs/file.o fs/f2fs/inode.o fs/f2fs/namei.o fs/f2fs/hash.o fs/f2fs/super.o fs/f2fs/inline.o fs/f2fs/checkpoint.o fs/f2fs/gc.o fs/f2fs/data.o fs/f2fs/node.o fs/f2fs/segment.o fs/f2fs/recovery.o fs/f2fs/shrinker.o fs/f2fs/extent_cache.o fs/f2fs/sysfs.o fs/f2fs/debug.o fs/f2fs/xattr.o fs/f2fs/acl.o fs/f2fs/iostat.o; echo; } > fs/f2fs/f2fs.mod; printf '%s\n' 'cmd_fs/f2fs/f2fs.mod := { echo fs/f2fs/dir.o fs/f2fs/file.o fs/f2fs/inode.o fs/f2fs/namei.o fs/f2fs/hash.o fs/f2fs/super.o fs/f2fs/inline.o fs/f2fs/checkpoint.o fs/f2fs/gc.o fs/f2fs/data.o fs/f2fs/node.o fs/f2fs/segment.o fs/f2fs/recovery.o fs/f2fs/shrinker.o fs/f2fs/extent_cache.o fs/f2fs/sysfs.o fs/f2fs/debug.o fs/f2fs/xattr.o fs/f2fs/acl.o fs/f2fs/iostat.o; echo; } > fs/f2fs/f2fs.mod' > fs/f2fs/.f2fs.mod.cmd
> ##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> Successfully remade target file 'fs/f2fs/f2fs.mod'.
> Prerequisite 'fs/f2fs/f2fs.o' is newer than target 'fs/f2fs/f2fs.objtool'.
> Prerequisite 'FORCE' of target 'fs/f2fs/f2fs.objtool' does not exist.
> Must remake target 'fs/f2fs/f2fs.objtool'.
> ../scripts/Makefile.build:287: update target 'fs/f2fs/f2fs.objtool' due to: fs/f2fs/f2fs.o
> ##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> set -e; { echo fs/f2fs/f2fs.o ; ./tools/objtool/objtool orc generate --module --lto --ibt --no-fp --uaccess fs/f2fs/f2fs.o ; } > fs/f2fs/f2fs.objtool; printf '%s\n' 'cmd_fs/f2fs/f2fs.objtool := { echo fs/f2fs/f2fs.o ; ./tools/objtool/objtool orc generate --module --lto --ibt --no-fp --uaccess fs/f2fs/f2fs.o ; } > fs/f2fs/f2fs.objtool' > fs/f2fs/.f2fs.objtool.cmd
> ##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> fs/f2fs/f2fs.o: warning: objtool: file already has .static_call_sites section, skipping
> fs/f2fs/f2fs.o: warning: objtool: file already has .ibt_endbr_seal, skipping
> fs/f2fs/f2fs.o: warning: objtool: file already has .orc_unwind section, skipping
> ../scripts/Makefile.build:286: *** [fs/f2fs/f2fs.objtool] error 255
>
>
> Where we can see that we don't re-generate f2fs.o (empty command), but
> then we do re-generate f2fs.mod because f2fs.o is newer and the same
> happens for the new f2fs.objtool.
>
> Help?
Help?
I had never noticed this thread before because
you did not CC me or kbuild ML.
Looking at the build system changes:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/
Both patches are wrong.
So is the fix-up you appended lator in this thread.
Apparently, you were screwing up Kbuild in a brown paper bag.
So scared.
>
> ---
> Index: linux-2.6/scripts/Makefile.build
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.build
> +++ linux-2.6/scripts/Makefile.build
> @@ -92,6 +92,10 @@ ifdef CONFIG_LTO_CLANG
> targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
> endif
>
> +ifdef CONFIG_X86_KERNEL_IBT
> +targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
> +endif
> +
> ifdef need-modorder
> targets-for-modules += $(obj)/modules.order
> endif
> @@ -276,6 +280,12 @@ cmd_mod = { \
> $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
> $(call if_changed,mod)
>
> +cmd_objtool_mod = \
> + { echo $< $(cmd_objtool) ; } > $@
> +
> +$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
> + $(call if_changed,objtool_mod)
> +
> quiet_cmd_cc_lst_c = MKLST $@
> cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
> $(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
> Index: linux-2.6/scripts/Makefile.lib
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.lib
> +++ linux-2.6/scripts/Makefile.lib
> @@ -552,9 +552,8 @@ objtool_args = \
> $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
> $(if $(CONFIG_SLS), --sls)
>
> -cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> -cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
> -cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
> +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
>
> endif # CONFIG_STACK_VALIDATION
>
> @@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
>
> # instead run objtool on the module as a whole, right before
> # the final link pass with the linker script.
> -%.ko: objtool-enabled = y
> -%.ko: part-of-module := y
> +$(obj)/%.objtool: objtool-enabled = y
> +$(obj)/%.objtool: part-of-module := y
>
> else
>
> Index: linux-2.6/scripts/Makefile.modfinal
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.modfinal
> +++ linux-2.6/scripts/Makefile.modfinal
> @@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
>
> quiet_cmd_ld_ko_o = LD [M] $@
> cmd_ld_ko_o += \
> - $(cmd_objtool_mod) \
> $(LD) -r $(KBUILD_LDFLAGS) \
> $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \
> -T scripts/module.lds -o $@ $(filter %.o, $^); \
--
Best Regards
Masahiro Yamada
The following commit has been merged into the x86/core branch of tip:
Commit-ID: 105cd68596392cfe15056a891b0723609dcad247
Gitweb: https://git.kernel.org/tip/105cd68596392cfe15056a891b0723609dcad247
Author: Peter Zijlstra <[email protected]>
AuthorDate: Mon, 14 Mar 2022 17:58:35 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 15 Mar 2022 10:32:44 +01:00
x86: Mark __invalid_creds() __noreturn
vmlinux.o: warning: objtool: ksys_unshare()+0x36c: unreachable instruction
0000 0000000000067040 <ksys_unshare>:
...
0364 673a4: 4c 89 ef mov %r13,%rdi
0367 673a7: e8 00 00 00 00 call 673ac <ksys_unshare+0x36c> 673a8: R_X86_64_PLT32 __invalid_creds-0x4
036c 673ac: e9 28 ff ff ff jmp 672d9 <ksys_unshare+0x299>
0371 673b1: 41 bc f4 ff ff ff mov $0xfffffff4,%r12d
0377 673b7: e9 80 fd ff ff jmp 6713c <ksys_unshare+0xfc>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
include/linux/cred.h | 2 +-
kernel/cred.c | 2 +-
tools/objtool/check.c | 1 +
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index fcbc688..9ed9232 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -176,7 +176,7 @@ extern int set_cred_ucounts(struct cred *);
* check for validity of credentials
*/
#ifdef CONFIG_DEBUG_CREDENTIALS
-extern void __invalid_creds(const struct cred *, const char *, unsigned);
+extern void __noreturn __invalid_creds(const struct cred *, const char *, unsigned);
extern void __validate_process_creds(struct task_struct *,
const char *, unsigned);
diff --git a/kernel/cred.c b/kernel/cred.c
index 933155c..e10c15f 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -870,7 +870,7 @@ static void dump_invalid_creds(const struct cred *cred, const char *label,
/*
* report use of invalid credentials
*/
-void __invalid_creds(const struct cred *cred, const char *file, unsigned line)
+void __noreturn __invalid_creds(const struct cred *cred, const char *file, unsigned line)
{
printk(KERN_ERR "CRED: Invalid credentials\n");
printk(KERN_ERR "CRED: At %s:%u\n", file, line);
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 9896562..0c857e7 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -183,6 +183,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"cpu_bringup_and_idle",
"do_group_exit",
"stop_this_cpu",
+ "__invalid_creds",
};
if (!func)
On Sun, Mar 13, 2022 at 07:03:39AM IST, Alexei Starovoitov wrote:
> On Sat, Mar 12, 2022 at 7:44 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Fri, Mar 11, 2022 at 09:09:38AM -0800, Alexei Starovoitov wrote:
> > > On Fri, Mar 11, 2022 at 2:40 AM Peter Zijlstra <[email protected]> wrote:
> > > >
> > > > On Thu, Mar 10, 2022 at 05:29:11PM +0100, Peter Zijlstra wrote:
> > > >
> > > > > This seems to cure most of the rest. I'm still seeing one failure:
> > > > >
> > > > > libbpf: prog 'connect_v4_prog': BPF program load failed: Invalid argument
> > > > > libbpf: failed to load program 'connect_v4_prog'
> > > > > libbpf: failed to load object './connect4_prog.o'
> > > > > test_fexit_bpf2bpf_common:FAIL:tgt_prog_load unexpected error: -22 (errno 22)
> > > > > #48/4 fexit_bpf2bpf/func_replace_verify:FAIL
> > > >
> > > >
> > > > Hmm, with those two patches on I get:
> > > >
> > > > root@tigerlake:/usr/src/linux-2.6/tgl-build# ./test_progs -t fexit
> > > > #46 fentry_fexit:OK
> > > > #48 fexit_bpf2bpf:OK
> > > > #49 fexit_sleep:OK
> > > > #50 fexit_stress:OK
> > > > #51 fexit_test:OK
> > > > Summary: 5/9 PASSED, 0 SKIPPED, 0 FAILED
> > > >
> > > > On the tigerlake, I suppose I'm doing something wrong on the other
> > > > machine because there it's even failing on the pre-ibt kernel image.
> > > >
> > > > I'll go write up changelogs and stick these on.
> > >
> > > What is the latest branch I can use to test it?
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
> >
> > that also include bpf-next. Thanks!
>
> Looks better.
> During the build with gcc 8.5 I see:
>
> arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> .ibt_endbr_seal, skipping
> arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> .orc_unwind section, skipping
> LD [M] crypto/async_tx/async_xor.ko
> LD [M] crypto/authenc.ko
> make[3]: *** [../scripts/Makefile.modfinal:61:
> arch/x86/crypto/crc32c-intel.ko] Error 255
> make[3]: *** Waiting for unfinished jobs....
>
> but make clean cures it.
> I suspect it's some missing makefile dependency.
>
> and:
> vmlinux.o: warning: objtool: ksys_unshare()+0x626: unreachable instruction
> which stays even after make clean.
>
> The rcu "false positive" is still there that causes
> sporadic hangs during the boot.
>
> The test_progs shows:
> Summary: 228/1122 PASSED, 4 SKIPPED, 6 FAILED
> (when I remove one test)
>
> That test is actually crashing the kernel:
> ./test_progs -t mod_race
> [ 39.202593] bpf_testmod: loading out-of-tree module taints kernel.
> [ 39.303142] general protection fault, probably for non-canonical
> address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN PTI
> [ 39.304610] KASAN: null-ptr-deref in range
> [0x0000000000000000-0x0000000000000007]
> [ 39.305514] CPU: 9 PID: 1599 Comm: test_progs Tainted: G
> O 5.17.0-rc7-02525-g5dd5efb53cf1 #4
> [ 39.306675] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> [ 39.308036] RIP: 0010:do_init_module+0x9/0x6f0
> [ 39.308583] Code: fe ff ff e8 59 13 46 00 e9 7f fe ff ff e8 4f 13
> 46 00 e9 49 fe ff ff 66 2e 0f 1f 84 00 00 00 00 00 e8 cb 8d eb 1e 48
> b8 00 00 <00> 00 00 fc ff df 41 57 49 89 ff 48 c7 c7 20 f6 5c 84 48 89
> fa 41
> [ 39.310815] RSP: 0018:ffff88810f7e7aa0 EFLAGS: 00010282
> [ 39.311450] RAX: dffffc0000000000 RBX: 0000000000000009 RCX: ffffffff81283b16
> [ 39.312253] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffffffffa0224c00
> [ 39.313031] RBP: ffff88810f7e7ac8 R08: 0000000000000000 R09: fffffbfff0b4d557
> [ 39.313813] R10: ffffffff85a6aab7 R11: fffffbfff0b4d556 R12: ffff88811171f518
> [ 39.314591] R13: dffffc0000000000 R14: ffffffffa0224c00 R15: ffff88810f7e7e50
> [ 39.315374] FS: 00007f8e1b981700(0000) GS:ffff8881f6a80000(0000)
> knlGS:0000000000000000
> [ 39.316293] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 39.316984] CR2: 00007fdf39350ff0 CR3: 000000011952e006 CR4: 00000000003706e0
> [ 39.317860] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 39.318680] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 39.319467] Call Trace:
> [ 39.319744] <TASK>
> [ 39.319982] bpf_trampoline_6442471603_0+0x32/0x1000
> [ 39.320537] do_init_module+0x5/0x6f0
> [ 39.320945] load_module+0x77c0/0x9c00
> [ 39.321376] ? module_frob_arch_sections+0x20/0x20
> [ 39.321892] ? ima_post_read_file+0x161/0x180
> [ 39.322392] ? ima_read_file+0x140/0x140
> [ 39.322827] ? security_kernel_post_read_file+0x55/0xb0
> [ 39.323406] ? __x64_sys_fsconfig+0x630/0x630
> [ 39.323889] ? fput_many+0x1e/0x120
> [ 39.324285] ? __do_sys_finit_module+0xf3/0x150
> [ 39.324822] __do_sys_finit_module+0xf3/0x150
> [ 39.325311] ? __ia32_sys_init_module+0xb0/0xb0
> [ 39.325826] ? rcu_read_lock_held_common+0xe/0xa0
> [ 39.326349] ? rcu_read_lock_sched_held+0x5a/0xc0
> [ 39.326869] ? rcu_read_lock_bh_held+0xa0/0xa0
> [ 39.327362] ? file_open_root+0x1f0/0x1f0
> [ 39.327812] ? syscall_trace_enter.isra.17+0x184/0x250
> [ 39.328411] do_syscall_64+0x38/0x80
> [ 39.328812] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> The test was designed to check whether the kernel bug is fixed.
> If not it would crash the kernel.
>
> Kumar,
> you've added that test.
> Could you please take a look at why it is crashing in Peter's tree?
The crash does not seem to be resurfacing the bug, AFAICT.
[ Note: I have no experience with trampoline code or IBT so what follows might
be incorrect. ]
In case of fexit and fmod_ret, we call original function (but skip
X86_PATCH_SIZE bytes), with ENDBR we must also skip those 4 bytes, but in some
cases like bpf_fentry_test1, for which this test has fmod_ret prog, compiler
(gcc 11) emits endbr64, but not for do_init_module, for which we do fexit.
This means for do_init_module module, orig_call += X86_PATCH_SIZE +
ENDBR_INSN_SIZE would skip more bytes than needed to emit call to original
function, which explains why I was seeing crash in the middle of
'mov edx, 0x10' instruction.
The diff below fixes the problem for me, and allows the test to pass.
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b98e1c95bcc4..760c9a3c075f 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2031,11 +2031,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
ip_off = stack_size;
- if (flags & BPF_TRAMP_F_SKIP_FRAME)
+ if (flags & BPF_TRAMP_F_SKIP_FRAME) {
/* skip patched call instruction and point orig_call to actual
* body of the kernel function.
*/
- orig_call += X86_PATCH_SIZE + ENDBR_INSN_SIZE;
+ if (is_endbr(*(u32 *)orig_call))
+ orig_call += ENDBR_INSN_SIZE;
+ orig_call += X86_PATCH_SIZE;
+ }
prog = image;
--
Kartikeya
The following commit has been merged into the x86/core branch of tip:
Commit-ID: dca5da2abe406168b85f97e22109710ebe0bda08
Gitweb: https://git.kernel.org/tip/dca5da2abe406168b85f97e22109710ebe0bda08
Author: Peter Zijlstra <[email protected]>
AuthorDate: Mon, 14 Mar 2022 18:05:52 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 15 Mar 2022 10:32:45 +01:00
x86,objtool: Move the ASM_REACHABLE annotation to objtool.h
Because we need a variant for .S files too.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/include/asm/bug.h | 1 +
arch/x86/include/asm/irq_stack.h | 1 +
include/linux/compiler.h | 7 -------
include/linux/objtool.h | 16 ++++++++++++++++
tools/include/linux/objtool.h | 16 ++++++++++++++++
5 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index bab883c..4d20a29 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -4,6 +4,7 @@
#include <linux/stringify.h>
#include <linux/instrumentation.h>
+#include <linux/objtool.h>
/*
* Despite that some emulators terminate on UD2, we use it for WARN().
diff --git a/arch/x86/include/asm/irq_stack.h b/arch/x86/include/asm/irq_stack.h
index 05af249..63f818a 100644
--- a/arch/x86/include/asm/irq_stack.h
+++ b/arch/x86/include/asm/irq_stack.h
@@ -3,6 +3,7 @@
#define _ASM_X86_IRQ_STACK_H
#include <linux/ptrace.h>
+#include <linux/objtool.h>
#include <asm/processor.h>
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 0f7fd20..219aa5d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -125,18 +125,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
})
#define annotate_unreachable() __annotate_unreachable(__COUNTER__)
-#define ASM_REACHABLE \
- "998:\n\t" \
- ".pushsection .discard.reachable\n\t" \
- ".long 998b - .\n\t" \
- ".popsection\n\t"
-
/* Annotate a C jump table to allow objtool to follow the code flow */
#define __annotate_jump_table __section(".rodata..c_jump_table")
#else
#define annotate_unreachable()
-# define ASM_REACHABLE
#define __annotate_jump_table
#endif
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index f797368..586d357 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -83,6 +83,12 @@ struct unwind_hint {
_ASM_PTR " 986b\n\t" \
".popsection\n\t"
+#define ASM_REACHABLE \
+ "998:\n\t" \
+ ".pushsection .discard.reachable\n\t" \
+ ".long 998b - .\n\t" \
+ ".popsection\n\t"
+
#else /* __ASSEMBLY__ */
/*
@@ -142,6 +148,13 @@ struct unwind_hint {
.popsection
.endm
+.macro REACHABLE
+.Lhere_\@:
+ .pushsection .discard.reachable
+ .long .Lhere_\@ - .
+ .popsection
+.endm
+
#endif /* __ASSEMBLY__ */
#else /* !CONFIG_STACK_VALIDATION */
@@ -153,6 +166,7 @@ struct unwind_hint {
#define STACK_FRAME_NON_STANDARD(func)
#define STACK_FRAME_NON_STANDARD_FP(func)
#define ANNOTATE_NOENDBR
+#define ASM_REACHABLE
#else
#define ANNOTATE_INTRA_FUNCTION_CALL
.macro UNWIND_HINT sp_reg:req sp_offset=0 type:req end=0
@@ -161,6 +175,8 @@ struct unwind_hint {
.endm
.macro ANNOTATE_NOENDBR
.endm
+.macro REACHABLE
+.endm
#endif
#endif /* CONFIG_STACK_VALIDATION */
diff --git a/tools/include/linux/objtool.h b/tools/include/linux/objtool.h
index f797368..586d357 100644
--- a/tools/include/linux/objtool.h
+++ b/tools/include/linux/objtool.h
@@ -83,6 +83,12 @@ struct unwind_hint {
_ASM_PTR " 986b\n\t" \
".popsection\n\t"
+#define ASM_REACHABLE \
+ "998:\n\t" \
+ ".pushsection .discard.reachable\n\t" \
+ ".long 998b - .\n\t" \
+ ".popsection\n\t"
+
#else /* __ASSEMBLY__ */
/*
@@ -142,6 +148,13 @@ struct unwind_hint {
.popsection
.endm
+.macro REACHABLE
+.Lhere_\@:
+ .pushsection .discard.reachable
+ .long .Lhere_\@ - .
+ .popsection
+.endm
+
#endif /* __ASSEMBLY__ */
#else /* !CONFIG_STACK_VALIDATION */
@@ -153,6 +166,7 @@ struct unwind_hint {
#define STACK_FRAME_NON_STANDARD(func)
#define STACK_FRAME_NON_STANDARD_FP(func)
#define ANNOTATE_NOENDBR
+#define ASM_REACHABLE
#else
#define ANNOTATE_INTRA_FUNCTION_CALL
.macro UNWIND_HINT sp_reg:req sp_offset=0 type:req end=0
@@ -161,6 +175,8 @@ struct unwind_hint {
.endm
.macro ANNOTATE_NOENDBR
.endm
+.macro REACHABLE
+.endm
#endif
#endif /* CONFIG_STACK_VALIDATION */
On Tue, Mar 15, 2022 at 02:30:43PM IST, Peter Zijlstra wrote:
> On Tue, Mar 15, 2022 at 02:14:02AM +0530, Kumar Kartikeya Dwivedi wrote:
>
> > [ Note: I have no experience with trampoline code or IBT so what follows might
> > be incorrect. ]
> >
> > In case of fexit and fmod_ret, we call original function (but skip
> > X86_PATCH_SIZE bytes), with ENDBR we must also skip those 4 bytes, but in some
> > cases like bpf_fentry_test1, for which this test has fmod_ret prog, compiler
> > (gcc 11) emits endbr64, but not for do_init_module, for which we do fexit.
> >
> > This means for do_init_module module, orig_call += X86_PATCH_SIZE +
> > ENDBR_INSN_SIZE would skip more bytes than needed to emit call to original
> > function, which explains why I was seeing crash in the middle of
> > 'mov edx, 0x10' instruction.
> >
> > The diff below fixes the problem for me, and allows the test to pass.
> >
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index b98e1c95bcc4..760c9a3c075f 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -2031,11 +2031,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >
> > ip_off = stack_size;
> >
> > - if (flags & BPF_TRAMP_F_SKIP_FRAME)
> > + if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> > /* skip patched call instruction and point orig_call to actual
> > * body of the kernel function.
> > */
> > - orig_call += X86_PATCH_SIZE + ENDBR_INSN_SIZE;
> > + if (is_endbr(*(u32 *)orig_call))
> > + orig_call += ENDBR_INSN_SIZE;
> > + orig_call += X86_PATCH_SIZE;
> > + }
> >
> > prog = image;
>
> Hmm, so I was under the impression that this was targeting the NOP from
> emit_prologue(), and that has an unconditional ENDBR. If this is instead
> targeting the 'start of random kernel function' then yes, what you
> propose will work.
>
> (obviously, once we go do more complicated CFI schemes, all this needs
> revisiting yet again).
>
> I don't seem able to run this mod_race test, it keeps saying:
>
> tgl-build# ./test_progs -v -t mod_race
> bpf_testmod.ko is already unloaded.
> Loading bpf_testmod.ko...
> Successfully loaded bpf_testmod.ko.
> Summary: 0/0 PASSED, 0 SKIPPED, 0 FAILED
> Successfully unloaded bpf_testmod.ko.
>
`./test_progs -v -t bpf_mod_race` should work.
> Which I'm taking to mean I'm doing it wrong... so I can't immediately
> verify, but your proposal looks sane so I'll fold it in.
>
> Thanks!
--
Kartikeya
On Tue, Mar 15, 2022 at 5:15 PM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Mar 14, 2022 at 03:59:05PM +0100, Peter Zijlstra wrote:
> > On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > > On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > > > During the build with gcc 8.5 I see:
> > > >
> > > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > > .ibt_endbr_seal, skipping
> > > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > > .orc_unwind section, skipping
> > > > LD [M] crypto/async_tx/async_xor.ko
> > > > LD [M] crypto/authenc.ko
> > > > make[3]: *** [../scripts/Makefile.modfinal:61:
> > > > arch/x86/crypto/crc32c-intel.ko] Error 255
> > > > make[3]: *** Waiting for unfinished jobs....
> > > >
> > > > but make clean cures it.
> > > > I suspect it's some missing makefile dependency.
> > >
> > > Yes, I recently ran into it; I've been trying to kick Makefile into
> > > submission but have not had success yet. Will try again on Monday.
> > >
> > > Problem appears to be that it will re-link .ko even though .o hasn't
> > > changed, resulting in duplicate objtool runs. I've been trying to have
> > > makefile generate .o.objtool empty file to serve as dependency marker to
> > > avoid doing second objtool run, but like said, no luck yet.
> >
> > Masahiro-san, I'm trying the below, but afaict it's not working because
> > the rule for the .o file itself:
> >
> Ha, sleep, it is marvelous!
>
> The below appears to be working as desired.
>
> ---
> Index: linux-2.6/scripts/Makefile.build
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.build
> +++ linux-2.6/scripts/Makefile.build
> @@ -86,12 +86,18 @@ ifdef need-builtin
> targets-for-builtin += $(obj)/built-in.a
> endif
>
> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +targets-for-modules :=
Why do you need to change this line?
>
> ifdef CONFIG_LTO_CLANG
> targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
> endif
>
> +ifdef CONFIG_X86_KERNEL_IBT
> +targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
> +endif
> +
> +targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +
> ifdef need-modorder
> targets-for-modules += $(obj)/modules.order
> endif
> @@ -276,6 +282,19 @@ cmd_mod = { \
> $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
> $(call if_changed,mod)
>
> +#
> +# Since objtool will re-write the file it will change the timestamps, therefore
> +# it is critical that the %.objtool file gets a timestamp *after* objtool runs.
Thanks for explaining how stupidly this works.
NACK.
I guess re-using the current clang-lto rule is much cleaner.
(but please rename %.lto.o to %.prelink.o)
Roughly like this:
if CONFIG_LTO_CLANG || CONFIG_X86_KERNEL_IBT
$(obj)/%.prelink: $(obj)/%.o FORCE
[ $(LD) if CONFIG_LTO_CLANG ] + $(cmd_objtool)
endif
> +#
> +# Additionally, care must be had with ordering this rule against the other rules
> +# that take %.o as a dependency.
> +#
> +cmd_objtool_mod = \
> + true $(cmd_objtool) ; touch $@
> +
> +$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
> + $(call if_changed,objtool_mod)
> +
> quiet_cmd_cc_lst_c = MKLST $@
> cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
> $(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
> Index: linux-2.6/scripts/Makefile.lib
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.lib
> +++ linux-2.6/scripts/Makefile.lib
> @@ -552,9 +552,8 @@ objtool_args = \
> $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
> $(if $(CONFIG_SLS), --sls)
>
> -cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> -cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
> -cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
> +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
>
> endif # CONFIG_STACK_VALIDATION
>
> @@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
>
> # instead run objtool on the module as a whole, right before
> # the final link pass with the linker script.
> -%.ko: objtool-enabled = y
> -%.ko: part-of-module := y
> +$(obj)/%.objtool: objtool-enabled = y
> +$(obj)/%.objtool: part-of-module := y
>
> else
>
> Index: linux-2.6/scripts/Makefile.modfinal
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.modfinal
> +++ linux-2.6/scripts/Makefile.modfinal
> @@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
>
> quiet_cmd_ld_ko_o = LD [M] $@
> cmd_ld_ko_o += \
> - $(cmd_objtool_mod) \
> $(LD) -r $(KBUILD_LDFLAGS) \
> $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \
> -T scripts/module.lds -o $@ $(filter %.o, $^); \
--
Best Regards
Masahiro Yamada
On Tue, Mar 15, 2022 at 10:00:43AM +0100, Peter Zijlstra wrote:
> On Tue, Mar 15, 2022 at 02:14:02AM +0530, Kumar Kartikeya Dwivedi wrote:
>
> > [ Note: I have no experience with trampoline code or IBT so what follows might
> > be incorrect. ]
> >
> > In case of fexit and fmod_ret, we call original function (but skip
> > X86_PATCH_SIZE bytes), with ENDBR we must also skip those 4 bytes, but in some
> > cases like bpf_fentry_test1, for which this test has fmod_ret prog, compiler
> > (gcc 11) emits endbr64, but not for do_init_module, for which we do fexit.
> >
> > This means for do_init_module module, orig_call += X86_PATCH_SIZE +
> > ENDBR_INSN_SIZE would skip more bytes than needed to emit call to original
> > function, which explains why I was seeing crash in the middle of
> > 'mov edx, 0x10' instruction.
> >
> > The diff below fixes the problem for me, and allows the test to pass.
> >
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index b98e1c95bcc4..760c9a3c075f 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -2031,11 +2031,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >
> > ip_off = stack_size;
> >
> > - if (flags & BPF_TRAMP_F_SKIP_FRAME)
> > + if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> > /* skip patched call instruction and point orig_call to actual
> > * body of the kernel function.
> > */
> > - orig_call += X86_PATCH_SIZE + ENDBR_INSN_SIZE;
> > + if (is_endbr(*(u32 *)orig_call))
> > + orig_call += ENDBR_INSN_SIZE;
> > + orig_call += X86_PATCH_SIZE;
> > + }
> >
> > prog = image;
>
> Hmm, so I was under the impression that this was targeting the NOP from
> emit_prologue(), and that has an unconditional ENDBR. If this is instead
> targeting the 'start of random kernel function' then yes, what you
> propose will work.
Can you confirm that orig_call can be any kernel function? Because if
so, I'm thinking it will still do the wrong thing for a notrace
function, that will not have a __fentry__ site, so unconditionally
skipping those 5 bytes will place you somewhere non-sensible.
This would not be a new issue; but perhaps it should be clarified and or
fixed.
On Tue, Mar 15, 2022 at 5:15 PM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Mar 14, 2022 at 03:59:05PM +0100, Peter Zijlstra wrote:
> > On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > > On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > > > During the build with gcc 8.5 I see:
> > > >
> > > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > > .ibt_endbr_seal, skipping
> > > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > > .orc_unwind section, skipping
> > > > LD [M] crypto/async_tx/async_xor.ko
> > > > LD [M] crypto/authenc.ko
> > > > make[3]: *** [../scripts/Makefile.modfinal:61:
> > > > arch/x86/crypto/crc32c-intel.ko] Error 255
> > > > make[3]: *** Waiting for unfinished jobs....
> > > >
> > > > but make clean cures it.
> > > > I suspect it's some missing makefile dependency.
> > >
> > > Yes, I recently ran into it; I've been trying to kick Makefile into
> > > submission but have not had success yet. Will try again on Monday.
> > >
> > > Problem appears to be that it will re-link .ko even though .o hasn't
> > > changed, resulting in duplicate objtool runs. I've been trying to have
> > > makefile generate .o.objtool empty file to serve as dependency marker to
> > > avoid doing second objtool run, but like said, no luck yet.
> >
> > Masahiro-san, I'm trying the below, but afaict it's not working because
> > the rule for the .o file itself:
> >
> Ha, sleep, it is marvelous!
>
> The below appears to be working as desired.
>
> ---
> Index: linux-2.6/scripts/Makefile.build
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.build
> +++ linux-2.6/scripts/Makefile.build
> @@ -86,12 +86,18 @@ ifdef need-builtin
> targets-for-builtin += $(obj)/built-in.a
> endif
>
> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +targets-for-modules :=
>
> ifdef CONFIG_LTO_CLANG
> targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
> endif
>
> +ifdef CONFIG_X86_KERNEL_IBT
> +targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
> +endif
> +
> +targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +
> ifdef need-modorder
> targets-for-modules += $(obj)/modules.order
> endif
> @@ -276,6 +282,19 @@ cmd_mod = { \
> $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
> $(call if_changed,mod)
>
> +#
> +# Since objtool will re-write the file it will change the timestamps, therefore
> +# it is critical that the %.objtool file gets a timestamp *after* objtool runs.
> +#
> +# Additionally, care must be had with ordering this rule against the other rules
> +# that take %.o as a dependency.
> +#
> +cmd_objtool_mod = \
> + true $(cmd_objtool) ; touch $@
> +
> +$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
> + $(call if_changed,objtool_mod)
> +
> quiet_cmd_cc_lst_c = MKLST $@
> cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
> $(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
> Index: linux-2.6/scripts/Makefile.lib
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.lib
> +++ linux-2.6/scripts/Makefile.lib
> @@ -552,9 +552,8 @@ objtool_args = \
> $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
> $(if $(CONFIG_SLS), --sls)
>
> -cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> -cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
> -cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
> +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
>
> endif # CONFIG_STACK_VALIDATION
>
> @@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
>
> # instead run objtool on the module as a whole, right before
> # the final link pass with the linker script.
> -%.ko: objtool-enabled = y
> -%.ko: part-of-module := y
> +$(obj)/%.objtool: objtool-enabled = y
> +$(obj)/%.objtool: part-of-module := y
>
> else
>
> Index: linux-2.6/scripts/Makefile.modfinal
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.modfinal
> +++ linux-2.6/scripts/Makefile.modfinal
> @@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
>
> quiet_cmd_ld_ko_o = LD [M] $@
> cmd_ld_ko_o += \
> - $(cmd_objtool_mod) \
> $(LD) -r $(KBUILD_LDFLAGS) \
> $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \
> -T scripts/module.lds -o $@ $(filter %.o, $^); \
I wrote cleaner code for the Kbuild part.
This replaces
scripts/Makefile*
scripts/mod/modpost.c
of ("x86/alternative: Use .ibt_endbr_seal to seal indirect calls")
If there is more time, I have an even cleaner idea.
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a4b89b757287..12812cbb54cd 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -88,8 +88,8 @@ endif
targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
-ifdef CONFIG_LTO_CLANG
-targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
+targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
endif
ifdef need-modorder
@@ -170,7 +170,7 @@ ifdef CONFIG_MODVERSIONS
# the actual value of the checksum generated by genksyms
# o remove .tmp_<file>.o to <file>.o
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Generate .o.symversions files for each .o with exported symbols,
and link these
# to the kernel and/or modules at the end.
cmd_modversions_c =
\
@@ -230,6 +230,7 @@ objtool := $(objtree)/tools/objtool/objtool
objtool_args = \
$(if $(CONFIG_UNWINDER_ORC),orc generate,check) \
$(if $(part-of-module), --module) \
+ $(if $(CONFIG_X86_KERNEL_IBT), --lto --ibt) \
$(if $(CONFIG_FRAME_POINTER),, --no-fp) \
$(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), --no-unreachable)\
$(if $(CONFIG_RETPOLINE), --retpoline) \
@@ -242,7 +243,7 @@ cmd_gen_objtooldep = $(if $(objtool-enabled), {
echo ; echo '$@: $$(wildcard $(o
endif # CONFIG_STACK_VALIDATION
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Skip objtool for LLVM bitcode
$(obj)/%.o: objtool-enabled :=
@@ -288,24 +289,24 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Module .o files may contain LLVM bitcode, compile them into native code
# before ELF processing
-quiet_cmd_cc_lto_link_modules = LTO [M] $@
-cmd_cc_lto_link_modules = \
+quiet_cmd_cc_prelink_modules = LD [M] $@
+ cmd_cc_prelink_modules = \
$(LD) $(ld_flags) -r -o $@ \
- $(shell [ -s $(@:.lto.o=.o.symversions) ] && \
- echo -T $(@:.lto.o=.o.symversions)) \
+ $(shell [ -s $(@:.prelink.o=.o.symversions) ] &&
\
+ echo -T $(@:.prelink.o=.o.symversions)) \
--whole-archive $(filter-out FORCE,$^) \
$(cmd_objtool)
# objtool was skipped for LLVM bitcode, run it now that we have compiled
# modules into native code
-$(obj)/%.lto.o: objtool-enabled = y
-$(obj)/%.lto.o: part-of-module := y
+$(obj)/%.prelink.o: objtool-enabled = y
+$(obj)/%.prelink.o: part-of-module := y
-$(obj)/%.lto.o: $(obj)/%.o FORCE
- $(call if_changed,cc_lto_link_modules)
+$(obj)/%.prelink.o: $(obj)/%.o FORCE
+ $(call if_changed,cc_prelink_modules)
endif
cmd_mod = { \
@@ -469,7 +470,7 @@ $(obj)/lib.a: $(lib-y) FORCE
# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
# module is turned into a multi object module, $^ will contain header file
# dependencies recorded in the .*.cmd file.
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
quiet_cmd_link_multi-m = AR [M] $@
cmd_link_multi-m = \
$(cmd_update_lto_symversions); \
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 79be57fdd32a..8bfc9238237c 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -230,11 +230,11 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp
-nostdinc \
$(addprefix -I,$(DTC_INCLUDE)) \
-undef -D__DTS__
-ifeq ($(CONFIG_LTO_CLANG),y)
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
# need to run LTO to compile them into native code (.lto.o) before further
# processing.
-mod-prelink-ext := .lto
+mod-prelink-ext := .prelink
endif
# Useful for describing the dependency of composite objects
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6bfa33217914..09c3ab0a9b37 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1989,9 +1989,9 @@ static char *remove_dot(char *s)
if (m && (s[n + m] == '.' || s[n + m] == 0))
s[n] = 0;
- /* strip trailing .lto */
- if (strends(s, ".lto"))
- s[strlen(s) - 4] = '\0';
+ /* strip trailing .prelink */
+ if (strends(s, ".prelink"))
+ s[strlen(s) - 8] = '\0';
}
return s;
}
@@ -2015,9 +2015,9 @@ static void read_symbols(const char *modname)
/* strip trailing .o */
tmp = NOFAIL(strdup(modname));
tmp[strlen(tmp) - 2] = '\0';
- /* strip trailing .lto */
- if (strends(tmp, ".lto"))
- tmp[strlen(tmp) - 4] = '\0';
+ /* strip trailing .prelink */
+ if (strends(tmp, ".prelink"))
+ tmp[strlen(tmp) - 8] = '\0';
mod = new_module(tmp);
free(tmp);
}
--
Best Regards
Masahiro Yamada
On Wed, Mar 16, 2022 at 01:26:25AM +0900, Masahiro Yamada wrote:
> Help?
>
> I had never noticed this thread before because
> you did not CC me or kbuild ML.
I thought I did.. the copy in my sent folder has you on Cc. Sorry if it
went MIA. I'll go look at the patch.
On Wed, Mar 16, 2022 at 01:28:08AM +0900, Masahiro Yamada wrote:
> On Tue, Mar 15, 2022 at 5:15 PM Peter Zijlstra <[email protected]> wrote:
> > Index: linux-2.6/scripts/Makefile.build
> > ===================================================================
> > --- linux-2.6.orig/scripts/Makefile.build
> > +++ linux-2.6/scripts/Makefile.build
> > @@ -86,12 +86,18 @@ ifdef need-builtin
> > targets-for-builtin += $(obj)/built-in.a
> > endif
> >
> > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > +targets-for-modules :=
>
>
> Why do you need to change this line?
>
>
>
> >
> > ifdef CONFIG_LTO_CLANG
> > targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
> > endif
> >
> > +ifdef CONFIG_X86_KERNEL_IBT
> > +targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
> > +endif
> > +
> > +targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > +
> > ifdef need-modorder
> > targets-for-modules += $(obj)/modules.order
> > endif
The thinking was that by having the .objtool rule before the .mod rule,
objtool runs first. If mod runs before objtool, objtool will change the
timestamp and then mod will get remade, even if nothing's changed.
On Fri, Mar 18, 2022 at 03:15:59AM +0900, Masahiro Yamada wrote:
This is somewhat similar to my first attempt, except I thought it had a
extra/superflous link pass in it..
> @@ -288,24 +289,24 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
> $(call if_changed_rule,cc_o_c)
> $(call cmd,force_checksrc)
>
> -ifdef CONFIG_LTO_CLANG
> +ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
> # Module .o files may contain LLVM bitcode, compile them into native code
> # before ELF processing
> -quiet_cmd_cc_lto_link_modules = LTO [M] $@
> -cmd_cc_lto_link_modules = \
> +quiet_cmd_cc_prelink_modules = LD [M] $@
> + cmd_cc_prelink_modules = \
> $(LD) $(ld_flags) -r -o $@ \
> - $(shell [ -s $(@:.lto.o=.o.symversions) ] && \
> - echo -T $(@:.lto.o=.o.symversions)) \
> + $(shell [ -s $(@:.prelink.o=.o.symversions) ] &&
> \
> + echo -T $(@:.prelink.o=.o.symversions)) \
> --whole-archive $(filter-out FORCE,$^) \
> $(cmd_objtool)
>
> @@ -469,7 +470,7 @@ $(obj)/lib.a: $(lib-y) FORCE
> # Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> # module is turned into a multi object module, $^ will contain header file
> # dependencies recorded in the .*.cmd file.
> -ifdef CONFIG_LTO_CLANG
> +ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
> quiet_cmd_link_multi-m = AR [M] $@
> cmd_link_multi-m = \
> $(cmd_update_lto_symversions); \
Except I overlooked this part, where ar is used instead of ld to combine
the individual files.
Let me go make the change, Thanks!
On Tue, Mar 15, 2022 at 11:26:10AM -0700, Alexei Starovoitov wrote:
> The bpf trampoline can attach to both indirect and non-indirect
> functions. My understanding is that only indirect targets will have
> endbr first insn. So the fix totally makes sense.
Correct, the compiler is free to not emit endbr if it can determine the
function will never be called indirectly (or it is explicitly marked so
with a function attribute).
From: Peter Zijlstra
> Sent: 17 March 2022 19:45
>
> On Wed, Mar 16, 2022 at 01:28:08AM +0900, Masahiro Yamada wrote:
> > On Tue, Mar 15, 2022 at 5:15 PM Peter Zijlstra <[email protected]> wrote:
>
> > > Index: linux-2.6/scripts/Makefile.build
> > > ===================================================================
> > > --- linux-2.6.orig/scripts/Makefile.build
> > > +++ linux-2.6/scripts/Makefile.build
> > > @@ -86,12 +86,18 @@ ifdef need-builtin
> > > targets-for-builtin += $(obj)/built-in.a
> > > endif
> > >
> > > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > +targets-for-modules :=
> >
> >
> > Why do you need to change this line?
> >
> >
> >
> > >
> > > ifdef CONFIG_LTO_CLANG
> > > targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
> > > endif
> > >
> > > +ifdef CONFIG_X86_KERNEL_IBT
> > > +targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
> > > +endif
> > > +
> > > +targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > +
> > > ifdef need-modorder
> > > targets-for-modules += $(obj)/modules.order
> > > endif
>
> The thinking was that by having the .objtool rule before the .mod rule,
> objtool runs first. If mod runs before objtool, objtool will change the
> timestamp and then mod will get remade, even if nothing's changed.
I don't think it should make any difference.
A quick peruse didn't show where targets-for-modules actually
ends up being used (after being added to targets).
But in a makefile, if you have:
x: a b
nothing requires make to generate 'a' before or after 'b'.
gmake might have something similar to nmake's .ORDER directive
but I don't remember seeing it defined anywhere.
You can add 'b: a' to force the order (which is how .ORDER
ends up being implemented).
But I didn't spot anything of that nature.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
The following commit has been merged into the x86/core branch of tip:
Commit-ID: 2f35e67f621fffc636cb802a4f93fd168cf38274
Gitweb: https://git.kernel.org/tip/2f35e67f621fffc636cb802a4f93fd168cf38274
Author: Peter Zijlstra <[email protected]>
AuthorDate: Fri, 18 Mar 2022 12:19:27 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 18 Mar 2022 12:43:44 +01:00
kbuild: Fixup the IBT kbuild changes
Masahiro-san deemed my kbuild changes to support whole module objtool
runs too terrible to live and gracefully provided an alternative.
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/CAK7LNAQ2mYMnOKMQheVi+6byUFE3KEkjm1zcndNUfe0tORGvug@mail.gmail.com
---
scripts/Makefile.build | 68 ++++++++++++-----------------------------
scripts/Makefile.lib | 4 +-
scripts/mod/modpost.c | 12 +++----
3 files changed, 28 insertions(+), 56 deletions(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 926d254..abf93d1 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -86,18 +86,12 @@ ifdef need-builtin
targets-for-builtin += $(obj)/built-in.a
endif
-targets-for-modules :=
+targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
-ifdef CONFIG_LTO_CLANG
-targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
+targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
endif
-ifdef CONFIG_X86_KERNEL_IBT
-targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
-endif
-
-targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
-
ifdef need-modorder
targets-for-modules += $(obj)/modules.order
endif
@@ -176,7 +170,7 @@ ifdef CONFIG_MODVERSIONS
# the actual value of the checksum generated by genksyms
# o remove .tmp_<file>.o to <file>.o
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Generate .o.symversions files for each .o with exported symbols, and link these
# to the kernel and/or modules at the end.
cmd_modversions_c = \
@@ -244,31 +238,16 @@ objtool_args = \
$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
$(if $(CONFIG_SLS), --sls)
-cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
-cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
endif # CONFIG_STACK_VALIDATION
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Skip objtool for LLVM bitcode
$(obj)/%.o: objtool-enabled :=
-# objtool was skipped for LLVM bitcode, run it now that we have compiled
-# modules into native code
-$(obj)/%.lto.o: objtool-enabled = y
-$(obj)/%.lto.o: part-of-module := y
-
-else ifdef CONFIG_X86_KERNEL_IBT
-
-# Skip objtool on individual files
-$(obj)/%.o: objtool-enabled :=
-
-# instead run objtool on the module as a whole, right before
-# the final link pass with the linker script.
-$(obj)/%.objtool: objtool-enabled = y
-$(obj)/%.objtool: part-of-module := y
-
else
# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
@@ -310,19 +289,24 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Module .o files may contain LLVM bitcode, compile them into native code
# before ELF processing
-quiet_cmd_cc_lto_link_modules = LTO [M] $@
- cmd_cc_lto_link_modules = \
+quiet_cmd_cc_prelink_modules = LD [M] $@
+ cmd_cc_prelink_modules = \
$(LD) $(ld_flags) -r -o $@ \
- $(shell [ -s $(@:.lto.o=.o.symversions) ] && \
- echo -T $(@:.lto.o=.o.symversions)) \
+ $(shell [ -s $(@:.prelink.o=.o.symversions) ] && \
+ echo -T $(@:.prelink.o=.o.symversions)) \
--whole-archive $(filter-out FORCE,$^) \
$(cmd_objtool)
-$(obj)/%.lto.o: $(obj)/%.o FORCE
- $(call if_changed,cc_lto_link_modules)
+# objtool was skipped for LLVM bitcode, run it now that we have compiled
+# modules into native code
+$(obj)/%.prelink.o: objtool-enabled = y
+$(obj)/%.prelink.o: part-of-module := y
+
+$(obj)/%.prelink.o: $(obj)/%.o FORCE
+ $(call if_changed,cc_prelink_modules)
endif
cmd_mod = { \
@@ -333,18 +317,6 @@ cmd_mod = { \
$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
$(call if_changed,mod)
-#
-# Since objtool will re-write the file it will change the timestamps, therefore
-# it is critical that the %.objtool file gets a timestamp *after* objtool runs.
-#
-# Additionally, care must be had with ordering this rule against the other rules
-# that take %.o as a dependency.
-#
-cmd_objtool_mod = true $(cmd_objtool) ; touch $@
-
-$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
- $(call if_changed,objtool_mod)
-
quiet_cmd_cc_lst_c = MKLST $@
cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
$(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
@@ -498,7 +470,7 @@ $(obj)/lib.a: $(lib-y) FORCE
# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
# module is turned into a multi object module, $^ will contain header file
# dependencies recorded in the .*.cmd file.
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
quiet_cmd_link_multi-m = AR [M] $@
cmd_link_multi-m = \
$(cmd_update_lto_symversions); \
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 79be57f..8bfc923 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -230,11 +230,11 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp -nostdinc \
$(addprefix -I,$(DTC_INCLUDE)) \
-undef -D__DTS__
-ifeq ($(CONFIG_LTO_CLANG),y)
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
# need to run LTO to compile them into native code (.lto.o) before further
# processing.
-mod-prelink-ext := .lto
+mod-prelink-ext := .prelink
endif
# Useful for describing the dependency of composite objects
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6bfa332..09c3ab0 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1989,9 +1989,9 @@ static char *remove_dot(char *s)
if (m && (s[n + m] == '.' || s[n + m] == 0))
s[n] = 0;
- /* strip trailing .lto */
- if (strends(s, ".lto"))
- s[strlen(s) - 4] = '\0';
+ /* strip trailing .prelink */
+ if (strends(s, ".prelink"))
+ s[strlen(s) - 8] = '\0';
}
return s;
}
@@ -2015,9 +2015,9 @@ static void read_symbols(const char *modname)
/* strip trailing .o */
tmp = NOFAIL(strdup(modname));
tmp[strlen(tmp) - 2] = '\0';
- /* strip trailing .lto */
- if (strends(tmp, ".lto"))
- tmp[strlen(tmp) - 4] = '\0';
+ /* strip trailing .prelink */
+ if (strends(tmp, ".prelink"))
+ tmp[strlen(tmp) - 8] = '\0';
mod = new_module(tmp);
free(tmp);
}
The following commit has been merged into the x86/core branch of tip:
Commit-ID: d31ed5d767c0452b4f49846d80a0bfeafa3a4ded
Gitweb: https://git.kernel.org/tip/d31ed5d767c0452b4f49846d80a0bfeafa3a4ded
Author: Peter Zijlstra <[email protected]>
AuthorDate: Fri, 18 Mar 2022 12:19:27 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 22 Mar 2022 21:12:04 +01:00
kbuild: Fixup the IBT kbuild changes
Masahiro-san deemed my kbuild changes to support whole module objtool
runs too terrible to live and gracefully provided an alternative.
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/CAK7LNAQ2mYMnOKMQheVi+6byUFE3KEkjm1zcndNUfe0tORGvug@mail.gmail.com
---
scripts/Makefile.build | 66 +++++++++++------------------------------
scripts/Makefile.lib | 4 +-
scripts/mod/modpost.c | 12 +++----
3 files changed, 27 insertions(+), 55 deletions(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 926d254..2173a67 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -86,18 +86,12 @@ ifdef need-builtin
targets-for-builtin += $(obj)/built-in.a
endif
-targets-for-modules :=
+targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
-ifdef CONFIG_LTO_CLANG
-targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
-endif
-
-ifdef CONFIG_X86_KERNEL_IBT
-targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
+targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
endif
-targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
-
ifdef need-modorder
targets-for-modules += $(obj)/modules.order
endif
@@ -244,31 +238,16 @@ objtool_args = \
$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
$(if $(CONFIG_SLS), --sls)
-cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
-cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
endif # CONFIG_STACK_VALIDATION
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Skip objtool for LLVM bitcode
$(obj)/%.o: objtool-enabled :=
-# objtool was skipped for LLVM bitcode, run it now that we have compiled
-# modules into native code
-$(obj)/%.lto.o: objtool-enabled = y
-$(obj)/%.lto.o: part-of-module := y
-
-else ifdef CONFIG_X86_KERNEL_IBT
-
-# Skip objtool on individual files
-$(obj)/%.o: objtool-enabled :=
-
-# instead run objtool on the module as a whole, right before
-# the final link pass with the linker script.
-$(obj)/%.objtool: objtool-enabled = y
-$(obj)/%.objtool: part-of-module := y
-
else
# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
@@ -310,19 +289,24 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# Module .o files may contain LLVM bitcode, compile them into native code
# before ELF processing
-quiet_cmd_cc_lto_link_modules = LTO [M] $@
- cmd_cc_lto_link_modules = \
+quiet_cmd_cc_prelink_modules = LD [M] $@
+ cmd_cc_prelink_modules = \
$(LD) $(ld_flags) -r -o $@ \
- $(shell [ -s $(@:.lto.o=.o.symversions) ] && \
- echo -T $(@:.lto.o=.o.symversions)) \
+ $(shell [ -s $(@:.prelink.o=.o.symversions) ] && \
+ echo -T $(@:.prelink.o=.o.symversions)) \
--whole-archive $(filter-out FORCE,$^) \
$(cmd_objtool)
-$(obj)/%.lto.o: $(obj)/%.o FORCE
- $(call if_changed,cc_lto_link_modules)
+# objtool was skipped for LLVM bitcode, run it now that we have compiled
+# modules into native code
+$(obj)/%.prelink.o: objtool-enabled = y
+$(obj)/%.prelink.o: part-of-module := y
+
+$(obj)/%.prelink.o: $(obj)/%.o FORCE
+ $(call if_changed,cc_prelink_modules)
endif
cmd_mod = { \
@@ -333,18 +317,6 @@ cmd_mod = { \
$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
$(call if_changed,mod)
-#
-# Since objtool will re-write the file it will change the timestamps, therefore
-# it is critical that the %.objtool file gets a timestamp *after* objtool runs.
-#
-# Additionally, care must be had with ordering this rule against the other rules
-# that take %.o as a dependency.
-#
-cmd_objtool_mod = true $(cmd_objtool) ; touch $@
-
-$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
- $(call if_changed,objtool_mod)
-
quiet_cmd_cc_lst_c = MKLST $@
cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
$(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
@@ -498,7 +470,7 @@ $(obj)/lib.a: $(lib-y) FORCE
# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
# module is turned into a multi object module, $^ will contain header file
# dependencies recorded in the .*.cmd file.
-ifdef CONFIG_LTO_CLANG
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
quiet_cmd_link_multi-m = AR [M] $@
cmd_link_multi-m = \
$(cmd_update_lto_symversions); \
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 79be57f..8bfc923 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -230,11 +230,11 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp -nostdinc \
$(addprefix -I,$(DTC_INCLUDE)) \
-undef -D__DTS__
-ifeq ($(CONFIG_LTO_CLANG),y)
+ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
# need to run LTO to compile them into native code (.lto.o) before further
# processing.
-mod-prelink-ext := .lto
+mod-prelink-ext := .prelink
endif
# Useful for describing the dependency of composite objects
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6bfa332..09c3ab0 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1989,9 +1989,9 @@ static char *remove_dot(char *s)
if (m && (s[n + m] == '.' || s[n + m] == 0))
s[n] = 0;
- /* strip trailing .lto */
- if (strends(s, ".lto"))
- s[strlen(s) - 4] = '\0';
+ /* strip trailing .prelink */
+ if (strends(s, ".prelink"))
+ s[strlen(s) - 8] = '\0';
}
return s;
}
@@ -2015,9 +2015,9 @@ static void read_symbols(const char *modname)
/* strip trailing .o */
tmp = NOFAIL(strdup(modname));
tmp[strlen(tmp) - 2] = '\0';
- /* strip trailing .lto */
- if (strends(tmp, ".lto"))
- tmp[strlen(tmp) - 4] = '\0';
+ /* strip trailing .prelink */
+ if (strends(tmp, ".prelink"))
+ tmp[strlen(tmp) - 8] = '\0';
mod = new_module(tmp);
free(tmp);
}