Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
On Thu, May 05, 2022 at 06:09:45PM -0700, Josh Poimboeuf wrote: > With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative > pointers are calculated weirdly: based on the beginning of the bug_entry > struct address, rather than their respective pointer addresses. > > Make the relative pointers less surprising to both humans and tools by > calculating them the normal way. > > Signed-off-by: Josh Poimboeuf > --- > arch/arm64/include/asm/asm-bug.h | 4 ++-- Acked-by: Catalin Marinas
Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
On Mon, May 09, 2022 at 10:31:14PM +1000, Michael Ellerman wrote: > Embarrassingly, we have another copy of the logic, used in the C > versions, they need updating too: Oops, thanks for finding that. > With that added it seems to be working correctly for me. > > Acked-by: Michael Ellerman (powerpc) Thanks! -- Josh
Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
Josh Poimboeuf writes: > With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative > pointers are calculated weirdly: based on the beginning of the bug_entry > struct address, rather than their respective pointer addresses. > > Make the relative pointers less surprising to both humans and tools by > calculating them the normal way. > > Signed-off-by: Josh Poimboeuf > --- ... > diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h > index ecbae1832de3..76252576d889 100644 > --- a/arch/powerpc/include/asm/bug.h > +++ b/arch/powerpc/include/asm/bug.h > @@ -13,7 +13,8 @@ > #ifdef CONFIG_DEBUG_BUGVERBOSE > .macro __EMIT_BUG_ENTRY addr,file,line,flags >.section __bug_table,"aw" > -5001: .4byte \addr - 5001b, 5002f - 5001b > +5001: .4byte \addr - . > + .4byte 5002f - . >.short \line, \flags >.org 5001b+BUG_ENTRY_SIZE >.previous > @@ -24,7 +25,7 @@ > #else > .macro __EMIT_BUG_ENTRY addr,file,line,flags >.section __bug_table,"aw" > -5001: .4byte \addr - 5001b > +5001: .4byte \addr - . >.short \flags >.org 5001b+BUG_ENTRY_SIZE >.previous Embarrassingly, we have another copy of the logic, used in the C versions, they need updating too: diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index ecbae1832de3..3fde35fd67f8 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -49,14 +49,14 @@ #ifdef CONFIG_DEBUG_BUGVERBOSE #define _EMIT_BUG_ENTRY\ ".section __bug_table,\"aw\"\n" \ - "2:\t.4byte 1b - 2b, %0 - 2b\n" \ + "2:\t.4byte 1b - ., %0 - .\n" \ "\t.short %1, %2\n" \ ".org 2b+%3\n" \ ".previous\n" #else #define _EMIT_BUG_ENTRY\ ".section __bug_table,\"aw\"\n" \ - "2:\t.4byte 1b - 2b\n" \ + "2:\t.4byte 1b - .\n" \ "\t.short %2\n" \ ".org 2b+%3\n" \ ".previous\n" With that added it seems to be working correctly for me. Acked-by: Michael Ellerman (powerpc) cheers
Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
On Thu, May 05, 2022 at 06:09:45PM -0700, Josh Poimboeuf wrote: > With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative > pointers are calculated weirdly: based on the beginning of the bug_entry > struct address, rather than their respective pointer addresses. > > Make the relative pointers less surprising to both humans and tools by > calculating them the normal way. > > Signed-off-by: Josh Poimboeuf This looks good to me. Just in case, I gave this a spin on arm64 defconfig atop v5.18-rc4. This builds cleanly with both GCC 11.1.0 and LLVM 14.0.0, and works correctly in testing on both with the LKDTM BUG/WARNING/WARNING_MESSAGE tests, i.e. echo WARNING > /sys/kernel/debug/provoke-crash/DIRECT echo WARNING_MESSAGE > /sys/kernel/debug/provoke-crash/DIRECT echo BUG > /sys/kernel/debug/provoke-crash/DIRECT FWIW: Reviewed-by: Mark Rutland Tested-by: Mark Rutland [arm64] As an aside (and for anyone else trying to duplicate my results), on arm64 there's a latent issue (prior to this patch) where BUG() will always result in a WARN_ON_ONCE() in rcu_eqs_enter(). Since BUG() uses a BRK, and we treat the BRK exception as an NMI, when we kill the task we do that in NMI context, but schedule another task in regular task context, and RCU doesn't like that: # echo BUG > /sys/kernel/debug/provoke-crash/DIRECT [ 28.284180] lkdtm: Performing direct entry BUG [ 28.285052] [ cut here ] [ 28.285940] kernel BUG at drivers/misc/lkdtm/bugs.c:78! [ 28.287008] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP [ 28.288143] Modules linked in: [ 28.288798] CPU: 0 PID: 151 Comm: bash Not tainted 5.18.0-rc4 #1 [ 28.290040] Hardware name: linux,dummy-virt (DT) [ 28.290979] pstate: 6045 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 28.292380] pc : lkdtm_BUG+0x4/0xc [ 28.293084] lr : lkdtm_do_action+0x24/0x30 [ 28.293923] sp : 883bbce0 [ 28.294624] x29: 883bbce0 x28: 3c574344 x27: [ 28.296057] x26: x25: a4bc19c480b0 x24: 883bbdf0 [ 28.297493] x23: 0004 x22: 3c57440f3000 x21: a4bc1a0bfba0 [ 28.298933] x20: a4bc19c480c0 x19: 0001 x18: [ 28.300369] x17: x16: x15: 0720072007200720 [ 28.301823] x14: 0720072007200747 x13: a4bc1a8d2520 x12: 03b1 [ 28.303257] x11: 013b x10: a4bc1a92a520 x9 : a4bc1a8d2520 [ 28.304689] x8 : efff x7 : a4bc1a92a520 x6 : [ 28.306120] x5 : x4 : 3c57bfbcc9e8 x3 : [ 28.307550] x2 : x1 : 3c574344 x0 : a4bc19279284 [ 28.308981] Call trace: [ 28.309496] lkdtm_BUG+0x4/0xc [ 28.310134] direct_entry+0x11c/0x1cc [ 28.310888] full_proxy_write+0x60/0xbc [ 28.311690] vfs_write+0xc4/0x2a4 [ 28.312383] ksys_write+0x68/0xf4 [ 28.313056] __arm64_sys_write+0x20/0x2c [ 28.313851] invoke_syscall+0x48/0x114 [ 28.314623] el0_svc_common.constprop.0+0xd4/0xfc [ 28.315584] do_el0_svc+0x28/0x90 [ 28.316276] el0_svc+0x34/0xb0 [ 28.316917] el0t_64_sync_handler+0xa4/0x130 [ 28.317786] el0t_64_sync+0x18c/0x190 [ 28.318560] Code: b90027e0 17ea 941b4d4c d503245f (d421) [ 28.319796] ---[ end trace ]--- [ 28.320736] note: bash[151] exited with preempt_count 1 [ 28.329377] [ cut here ] [ 28.330327] WARNING: CPU: 0 PID: 0 at kernel/rcu/tree.c:624 rcu_eqs_enter.constprop.0+0x7c/0x84 [ 28.332103] Modules linked in: [ 28.332757] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G D 5.18.0-rc4 #1 [ 28.334355] Hardware name: linux,dummy-virt (DT) [ 28.335318] pstate: 204000c5 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 28.336745] pc : rcu_eqs_enter.constprop.0+0x7c/0x84 [ 28.337766] lr : rcu_idle_enter+0x10/0x1c [ 28.338609] sp : a4bc1a8b3d40 [ 28.339309] x29: a4bc1a8b3d40 x28: 41168458 x27: [ 28.340788] x26: a4bc1a8c3340 x25: x24: [ 28.342255] x23: a4bc1a8b9b4c x22: a4bc1a37a6f8 x21: a4bc1a8b9a38 [ 28.343705] x20: a4bc1a8b9b40 x19: 3c57bfbd4800 x18: [ 28.345159] x17: x16: x15: 06a1d2912376 [ 28.346632] x14: 018a x13: 018a x12: [ 28.348089] x11: 0001 x10: 0a50 x9 : a4bc1a8b3ce0 [ 28.349551] x8 : a4bc1a8c3df0 x7 : 3c57bfbd3b80 x6 : 000154de2486 [ 28.351040] x5 : 03ff x4 : 0a5c x3 : a4bc1a8b79c0 [ 28.352505] x2 : 0a5c x1 : 4002 x0 : 4000 [ 28.353966] Call trace: [ 28.354496] rcu_eqs_enter.constprop.0+0x7c/0x84 [ 28.355467] rcu_idle_enter+0x10/0x1c [ 28.356230] default_idle_call+0x20/0x6c [ 28.357061] do_idle+0x22c/0x29c [ 28.357743]
Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
Josh Poimboeuf writes: > With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative > pointers are calculated weirdly: based on the beginning of the bug_entry > struct address, rather than their respective pointer addresses. > > Make the relative pointers less surprising to both humans and tools by > calculating them the normal way. > > Signed-off-by: Josh Poimboeuf Acked-by: Sven Schnelle # s390 > --- > arch/arm64/include/asm/asm-bug.h | 4 ++-- > arch/powerpc/include/asm/bug.h | 5 +++-- > arch/riscv/include/asm/bug.h | 4 ++-- > arch/s390/include/asm/bug.h | 5 +++-- > arch/x86/include/asm/bug.h | 2 +- > lib/bug.c| 15 +++ > 6 files changed, 18 insertions(+), 17 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-bug.h > b/arch/arm64/include/asm/asm-bug.h > index 03f52f84a4f3..c762038ba400 100644 > --- a/arch/arm64/include/asm/asm-bug.h > +++ b/arch/arm64/include/asm/asm-bug.h > @@ -14,7 +14,7 @@ > 14472: .string file; \ > .popsection;\ > \ > - .long 14472b - 14470b; \ > + .long 14472b - .; \ > .short line; > #else > #define _BUGVERBOSE_LOCATION(file, line) > @@ -25,7 +25,7 @@ > #define __BUG_ENTRY(flags) \ > .pushsection __bug_table,"aw"; \ > .align 2; \ > - 14470: .long 14471f - 14470b; \ > + 14470: .long 14471f - .; \ > _BUGVERBOSE_LOCATION(__FILE__, __LINE__) \ > .short flags; \ > .popsection;\ > diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h > index ecbae1832de3..76252576d889 100644 > --- a/arch/powerpc/include/asm/bug.h > +++ b/arch/powerpc/include/asm/bug.h > @@ -13,7 +13,8 @@ > #ifdef CONFIG_DEBUG_BUGVERBOSE > .macro __EMIT_BUG_ENTRY addr,file,line,flags >.section __bug_table,"aw" > -5001: .4byte \addr - 5001b, 5002f - 5001b > +5001: .4byte \addr - . > + .4byte 5002f - . >.short \line, \flags >.org 5001b+BUG_ENTRY_SIZE >.previous > @@ -24,7 +25,7 @@ > #else > .macro __EMIT_BUG_ENTRY addr,file,line,flags >.section __bug_table,"aw" > -5001: .4byte \addr - 5001b > +5001: .4byte \addr - . >.short \flags >.org 5001b+BUG_ENTRY_SIZE >.previous > diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h > index d3804a2f9aad..1aaea81fb141 100644 > --- a/arch/riscv/include/asm/bug.h > +++ b/arch/riscv/include/asm/bug.h > @@ -30,8 +30,8 @@ > typedef u32 bug_insn_t; > > #ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS > -#define __BUG_ENTRY_ADDR RISCV_INT " 1b - 2b" > -#define __BUG_ENTRY_FILE RISCV_INT " %0 - 2b" > +#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ." > +#define __BUG_ENTRY_FILE RISCV_INT " %0 - ." > #else > #define __BUG_ENTRY_ADDR RISCV_PTR " 1b" > #define __BUG_ENTRY_FILE RISCV_PTR " %0" > diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h > index 0b25f28351ed..aebe1e22c7be 100644 > --- a/arch/s390/include/asm/bug.h > +++ b/arch/s390/include/asm/bug.h > @@ -15,7 +15,8 @@ > "1: .asciz \""__FILE__"\"\n" \ > ".previous\n" \ > ".section __bug_table,\"awM\",@progbits,%2\n" \ > - "2: .long 0b-2b,1b-2b\n" \ > + "2: .long 0b-.\n" \ > + " .long 1b-.\n" \ > " .short %0,%1\n"\ > " .org2b+%2\n"\ > ".previous\n" \ > @@ -30,7 +31,7 @@ > asm_inline volatile(\ > "0: mc 0,0\n" \ > ".section __bug_table,\"awM\",@progbits,%1\n" \ > - "1: .long 0b-1b\n"\ > + "1: .long 0b-.\n" \ > " .short %0\n" \ > " .org1b+%1\n"\ > ".previous\n" \ > diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h > index aaf0cb0db4ae..a3ec87d198ac 100644 > --- a/arch/x86/include/asm/bug.h > +++ b/arch/x86/include/asm/bug.h > @@ -18,7 +18,7 @@ > #ifdef CONFIG_X86_32 > # define __BUG_REL(val) ".long " __stringify(val) > #else > -# define __BUG_REL(val) ".long " __stringify(val) " - 2b" > +#
Re: [PATCH] bug: Use normal relative pointers in 'struct bug_entry'
On Thu, May 05, 2022 at 06:09:45PM -0700, Josh Poimboeuf wrote: > With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative > pointers are calculated weirdly: based on the beginning of the bug_entry > struct address, rather than their respective pointer addresses. > > Make the relative pointers less surprising to both humans and tools by > calculating them the normal way. > > Signed-off-by: Josh Poimboeuf Acked-by: Peter Zijlstra (Intel)
[PATCH] bug: Use normal relative pointers in 'struct bug_entry'
With CONFIG_GENERIC_BUG_RELATIVE_POINTERS, the addr/file relative pointers are calculated weirdly: based on the beginning of the bug_entry struct address, rather than their respective pointer addresses. Make the relative pointers less surprising to both humans and tools by calculating them the normal way. Signed-off-by: Josh Poimboeuf --- arch/arm64/include/asm/asm-bug.h | 4 ++-- arch/powerpc/include/asm/bug.h | 5 +++-- arch/riscv/include/asm/bug.h | 4 ++-- arch/s390/include/asm/bug.h | 5 +++-- arch/x86/include/asm/bug.h | 2 +- lib/bug.c| 15 +++ 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h index 03f52f84a4f3..c762038ba400 100644 --- a/arch/arm64/include/asm/asm-bug.h +++ b/arch/arm64/include/asm/asm-bug.h @@ -14,7 +14,7 @@ 14472: .string file; \ .popsection;\ \ - .long 14472b - 14470b; \ + .long 14472b - .; \ .short line; #else #define _BUGVERBOSE_LOCATION(file, line) @@ -25,7 +25,7 @@ #define __BUG_ENTRY(flags) \ .pushsection __bug_table,"aw"; \ .align 2; \ - 14470: .long 14471f - 14470b; \ + 14470: .long 14471f - .; \ _BUGVERBOSE_LOCATION(__FILE__, __LINE__) \ .short flags; \ .popsection;\ diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index ecbae1832de3..76252576d889 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -13,7 +13,8 @@ #ifdef CONFIG_DEBUG_BUGVERBOSE .macro __EMIT_BUG_ENTRY addr,file,line,flags .section __bug_table,"aw" -5001: .4byte \addr - 5001b, 5002f - 5001b +5001: .4byte \addr - . +.4byte 5002f - . .short \line, \flags .org 5001b+BUG_ENTRY_SIZE .previous @@ -24,7 +25,7 @@ #else .macro __EMIT_BUG_ENTRY addr,file,line,flags .section __bug_table,"aw" -5001: .4byte \addr - 5001b +5001: .4byte \addr - . .short \flags .org 5001b+BUG_ENTRY_SIZE .previous diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h index d3804a2f9aad..1aaea81fb141 100644 --- a/arch/riscv/include/asm/bug.h +++ b/arch/riscv/include/asm/bug.h @@ -30,8 +30,8 @@ typedef u32 bug_insn_t; #ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS -#define __BUG_ENTRY_ADDR RISCV_INT " 1b - 2b" -#define __BUG_ENTRY_FILE RISCV_INT " %0 - 2b" +#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ." +#define __BUG_ENTRY_FILE RISCV_INT " %0 - ." #else #define __BUG_ENTRY_ADDR RISCV_PTR " 1b" #define __BUG_ENTRY_FILE RISCV_PTR " %0" diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h index 0b25f28351ed..aebe1e22c7be 100644 --- a/arch/s390/include/asm/bug.h +++ b/arch/s390/include/asm/bug.h @@ -15,7 +15,8 @@ "1: .asciz \""__FILE__"\"\n" \ ".previous\n" \ ".section __bug_table,\"awM\",@progbits,%2\n" \ - "2: .long 0b-2b,1b-2b\n" \ + "2: .long 0b-.\n" \ + " .long 1b-.\n" \ " .short %0,%1\n"\ " .org2b+%2\n"\ ".previous\n" \ @@ -30,7 +31,7 @@ asm_inline volatile(\ "0: mc 0,0\n" \ ".section __bug_table,\"awM\",@progbits,%1\n" \ - "1: .long 0b-1b\n"\ + "1: .long 0b-.\n" \ " .short %0\n" \ " .org1b+%1\n"\ ".previous\n" \ diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index aaf0cb0db4ae..a3ec87d198ac 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -18,7 +18,7 @@ #ifdef CONFIG_X86_32 # define __BUG_REL(val)".long " __stringify(val) #else -# define __BUG_REL(val)".long " __stringify(val) " - 2b" +# define __BUG_REL(val)".long " __stringify(val) " - ." #endif #ifdef CONFIG_DEBUG_BUGVERBOSE diff --git a/lib/bug.c b/lib/bug.c index 45a0584f6541..c223a2575b72 100644 --- a/lib/bug.c +++
