[PATCH 0/7] x86: Address various objtool complaints

[PATCH 0/7] x86: Address various objtool complaints

[Peter Zijlstra]

Hi!

A few objtool fixes that I mean to include in a late pull.

[PATCH 1/7] x86/entry: Anchor annotation

[Peter Zijlstra]

x86_64-allmodconfig gives:

  vmlinux.o: warning: objtool: entry_SYSCALL_compat+0x0: ANNOTATE_NOENDBR on ENDBR

This is due to:

	ANNOTATE_NOENDBR
SYM_CODE_END(entry_SYSENTER_compat)
SYM_CODE_START(entry_SYSCALL_compat)
	UNWIND_HINT_EMPTY
	ENDBR

And the hint then landing on the first instruction of the next symbol,
which happens to be ENDBR. Stick in an int3 to anchor the annotation
in the previous symbol.

Fixes: 3e3f06950434 ("x86/ibt: Annotate text references")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/entry/entry_64_compat.S |    1 +
 1 file changed, 1 insertion(+)

--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -124,6 +124,7 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_af
 	jmp	.Lsysenter_flags_fixed
 SYM_INNER_LABEL(__end_entry_SYSENTER_compat, SYM_L_GLOBAL)
 	ANNOTATE_NOENDBR // is_sysenter_singlestep
+	int3
 SYM_CODE_END(entry_SYSENTER_compat)
 
 /*

[PATCH 2/7] objtool: Extend UNWIND_HINT based ENDBR rules

[Peter Zijlstra]

Extend the UNWIND hint driven rules for ENDBR placement. Currently
objtool expects an ENDBR at any UNWINT_HINT_IRET_REGS that is at +0 of
an STB_GLOBAL symbol, with the expectation that this is an exception
entry point.

Extend this to also expect ENDBR at UNWIND_HINT_EMPTY at +0 for
STB_GLOBAL symbols, with the expectation that these are also machine
entry points (SYSCALL et. al.).

This guarantees all machine entry points are covered by objtool rules at
the expense of a few additional false positives:

  vmlinux.o: warning: objtool: startup_64+0x0: UNWIND_HINT_EMPTY without ENDBR
  vmlinux.o: warning: objtool: start_cpu0+0x0: UNWIND_HINT_EMPTY without ENDBR

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/head_64.S |    2 ++
 tools/objtool/check.c     |   23 ++++++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -43,6 +43,7 @@ L3_START_KERNEL = pud_index(__START_KERN
 	.code64
 SYM_CODE_START_NOALIGN(startup_64)
 	UNWIND_HINT_EMPTY
+	ANNOTATE_NOENDBR
 	/*
 	 * At this point the CPU runs in 64bit mode CS.L = 1 CS.D = 0,
 	 * and someone has loaded an identity mapped page table
@@ -371,6 +372,7 @@ SYM_CODE_END(secondary_startup_64)
  */
 SYM_CODE_START(start_cpu0)
 	UNWIND_HINT_EMPTY
+	ANNOTATE_NOENDBR
 	movq	initial_stack(%rip), %rsp
 	jmp	.Ljump_to_C_code
 SYM_CODE_END(start_cpu0)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1947,14 +1947,27 @@ static int read_unwind_hints(struct objt
 
 		insn->hint = true;
 
-		if (opts.ibt && hint->type == UNWIND_HINT_TYPE_REGS_PARTIAL) {
+		if (opts.ibt) {
 			struct symbol *sym = find_symbol_by_offset(insn->sec, insn->offset);
 
-			if (sym && sym->bind == STB_GLOBAL &&
-			    insn->type != INSN_ENDBR && !insn->noendbr) {
-				WARN_FUNC("UNWIND_HINT_IRET_REGS without ENDBR",
-					  insn->sec, insn->offset);
+			if (!sym || sym->bind != STB_GLOBAL)
+				goto no_entry;
+
+			if (hint->type == UNWIND_HINT_TYPE_CALL && !hint->sp_reg) {
+				if (insn->type != INSN_ENDBR && !insn->noendbr) {
+					WARN_FUNC("UNWIND_HINT_EMPTY without ENDBR",
+							insn->sec, insn->offset);
+				}
+			}
+
+			if (hint->type == UNWIND_HINT_TYPE_REGS_PARTIAL) {
+				if (insn->type != INSN_ENDBR && !insn->noendbr) {
+					WARN_FUNC("UNWIND_HINT_IRET_REGS without ENDBR",
+							insn->sec, insn->offset);
+				}
 			}
+
+no_entry:		;
 		}
 
 		if (hint->type == UNWIND_HINT_TYPE_FUNC) {

[PATCH 3/7] objtool: Mark __ubsan_handle_builtin_unreachable() as noreturn

[Peter Zijlstra]

  fs/ntfs3/ntfs3.prelink.o: warning: objtool: ni_read_frame() falls through to next function ni_readpage_cmpr.cold()

That is in fact:

000000000000124a <ni_read_frame.cold>:
    124a:       44 89 e0                mov    %r12d,%eax
    124d:       0f b6 55 98             movzbl -0x68(%rbp),%edx
    1251:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        1254: R_X86_64_32S      .data+0x1380
    1258:       48 89 c6                mov    %rax,%rsi
    125b:       e8 00 00 00 00          call   1260 <ni_read_frame.cold+0x16>   125c: R_X86_64_PLT32    __ubsan_handle_shift_out_of_bounds-0x4
    1260:       48 8d 7d cc             lea    -0x34(%rbp),%rdi
    1264:       e8 00 00 00 00          call   1269 <ni_read_frame.cold+0x1f>   1265: R_X86_64_PLT32    __tsan_read4-0x4
    1269:       8b 45 cc                mov    -0x34(%rbp),%eax
    126c:       e9 00 00 00 00          jmp    1271 <ni_read_frame.cold+0x27>   126d: R_X86_64_PC32     .text+0x19109
    1271:       48 8b 75 a0             mov    -0x60(%rbp),%rsi
    1275:       48 63 d0                movslq %eax,%rdx
    1278:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        127b: R_X86_64_32S      .data+0x13a0
    127f:       89 45 88                mov    %eax,-0x78(%rbp)
    1282:       e8 00 00 00 00          call   1287 <ni_read_frame.cold+0x3d>   1283: R_X86_64_PLT32    __ubsan_handle_shift_out_of_bounds-0x4
    1287:       8b 45 88                mov    -0x78(%rbp),%eax
    128a:       e9 00 00 00 00          jmp    128f <ni_read_frame.cold+0x45>   128b: R_X86_64_PC32     .text+0x19098
    128f:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        1292: R_X86_64_32S      .data+0x11f0
    1296:       e8 00 00 00 00          call   129b <ni_readpage_cmpr.cold>     1297: R_X86_64_PLT32    __ubsan_handle_builtin_unreachable-0x4

000000000000129b <ni_readpage_cmpr.cold>:

Tell objtool that __ubsan_handle_builtin_unreachable() is a noreturn.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20220502091514.GB479834@worktop.programming.kicks-ass.net
---
 tools/objtool/check.c |    1 +
 1 file changed, 1 insertion(+)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -185,6 +185,7 @@ static bool __dead_end_function(struct o
 		"stop_this_cpu",
 		"__invalid_creds",
                "cpu_startup_entry",
+		"__ubsan_handle_builtin_unreachable",
 	};
 
 	if (!func)

[PATCH 4/7] x86/cpu: Elide KCSAN for cpu_has() and friends

[Peter Zijlstra]

As x86 uses the <asm-generic/bitops/instrumented-*.h> headers, the
regular forms of all bitops are instrumented with explicit calls to
KASAN and KCSAN checks. As these are explicit calls, these are not
suppressed by the noinstr function attribute.

This can result in calls to those check functions in noinstr code, which
objtool warns about:

vmlinux.o: warning: objtool: enter_from_user_mode+0x24: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x28: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x24: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x24: call to __kcsan_check_access() leaves .noinstr.text section

Prevent this by using the arch_*() bitops, which are the underlying
bitops without explciit instrumentation.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20220502111216.290518605@infradead.org
---
 arch/x86/include/asm/cpufeature.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -51,7 +51,7 @@ extern const char * const x86_power_flag
 extern const char * const x86_bug_flags[NBUGINTS*32];
 
 #define test_cpu_cap(c, bit)						\
-	 test_bit(bit, (unsigned long *)((c)->x86_capability))
+	 arch_test_bit(bit, (unsigned long *)((c)->x86_capability))
 
 /*
  * There are 32 bits/features in each mask word.  The high bits

[PATCH 5/7] jump_label,noinstr: Avoid instrumentation for JUMP_LABEL=n builds

[Peter Zijlstra]

When building x86_64 with JUMP_LABEL=n it's possible for
instrumentation to sneak into noinstr:

vmlinux.o: warning: objtool: exit_to_user_mode+0x14: call to static_key_count.constprop.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_exit_to_user_mode+0x2d: call to static_key_count.constprop.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_exit_to_user_mode+0x1b: call to static_key_count.constprop.0() leaves .noinstr.text section

Switch to arch_ prefixed atomic to avoid the explicit instrumentation.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/jump_label.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -256,9 +256,9 @@ extern void static_key_disable_cpuslocke
 #include <linux/atomic.h>
 #include <linux/bug.h>
 
-static inline int static_key_count(struct static_key *key)
+static __always_inline int static_key_count(struct static_key *key)
 {
-	return atomic_read(&key->enabled);
+	return arch_atomic_read(&key->enabled);
 }
 
 static __always_inline void jump_label_init(void)

[PATCH 6/7] x86: Always inline on_thread_stack() and current_top_of_stack()

[Peter Zijlstra]

Becaues GCC clearly lost it's marbles again...

vmlinux.o: warning: objtool: enter_from_user_mode+0x4e: call to on_thread_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x53: call to on_thread_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x4e: call to on_thread_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x4e: call to on_thread_stack() leaves .noinstr.text section

vmlinux.o: warning: objtool: enter_from_user_mode+0x4e: call to current_top_of_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x53: call to current_top_of_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x4e: call to current_top_of_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x4e: call to current_top_of_stack() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/processor.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -559,7 +559,7 @@ static __always_inline void native_swapg
 #endif
 }
 
-static inline unsigned long current_top_of_stack(void)
+static __always_inline unsigned long current_top_of_stack(void)
 {
 	/*
 	 *  We can't read directly from tss.sp0: sp0 on x86_32 is special in
@@ -569,7 +569,7 @@ static inline unsigned long current_top_
 	return this_cpu_read_stable(cpu_current_top_of_stack);
 }
 
-static inline bool on_thread_stack(void)
+static __always_inline bool on_thread_stack(void)
 {
 	return (unsigned long)(current_top_of_stack() -
 			       current_stack_pointer) < THREAD_SIZE;

[PATCH 7/7] context_tracking: Always inline empty stubs

[Peter Zijlstra]

Because GCC is seriously challenged..

vmlinux.o: warning: objtool: enter_from_user_mode+0x85: call to context_tracking_enabled() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x8f: call to context_tracking_enabled() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x85: call to context_tracking_enabled() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x85: call to context_tracking_enabled() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/context_tracking_state.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- a/include/linux/context_tracking_state.h
+++ b/include/linux/context_tracking_state.h
@@ -46,10 +46,10 @@ static __always_inline bool context_trac
 	return __this_cpu_read(context_tracking.state) == CONTEXT_USER;
 }
 #else
-static inline bool context_tracking_in_user(void) { return false; }
-static inline bool context_tracking_enabled(void) { return false; }
-static inline bool context_tracking_enabled_cpu(int cpu) { return false; }
-static inline bool context_tracking_enabled_this_cpu(void) { return false; }
+static __always_inline bool context_tracking_in_user(void) { return false; }
+static __always_inline bool context_tracking_enabled(void) { return false; }
+static __always_inline bool context_tracking_enabled_cpu(int cpu) { return false; }
+static __always_inline bool context_tracking_enabled_this_cpu(void) { return false; }
 #endif /* CONFIG_CONTEXT_TRACKING */
 
 #endif

Re: [PATCH 7/7] context_tracking: Always inline empty stubs

[Josh Poimboeuf]

On Thu, May 26, 2022 at 12:52:59PM +0200, Peter Zijlstra wrote:
> Because GCC is seriously challenged..

Or are these CONFIG_DEBUG_SECTION_MISMATCH?

-- 
Josh

Re: [PATCH 1/7] x86/entry: Anchor annotation

[Josh Poimboeuf]

On Thu, May 26, 2022 at 12:52:53PM +0200, Peter Zijlstra wrote:
> x86_64-allmodconfig gives:
> 
>   vmlinux.o: warning: objtool: entry_SYSCALL_compat+0x0: ANNOTATE_NOENDBR on ENDBR
> 
> This is due to:
> 
> 	ANNOTATE_NOENDBR
> SYM_CODE_END(entry_SYSENTER_compat)
> SYM_CODE_START(entry_SYSCALL_compat)
> 	UNWIND_HINT_EMPTY
> 	ENDBR
> 
> And the hint then landing on the first instruction of the next symbol,
> which happens to be ENDBR. Stick in an int3 to anchor the annotation
> in the previous symbol.
> 
> Fixes: 3e3f06950434 ("x86/ibt: Annotate text references")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/entry/entry_64_compat.S |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -124,6 +124,7 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_af
>  	jmp	.Lsysenter_flags_fixed
>  SYM_INNER_LABEL(__end_entry_SYSENTER_compat, SYM_L_GLOBAL)
>  	ANNOTATE_NOENDBR // is_sysenter_singlestep
> +	int3
>  SYM_CODE_END(entry_SYSENTER_compat)

Hm, I get the feeling this warning is pretty much useless anyway.  Can
we just get rid of the warning instead?

-- 
Josh

Re: [PATCH 2/7] objtool: Extend UNWIND_HINT based ENDBR rules

[Josh Poimboeuf]

On Thu, May 26, 2022 at 12:52:54PM +0200, Peter Zijlstra wrote:
> Extend the UNWIND hint driven rules for ENDBR placement. Currently
> objtool expects an ENDBR at any UNWINT_HINT_IRET_REGS that is at +0 of
> an STB_GLOBAL symbol, with the expectation that this is an exception
> entry point.
> 
> Extend this to also expect ENDBR at UNWIND_HINT_EMPTY at +0 for
> STB_GLOBAL symbols, with the expectation that these are also machine
> entry points (SYSCALL et. al.).
> 
> This guarantees all machine entry points are covered by objtool rules at
> the expense of a few additional false positives:
> 
>   vmlinux.o: warning: objtool: startup_64+0x0: UNWIND_HINT_EMPTY without ENDBR
>   vmlinux.o: warning: objtool: start_cpu0+0x0: UNWIND_HINT_EMPTY without ENDBR
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

I can't remember if this was my bright idea, but it feels kind of
arbitrary.  Hopefully there won't be a lot of false positives.

Anyway, won't SYSCALL-type symbols typically be referenced elsewhere in
the kernel and thus be found by the regular IBT validation?

Do you have any examples of where this warning would trigger if there
were a missing ENDBR?

-- 
Josh

Re: [PATCH 7/7] context_tracking: Always inline empty stubs

[Mark Rutland]

On Thu, May 26, 2022 at 08:02:06AM -0700, Josh Poimboeuf wrote:
> On Thu, May 26, 2022 at 12:52:59PM +0200, Peter Zijlstra wrote:
> > Because GCC is seriously challenged..
> 
> Or are these CONFIG_DEBUG_SECTION_MISMATCH?

Does it matter?

The fact that CONFIG_DEBUG_SECTION_MISMATCH=y forces GCC to place these
out-of-line is really a debugging aid, since it shows us what it could
potentially leave out-of-line. It's not the *only* reason GCC might do this
(e.g. I see sanitizers and other debug options often have similar effects on
arm64).

So FWIW, for the patch:

  Acked-by: Mark Rutland <mark.rutland@arm.com>

... as I'd like this for arm64, even if you don't beleive that's necessary for
x86.

Thanks,
Mark.

Re: [PATCH 7/7] context_tracking: Always inline empty stubs

[Josh Poimboeuf]

On Thu, May 26, 2022 at 04:10:52PM +0100, Mark Rutland wrote:
> On Thu, May 26, 2022 at 08:02:06AM -0700, Josh Poimboeuf wrote:
> > On Thu, May 26, 2022 at 12:52:59PM +0200, Peter Zijlstra wrote:
> > > Because GCC is seriously challenged..
> > 
> > Or are these CONFIG_DEBUG_SECTION_MISMATCH?
> 
> Does it matter?

Yes, because I believe the only thing this option is good for is
creating a bunch of useless '__always_inline' patches:

  https://lore.kernel.org/all/7fad83ecde03540e65677959034315f8fbb3755e.1649434832.git.jpoimboe@redhat.com/

-- 
Josh

Re: [PATCH 7/7] context_tracking: Always inline empty stubs

[Mark Rutland]

On Thu, May 26, 2022 at 08:16:54AM -0700, Josh Poimboeuf wrote:
> On Thu, May 26, 2022 at 04:10:52PM +0100, Mark Rutland wrote:
> > On Thu, May 26, 2022 at 08:02:06AM -0700, Josh Poimboeuf wrote:
> > > On Thu, May 26, 2022 at 12:52:59PM +0200, Peter Zijlstra wrote:
> > > > Because GCC is seriously challenged..
> > > 
> > > Or are these CONFIG_DEBUG_SECTION_MISMATCH?
> > 
> > Does it matter?
> 
> Yes, because I believe the only thing this option is good for is
> creating a bunch of useless '__always_inline' patches:
> 
>   https://lore.kernel.org/all/7fad83ecde03540e65677959034315f8fbb3755e.1649434832.git.jpoimboe@redhat.com/

Sure, but as I said, there are other reasons why the compiler can generate code
in this way, even if that's unlikely. Without `__always_inline` we don't
actually have a guarantee of inlining, so those warning *is* legitimate, even
if 99.99% of the time the compiler doesn't decide to generate code in a silly
way.

Thanks,
Mark.

Re: [PATCH 2/7] objtool: Extend UNWIND_HINT based ENDBR rules

[Peter Zijlstra]

On Thu, May 26, 2022 at 08:05:49AM -0700, Josh Poimboeuf wrote:
> On Thu, May 26, 2022 at 12:52:54PM +0200, Peter Zijlstra wrote:
> > Extend the UNWIND hint driven rules for ENDBR placement. Currently
> > objtool expects an ENDBR at any UNWINT_HINT_IRET_REGS that is at +0 of
> > an STB_GLOBAL symbol, with the expectation that this is an exception
> > entry point.
> > 
> > Extend this to also expect ENDBR at UNWIND_HINT_EMPTY at +0 for
> > STB_GLOBAL symbols, with the expectation that these are also machine
> > entry points (SYSCALL et. al.).
> > 
> > This guarantees all machine entry points are covered by objtool rules at
> > the expense of a few additional false positives:
> > 
> >   vmlinux.o: warning: objtool: startup_64+0x0: UNWIND_HINT_EMPTY without ENDBR
> >   vmlinux.o: warning: objtool: start_cpu0+0x0: UNWIND_HINT_EMPTY without ENDBR
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> I can't remember if this was my bright idea, but it feels kind of
> arbitrary.  Hopefully there won't be a lot of false positives.

The existing UNWIND_HINT_IRET_REGS at +0 was your idea, I'm just trying
to cover more.

> Anyway, won't SYSCALL-type symbols typically be referenced elsewhere in
> the kernel and thus be found by the regular IBT validation?

They do indeed, and that's what we've been relying on. I just figured it
would be more consistent to have rules covering all machine entry
points.

(also all the Xen entry points are EMPTY like).

> Do you have any examples of where this warning would trigger if there
> were a missing ENDBR?

No.

Anyway, I can drop these first two patches for now.

[tip: objtool/urgent] context_tracking: Always inline empty stubs

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     620f8d3bd3d5e82dff8cc591c831827d4beeae2e
Gitweb:        https://git.kernel.org/tip/620f8d3bd3d5e82dff8cc591c831827d4beeae2e
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Sat, 07 May 2022 13:35:37 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 27 May 2022 12:34:44 +02:00

context_tracking: Always inline empty stubs

Because GCC is seriously challenged..

vmlinux.o: warning: objtool: enter_from_user_mode+0x85: call to context_tracking_enabled() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x8f: call to context_tracking_enabled() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x85: call to context_tracking_enabled() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x85: call to context_tracking_enabled() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Link: https://lkml.kernel.org/r/20220526105958.134113388@infradead.org
---
 include/linux/context_tracking_state.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/context_tracking_state.h b/include/linux/context_tracking_state.h
index 65a60d3..ae1e63e 100644
--- a/include/linux/context_tracking_state.h
+++ b/include/linux/context_tracking_state.h
@@ -46,10 +46,10 @@ static __always_inline bool context_tracking_in_user(void)
 	return __this_cpu_read(context_tracking.state) == CONTEXT_USER;
 }
 #else
-static inline bool context_tracking_in_user(void) { return false; }
-static inline bool context_tracking_enabled(void) { return false; }
-static inline bool context_tracking_enabled_cpu(int cpu) { return false; }
-static inline bool context_tracking_enabled_this_cpu(void) { return false; }
+static __always_inline bool context_tracking_in_user(void) { return false; }
+static __always_inline bool context_tracking_enabled(void) { return false; }
+static __always_inline bool context_tracking_enabled_cpu(int cpu) { return false; }
+static __always_inline bool context_tracking_enabled_this_cpu(void) { return false; }
 #endif /* CONFIG_CONTEXT_TRACKING */
 
 #endif

[tip: objtool/urgent] x86: Always inline on_thread_stack() and current_top_of_stack()

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the objtool/urgent branch of tip:

Commit-ID:     1894a4030582472336c2873cb07911ce67e0d14e
Gitweb:        https://git.kernel.org/tip/1894a4030582472336c2873cb07911ce67e0d14e
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Sat, 07 May 2022 13:37:45 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 27 May 2022 12:34:44 +02:00

x86: Always inline on_thread_stack() and current_top_of_stack()

Becaues GCC clearly lost it's marbles again...

vmlinux.o: warning: objtool: enter_from_user_mode+0x4e: call to on_thread_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x53: call to on_thread_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x4e: call to on_thread_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x4e: call to on_thread_stack() leaves .noinstr.text section

vmlinux.o: warning: objtool: enter_from_user_mode+0x4e: call to current_top_of_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x53: call to current_top_of_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x4e: call to current_top_of_stack() leaves .noinstr.text section
vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x4e: call to current_top_of_stack() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20220526105958.071435483@infradead.org
---
 arch/x86/include/asm/processor.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 91d0f93..356308c 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -559,7 +559,7 @@ static __always_inline void native_swapgs(void)
 #endif
 }
 
-static inline unsigned long current_top_of_stack(void)
+static __always_inline unsigned long current_top_of_stack(void)
 {
 	/*
 	 *  We can't read directly from tss.sp0: sp0 on x86_32 is special in
@@ -569,7 +569,7 @@ static inline unsigned long current_top_of_stack(void)
 	return this_cpu_read_stable(cpu_current_top_of_stack);
 }
 
-static inline bool on_thread_stack(void)
+static __always_inline bool on_thread_stack(void)
 {
 	return (unsigned long)(current_top_of_stack() -
 			       current_stack_pointer) < THREAD_SIZE;

Re: [PATCH 4/7] x86/cpu: Elide KCSAN for cpu_has() and friends

[Marco Elver]

On Thu, May 26, 2022 at 12:52PM +0200, Peter Zijlstra wrote:
> As x86 uses the <asm-generic/bitops/instrumented-*.h> headers, the
> regular forms of all bitops are instrumented with explicit calls to
> KASAN and KCSAN checks. As these are explicit calls, these are not
> suppressed by the noinstr function attribute.
> 
> This can result in calls to those check functions in noinstr code, which
> objtool warns about:
> 
> vmlinux.o: warning: objtool: enter_from_user_mode+0x24: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x28: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x24: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x24: call to __kcsan_check_access() leaves .noinstr.text section
> 
> Prevent this by using the arch_*() bitops, which are the underlying
> bitops without explciit instrumentation.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20220502111216.290518605@infradead.org

Acked-by: Marco Elver <elver@google.com>

> ---
>  arch/x86/include/asm/cpufeature.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -51,7 +51,7 @@ extern const char * const x86_power_flag
>  extern const char * const x86_bug_flags[NBUGINTS*32];
>  
>  #define test_cpu_cap(c, bit)						\
> -	 test_bit(bit, (unsigned long *)((c)->x86_capability))
> +	 arch_test_bit(bit, (unsigned long *)((c)->x86_capability))
>  
>  /*
>   * There are 32 bits/features in each mask word.  The high bits
> 
> 

Re: [PATCH 3/7] objtool: Mark __ubsan_handle_builtin_unreachable() as noreturn

[Marco Elver]

On Thu, May 26, 2022 at 12:52PM +0200, Peter Zijlstra wrote:
>   fs/ntfs3/ntfs3.prelink.o: warning: objtool: ni_read_frame() falls through to next function ni_readpage_cmpr.cold()
> 
> That is in fact:
> 
> 000000000000124a <ni_read_frame.cold>:
>     124a:       44 89 e0                mov    %r12d,%eax
>     124d:       0f b6 55 98             movzbl -0x68(%rbp),%edx
>     1251:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        1254: R_X86_64_32S      .data+0x1380
>     1258:       48 89 c6                mov    %rax,%rsi
>     125b:       e8 00 00 00 00          call   1260 <ni_read_frame.cold+0x16>   125c: R_X86_64_PLT32    __ubsan_handle_shift_out_of_bounds-0x4
>     1260:       48 8d 7d cc             lea    -0x34(%rbp),%rdi
>     1264:       e8 00 00 00 00          call   1269 <ni_read_frame.cold+0x1f>   1265: R_X86_64_PLT32    __tsan_read4-0x4
>     1269:       8b 45 cc                mov    -0x34(%rbp),%eax
>     126c:       e9 00 00 00 00          jmp    1271 <ni_read_frame.cold+0x27>   126d: R_X86_64_PC32     .text+0x19109
>     1271:       48 8b 75 a0             mov    -0x60(%rbp),%rsi
>     1275:       48 63 d0                movslq %eax,%rdx
>     1278:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        127b: R_X86_64_32S      .data+0x13a0
>     127f:       89 45 88                mov    %eax,-0x78(%rbp)
>     1282:       e8 00 00 00 00          call   1287 <ni_read_frame.cold+0x3d>   1283: R_X86_64_PLT32    __ubsan_handle_shift_out_of_bounds-0x4
>     1287:       8b 45 88                mov    -0x78(%rbp),%eax
>     128a:       e9 00 00 00 00          jmp    128f <ni_read_frame.cold+0x45>   128b: R_X86_64_PC32     .text+0x19098
>     128f:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        1292: R_X86_64_32S      .data+0x11f0
>     1296:       e8 00 00 00 00          call   129b <ni_readpage_cmpr.cold>     1297: R_X86_64_PLT32    __ubsan_handle_builtin_unreachable-0x4
> 
> 000000000000129b <ni_readpage_cmpr.cold>:
> 
> Tell objtool that __ubsan_handle_builtin_unreachable() is a noreturn.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20220502091514.GB479834@worktop.programming.kicks-ass.net

Acked-by: Marco Elver <elver@google.com>

> ---
>  tools/objtool/check.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -185,6 +185,7 @@ static bool __dead_end_function(struct o
>  		"stop_this_cpu",
>  		"__invalid_creds",
>                 "cpu_startup_entry",
> +		"__ubsan_handle_builtin_unreachable",
>  	};
>  
>  	if (!func)
> 
> 

Re: [PATCH 5/7] jump_label,noinstr: Avoid instrumentation for JUMP_LABEL=n builds

[Marco Elver]

On Thu, May 26, 2022 at 12:52PM +0200, Peter Zijlstra wrote:
> When building x86_64 with JUMP_LABEL=n it's possible for
> instrumentation to sneak into noinstr:
> 
> vmlinux.o: warning: objtool: exit_to_user_mode+0x14: call to static_key_count.constprop.0() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_exit_to_user_mode+0x2d: call to static_key_count.constprop.0() leaves .noinstr.text section
> vmlinux.o: warning: objtool: irqentry_exit_to_user_mode+0x1b: call to static_key_count.constprop.0() leaves .noinstr.text section
> 
> Switch to arch_ prefixed atomic to avoid the explicit instrumentation.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Acked-by: Marco Elver <elver@google.com>

> ---
>  include/linux/jump_label.h |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -256,9 +256,9 @@ extern void static_key_disable_cpuslocke
>  #include <linux/atomic.h>
>  #include <linux/bug.h>
>  
> -static inline int static_key_count(struct static_key *key)
> +static __always_inline int static_key_count(struct static_key *key)
>  {
> -	return atomic_read(&key->enabled);
> +	return arch_atomic_read(&key->enabled);
>  }
>  
>  static __always_inline void jump_label_init(void)
> 
> 

Re: [PATCH 6/7] x86: Always inline on_thread_stack() and current_top_of_stack()

[Marco Elver]

On Thu, May 26, 2022 at 12:52PM +0200, Peter Zijlstra wrote:
> Becaues GCC clearly lost it's marbles again...
> 

This must have been with some of the big sanitizers though, right?

> vmlinux.o: warning: objtool: enter_from_user_mode+0x4e: call to on_thread_stack() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x53: call to on_thread_stack() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x4e: call to on_thread_stack() leaves .noinstr.text section
> vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x4e: call to on_thread_stack() leaves .noinstr.text section
> 
> vmlinux.o: warning: objtool: enter_from_user_mode+0x4e: call to current_top_of_stack() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x53: call to current_top_of_stack() leaves .noinstr.text section
> vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0x4e: call to current_top_of_stack() leaves .noinstr.text section
> vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0x4e: call to current_top_of_stack() leaves .noinstr.text section
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Acked-by: Marco Elver <elver@google.com>

> ---
>  arch/x86/include/asm/processor.h |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -559,7 +559,7 @@ static __always_inline void native_swapg
>  #endif
>  }
>  
> -static inline unsigned long current_top_of_stack(void)
> +static __always_inline unsigned long current_top_of_stack(void)
>  {
>  	/*
>  	 *  We can't read directly from tss.sp0: sp0 on x86_32 is special in
> @@ -569,7 +569,7 @@ static inline unsigned long current_top_
>  	return this_cpu_read_stable(cpu_current_top_of_stack);
>  }
>  
> -static inline bool on_thread_stack(void)
> +static __always_inline bool on_thread_stack(void)
>  {
>  	return (unsigned long)(current_top_of_stack() -
>  			       current_stack_pointer) < THREAD_SIZE;
> 
>