| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 31 Dec 2021 12:48:07 +0100
From: Borislav Petkov <bp@...en8.de>
To: "Chen, Rong A" <rong.a.chen@...el.com>,
Peter Zijlstra <peterz@...radead.org>
Cc: kernel test robot <lkp@...el.com>, x86-ml <x86@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [tip:ras/core] BUILD SUCCESS
de768416b203ac84e02a757b782a32efb388476f
On Fri, Dec 31, 2021 at 08:46:52AM +0800, Chen, Rong A wrote:
> Hi Borislav,
>
> Below is the report:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ras/core
> head: de768416b203ac84e02a757b782a32efb388476f
> commit: b4813539d37fa31fed62cdfab7bd2dd8929c5b2e [15/23] x86/mce: Mark
> mce_end() noinstr
> config: x86_64-buildonly-randconfig-r006-20211228 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
> # https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=b4813539d37fa31fed62cdfab7bd2dd8929c5b2e
> git remote add tip
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> git fetch --no-tags tip ras/core
> git checkout b4813539d37fa31fed62cdfab7bd2dd8929c5b2e
> # save the config file to linux build tree
> mkdir build_dir
> make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@...el.com>
>
> All warnings (new ones prefixed by >>):
>
> >> vmlinux.o: warning: objtool: do_machine_check()+0x59a: call to test_bit()
> leaves .noinstr.text section
Thanks!
Hmm, so staring at this, it looks kinda weird. Lemme add Peter.
So with your .config I see
vmlinux.o: warning: objtool: mce_start()+0x6a: call to clear_bit() leaves .noinstr.text section
This one generates a call to clear_bit() even though that function is
inline. And yeah, I know, it doesn't always inline it and looking at the
asm, it does generate a clear_bit function down in that same compilation
unit:
.size set_bit, .-set_bit
.type clear_bit, @function
clear_bit:
pushq %rbp #
movq %rsp, %rbp #,
pushq %r12 #
movq %rdi, %r12 # tmp84, nr
pushq %rbx #
# ./include/asm-generic/bitops/instrumented-atomic.h:40: {
movq %rsi, %rbx # tmp85, addr
# ./arch/x86/include/asm/bitops.h:79: asm volatile(LOCK_PREFIX __ASM_SIZE(btr) " %1,%0"
call __sanitizer_cov_trace_pc #
#APP
# 79 "./arch/x86/include/asm/bitops.h" 1
btrq %r12,(%rbx) # nr, MEM[(volatile long int *)addr_2(D)]
# 0 "" 2
# ./include/asm-generic/bitops/instrumented-atomic.h:43: }
#NO_APP
popq %rbx #
popq %r12 #
popq %rbp #
ret
.size clear_bit, .-clear_bit
.type test_bit, @function
I guess the compiler decided not to inline the function. The fix for
that is easy, see below.
The next one is:
vmlinux.o: warning: objtool: mce_read_aux()+0x53: call to mca_msr_reg() leaves .noinstr.text section
That one needs instrumentation range widening too, see below.
And the next one is:
vmlinux.o: warning: objtool: do_machine_check()+0xc9: call to mce_no_way_out() leaves .noinstr.text section
mce_no_way_out() is called only once by do_machine_check(), I guess it can be __always_inline.
Which then leads to:
vmlinux.o: warning: objtool: do_machine_check()+0x48e: call to test_bit() leaves .noinstr.text section
and that is again those *_bit() functions which do not get inlined but
actual calls to them get generated:
test_bit:
pushq %rbp #
movq %rsp, %rbp #,
pushq %r12 #
movq %rdi, %r12 # tmp87, nr
pushq %rbx #
# ./include/asm-generic/bitops/instrumented-non-atomic.h:133: {
movq %rsi, %rbx # tmp88, addr
# ./arch/x86/include/asm/bitops.h:214: asm volatile(__ASM_SIZE(bt) " %2,%1"
call __sanitizer_cov_trace_pc #
#APP
# 214 "./arch/x86/include/asm/bitops.h" 1
btq %r12,(%rbx) # nr, MEM[(long unsigned int *)addr_5(D)]
/* output condition code c*/
# 0 "" 2
# ./include/asm-generic/bitops/instrumented-non-atomic.h:136: }
#NO_APP
popq %rbx #
# ./arch/x86/include/asm/bitops.h:214: asm volatile(__ASM_SIZE(bt) " %2,%1"
setc %al #, oldbit
# ./include/asm-generic/bitops/instrumented-non-atomic.h:136: }
popq %r12 #
popq %rbp #
ret
.size test_bit, .-test_bit
---
if I do:
diff --git a/arch/x86/kernel/cpu/mce/Makefile b/arch/x86/kernel/cpu/mce/Makefile
index 015856abdbb1..3afa0585bc75 100644
--- a/arch/x86/kernel/cpu/mce/Makefile
+++ b/arch/x86/kernel/cpu/mce/Makefile
@@ -1,4 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
+#
+
+KCOV_INSTRUMENT_core.o := n
+
then test_bit still remains a call:
---
.type test_bit, @function
test_bit:
# ./arch/x86/include/asm/bitops.h:214: asm volatile(__ASM_SIZE(bt) " %2,%1"
#APP
# 214 "./arch/x86/include/asm/bitops.h" 1
btq %rdi,(%rsi) # tmp87, MEM[(long unsigned int *)addr_5(D)]
/* output condition code c*/
# 0 "" 2
#NO_APP
setc %al #, oldbit
# ./include/asm-generic/bitops/instrumented-non-atomic.h:136: }
ret
.size test_bit, .-test_bit
---
and that *should* *get* inlined, for chrissakes! It is *two* insns!
Disabling CONFIG_KCOV doesn't help either - those *_bit() manipulation
functions are still not inlined.
Anyway, here's what I have so far:
---
diff --git a/arch/x86/kernel/cpu/mce/Makefile b/arch/x86/kernel/cpu/mce/Makefile
index 015856abdbb1..3afa0585bc75 100644
--- a/arch/x86/kernel/cpu/mce/Makefile
+++ b/arch/x86/kernel/cpu/mce/Makefile
@@ -1,4 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
+#
+
+KCOV_INSTRUMENT_core.o := n
+
obj-y = core.o severity.o genpool.o
obj-$(CONFIG_X86_ANCIENT_MCE) += winchip.o p5.o
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 5818b837fd4d..aca1408d2d93 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -648,7 +648,7 @@ static struct notifier_block mce_default_nb = {
/*
* Read ADDR and MISC registers.
*/
-static noinstr void mce_read_aux(struct mce *m, int i)
+static void mce_read_aux(struct mce *m, int i)
{
if (m->status & MCI_STATUS_MISCV)
m->misc = mce_rdmsrl(mca_msr_reg(i, MCA_MISC));
@@ -838,8 +838,8 @@ static void quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs)
* Do a quick check if any of the events requires a panic.
* This decides if we keep the events around or clear them.
*/
-static int mce_no_way_out(struct mce *m, char **msg, unsigned long *validp,
- struct pt_regs *regs)
+static __always_inline int mce_no_way_out(struct mce *m, char **msg, unsigned long *validp,
+ struct pt_regs *regs)
{
char *tmp = *msg;
int i;
@@ -1021,11 +1021,12 @@ static noinstr int mce_start(int *no_way_out)
* is updated before mce_callin.
*/
order = atomic_inc_return(&mce_callin);
- cpumask_clear_cpu(smp_processor_id(), &mce_missing_cpus);
/* Enable instrumentation around calls to external facilities */
instrumentation_begin();
+ cpumask_clear_cpu(smp_processor_id(), &mce_missing_cpus);
+
/*
* Wait for everyone.
*/
@@ -1250,16 +1251,17 @@ __mc_scan_banks(struct mce *m, struct pt_regs *regs, struct mce *final,
if (severity == MCE_NO_SEVERITY)
continue;
+ /*
+ * Enable instrumentation around the MCE logging which is
+ * done in #MC context, where instrumentation is disabled.
+ */
+ instrumentation_begin();
+
mce_read_aux(m, i);
/* assuming valid severity level != 0 */
m->severity = severity;
- /*
- * Enable instrumentation around the mce_log() call which is
- * done in #MC context, where instrumentation is disabled.
- */
- instrumentation_begin();
mce_log(m);
instrumentation_end();
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists