[Crash-utility] Re: arm64: Fix bt command show wrong stacktrace on ramdump source

Thank you for the update. On Mon, Jul 15, 2024 at 11:59 AM cb126yx <cb126yx(a)126.com&gt; wrote: > > Sorry, as the origin patch has aleardy been added to patch log, > > so the below comment in arm64.c is Inappropriate and inconsistent: > > * gki related commit url: > > * https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@arm.com/ > > so delete these url link comment in V4 version. > > > patch v4 here: > > ===> > > > From ab61e60987e4a835e41e4c19822174045888b84e Mon Sep 17 00:00:00 2001 > > From: bevis_chen <bevis_chen(a)asus.com&gt; > > Date: Wed, 3 Jul 2024 15:05:44 +0800 > > Subject: [PATCH] arm64: Fix bt command show wrong stacktrace on ramdump source > > > If we use crash to parse ramdump(Qcom phone device) rathen than vmcore. > > Start command should be like: crash vmlinux --kaslr=xxx DDRCS0_0.BIN@0x0000000080000000,... --machdep vabits_actual=39 > > Then We will see bt command show misleading backtrace information below: > > > crash> bt 16930 > > PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase Backgr" > > #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > > #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 > > #2 [ffffffc034c438b0] __kvm_nvhe_$d.2314 at 86c54c6004ceff80 > > #3 [ffffffc034c43950] __kvm_nvhe_$d.2314 at 55d6f96003a7b120 > > #4 [ffffffc034c439f0] __kvm_nvhe_$d.2314 at 9ccec46003a80a64 > > #5 [ffffffc034c43ac0] __kvm_nvhe_$d.2314 at 8cf41e6003a945c4 > > #6 [ffffffc034c43b10] __kvm_nvhe_$d.2314 at a8f181e00372c818 > > #7 [ffffffc034c43b40] __kvm_nvhe_$d.2314 at 6dedde600372c0d0 > > #8 [ffffffc034c43b90] __kvm_nvhe_$d.2314 at 62cc07e00373d0ac > > #9 [ffffffc034c43c00] __kvm_nvhe_$d.2314 at 72fb1de00373bedc > > ... > > PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 > > X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 > > X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 > > X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 > > X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 > > X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf > > X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 > > X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 > > X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 > > X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 > > X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 > > ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 > > > By checking the raw data below, will see the lr (fp+8) data show the pointer which already been replaced by PAC prefix. > > > crash> bt -f > > PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase Backgr" > > #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > > ffffffc034c437f0: ffffffc034c43850 6be732e004cf05a4 > > ffffffc034c43800: ffffffe006186108 a0ed07e004cf09c4 > > ffffffc034c43810: ffffff8a1a340000 ffffff8a8d343c00 > > ffffffc034c43820: ffffff89b3eada00 ffffff8b780db540 > > ffffffc034c43830: ffffff89b3eada00 0000000000000000 > > ffffffc034c43840: 0000000000000004 712b828118484a00 > > #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 > > ffffffc034c43850: ffffffc034c438b0 86c54c6004ceff84 > > ffffffc034c43860: 000000708070f000 ffffffc034c43938 > > ffffffc034c43870: ffffff88bd822878 ffffff89b3eada00 > > ... > > > So we check the CONFIG_ARM64_PTR_AUTH and CONFIG_ARM64_PTR_AUTH_KERNEL to double check if pac mechanism been enabled on this ramdump. > > Then we use vabits to figure it out. > > Fix then show the right backtrace below: > > crash> bt 16930 > > PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase Backgr" > > #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > > #1 [ffffffc034c43850] __schedule at ffffffe004cf05a0 > > #2 [ffffffc034c438b0] preempt_schedule_common at ffffffe004ceff80 > > #3 [ffffffc034c43950] unmap_page_range at ffffffe003a7b120 > > #4 [ffffffc034c439f0] unmap_vmas at ffffffe003a80a64 > > #5 [ffffffc034c43ac0] exit_mmap at ffffffe003a945c4 > > #6 [ffffffc034c43b10] __mmput at ffffffe00372c818 > > #7 [ffffffc034c43b40] mmput at ffffffe00372c0d0 > > #8 [ffffffc034c43b90] exit_mm at ffffffe00373d0ac > > #9 [ffffffc034c43c00] do_exit at ffffffe00373bedc > > PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 > > X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 > > X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 > > X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 > > X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 > > X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf > > X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 > > X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 > > X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 > > X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 > > X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 > > ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 > > > Let's use GENMASK to replace the pac pointer to fix it. > > gki related commit as below: > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Author: Amit Daniel Kachhap <amit.kachhap(a)arm.com&gt; > > Date: Fri Mar 13 14:34:58 2020 +0530 > > > arm64: mask PAC bits of __builtin_return_address > > > Functions like vmap() record how much memory has been allocated by their > > callers, and callers are identified using __builtin_return_address(). Once > > the kernel is using pointer-auth the return address will be signed. This > > means it will not match any kernel symbol, and will vary between threads > > even for the same caller. > > > The output of /proc/vmallocinfo in this case may look like, > > 0x(____ptrval____)-0x(____ptrval____) 20480 0x86e28000100e7c60 pages=4 vmalloc N0=4 > > 0x(____ptrval____)-0x(____ptrval____) 20480 0x86e28000100e7c60 pages=4 vmalloc N0=4 > > 0x(____ptrval____)-0x(____ptrval____) 20480 0xc5c78000100e7c60 pages=4 vmalloc N0=4 > > > The above three 64bit values should be the same symbol name and not > > different LR values. > > > Use the pre-processor to add logic to clear the PAC to > > __builtin_return_address() callers. This patch adds a new file > > asm/compiler.h and is transitively included via include/compiler_types.h on > > the compiler command line so it is guaranteed to be loaded and the users of > > this macro will not find a wrong version. > > > Helper macros ptrauth_kernel_pac_mask/ptrauth_clear_pac are created for > > this purpose and added in this file. Existing macro ptrauth_user_pac_mask > > moved from asm/pointer_auth.h. > > > Signed-off-by: Amit Daniel Kachhap <amit.kachhap(a)arm.com&gt; > > Reviewed-by: James Morse <james.morse(a)arm.com&gt; > > Signed-off-by: Catalin Marinas <catalin.marinas(a)arm.com&gt; > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index 87e2cbb76930..115ceea0293e 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -118,6 +118,7 @@ config ARM64 > > select HAVE_ALIGNED_STRUCT_PAGE if SLUB > > select HAVE_ARCH_AUDITSYSCALL > > select HAVE_ARCH_BITREVERSE > > + select HAVE_ARCH_COMPILER_H > > select HAVE_ARCH_HUGE_VMAP > > select HAVE_ARCH_JUMP_LABEL > > select HAVE_ARCH_JUMP_LABEL_RELATIVE > > diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h > > new file mode 100644 > > index 000000000000..eece20d2c55f > > --- /dev/null > > +++ b/arch/arm64/include/asm/compiler.h > > @@ -0,0 +1,24 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __ASM_COMPILER_H > > +#define __ASM_COMPILER_H > > + > > +#if defined(CONFIG_ARM64_PTR_AUTH) > > + > > +/* > > + * The EL0/EL1 pointer bits used by a pointer authentication code. > > + * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply. > > + */ > > +#define ptrauth_user_pac_mask() GENMASK_ULL(54, vabits_actual) > > +#define ptrauth_kernel_pac_mask() GENMASK_ULL(63, vabits_actual) > > + > > +/* Valid for EL0 TTBR0 and EL1 TTBR1 instruction pointers */ > > +#define ptrauth_clear_pac(ptr) \ > > + ((ptr & BIT_ULL(55)) ? (ptr | ptrauth_kernel_pac_mask()) : \ > > + (ptr & ~ptrauth_user_pac_mask())) > > + > > +#define __builtin_return_address(val) \ > > + (void *)(ptrauth_clear_pac((unsigned long)__builtin_return_address(val))) > > + > > +#endif /* CONFIG_ARM64_PTR_AUTH */ > > + > > +#endif /* __ASM_COMPILER_H */ > > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > > index 833d3f948de0..70c47156e54b 100644 > > --- a/arch/arm64/include/asm/pointer_auth.h > > +++ b/arch/arm64/include/asm/pointer_auth.h > > @@ -68,16 +68,9 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne > > > extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg); > > > -/* > > - * The EL0 pointer bits used by a pointer authentication code. > > - * This is dependent on TBI0 being enabled, or bits 63:56 would also apply. > > - */ > > -#define ptrauth_user_pac_mask() GENMASK(54, vabits_actual) > > - > > -/* Only valid for EL0 TTBR0 instruction pointers */ > > static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr) > > { > > - return ptr & ~ptrauth_user_pac_mask(); > > + return ptrauth_clear_pac(ptr); > > } > > > #define ptrauth_thread_init_user(tsk) > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > Signed-off-by: bevis_chen <bevis_chen(a)asus.com&gt; > > --- > > arm64.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > diff --git a/arm64.c b/arm64.c > > index b3040d7..c83a91a 100644 > > --- a/arm64.c > > +++ b/arm64.c > > @@ -92,6 +92,7 @@ static void arm64_get_crash_notes(void); > > static void arm64_calc_VA_BITS(void); > > static int arm64_is_uvaddr(ulong, struct task_context *); > > static void arm64_calc_KERNELPACMASK(void); > > +static void arm64_recalc_KERNELPACMASK(void); > > static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base); > > > struct kernel_range { > > @@ -581,6 +582,16 @@ arm64_init(int when) > > if (!machdep->hz) > > machdep->hz = 100; > > > + /* > > + * In the case of using ramdump rather than vmcore, > > + * Will fail to parse out KERNELPAC. > > + * So let's try again from kconfig to ensure if PAC enabled. > > + * If yes, then we use vabits to figure it out. > > + */ > > + if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK) > > + arm64_recalc_KERNELPACMASK(); > > + > > + > > arm64_irq_stack_init(); > > arm64_overflow_stack_init(); > > arm64_stackframe_init(); > > @@ -4921,6 +4932,26 @@ static void arm64_calc_KERNELPACMASK(void) > > } > > } > > > + > > +#define GENMASK_UL(h, l) \ > > + (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) > > + Think it again, the IKCONFIG is not available in most distributions, so it may not work well in such cases. Can we use some symbols to determine if the PTR AUTH mechanism is enabled? For example([1] or [2]): #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL struct ptrauth_keys_kernel { struct ptrauth_key apia; }; [1] check if the struct ptrauth_keys_kernel exists, eg: STRUCT_SIZE_INIT(ptrauth_keys_kernel, "ptrauth_keys_kernel"); if (VALID_SIZE(ptrauth_keys_kernel) > 0) {

if (machdep->machspec->VA_BITS_ACTUAL){ machdep->machspec->CONFIG_ARM64_KERNELPACMASK = GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); }

------------------------------------------------------------------------------------------------------------------ static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys) { if (system_supports_address_auth()) get_random_bytes(&keys->apia, sizeof(keys->apia)); } ... [2] check if the kernel symbol exists, eg: if (kernel_symbol_exists("ptrauth_keys_init_kernel")) {

if (machdep->machspec->VA_BITS_ACTUAL){ machdep->machspec->CONFIG_ARM64_KERNELPACMASK = GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); } What do you think? bevis_chen and Tao. Or any thoughts? BTW: In addition, the CONFIG_ARM64_PTR_AUTH_KERNEL relies on the CONFIG_ARM64_PTR_AUTH, no need to check them simultaneously. Thanks Lianbo > +static void arm64_recalc_KERNELPACMASK(void){ > > + if (kt->ikconfig_flags & IKCONFIG_AVAIL){ > > + /* arm64: check pac already enabled yet.*/ > > + if ((get_kernel_config("CONFIG_ARM64_PTR_AUTH", NULL) == IKCONFIG_Y) > > + && (get_kernel_config("CONFIG_ARM64_PTR_AUTH_KERNEL", NULL) == IKCONFIG_Y)){ > > + if (machdep->machspec->VA_BITS_ACTUAL){ > > + machdep->machspec->CONFIG_ARM64_KERNELPACMASK = > > + GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); > > + if (CRASHDEBUG(1)) > > + fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", > > + machdep->machspec->CONFIG_ARM64_KERNELPACMASK); > > + } > > + } > > + } > > +} > > + > > #endif /* ARM64 */ > > > > -- > > 2.27.0 > > > > > > > > At 2024-07-15 11:33:13, "cb126yx" <cb126yx(a)126.com&gt; wrote: > > > Hi Lianbo > > > I have already added the origin commit information to patch log, as V3 below. > > > About the macro __GENMASK and GENMASK_UL. > > Both the two macros achieve the same effect, but __GENMASK uses more addition and subtraction operations and is usually called in an internal kernel core bit operation function,. > > Whereas GENMASK_UL uses shift operations directly, which are more efficient and been exposed to external users as api, so I indeed use the GENMASK_UL on purpose. > > > If you have any other suggestions, please feel free to tell me ! > > > > From 1da252fcba90af00a42328f2a5e2ec5058ff4f7e Mon Sep 17 00:00:00 2001 > > From: bevis_chen <bevis_chen(a)asus.com&gt; > > Date: Wed, 3 Jul 2024 15:05:44 +0800 > > Subject: [PATCH] arm64: Fix bt command show wrong stacktrace on ramdump source > > > If we use crash to parse ramdump(Qcom phone device) rathen than vmcore. > > Start command should be like: crash vmlinux --kaslr=xxx DDRCS0_0.BIN@0x0000000080000000,... --machdep vabits_actual=39 > > Then We will see bt command show misleading backtrace information below: > > > crash> bt 16930 > > PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase Backgr" > > #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > > #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 > > #2 [ffffffc034c438b0] __kvm_nvhe_$d.2314 at 86c54c6004ceff80 > > #3 [ffffffc034c43950] __kvm_nvhe_$d.2314 at 55d6f96003a7b120 > > #4 [ffffffc034c439f0] __kvm_nvhe_$d.2314 at 9ccec46003a80a64 > > #5 [ffffffc034c43ac0] __kvm_nvhe_$d.2314 at 8cf41e6003a945c4 > > #6 [ffffffc034c43b10] __kvm_nvhe_$d.2314 at a8f181e00372c818 > > #7 [ffffffc034c43b40] __kvm_nvhe_$d.2314 at 6dedde600372c0d0 > > #8 [ffffffc034c43b90] __kvm_nvhe_$d.2314 at 62cc07e00373d0ac > > #9 [ffffffc034c43c00] __kvm_nvhe_$d.2314 at 72fb1de00373bedc > > ... > > PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 > > X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 > > X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 > > X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 > > X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 > > X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf > > X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 > > X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 > > X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 > > X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 > > X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 > > ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 > > > By checking the raw data below, will see the lr (fp+8) data show the pointer which already been replaced by PAC prefix. > > > crash> bt -f > > PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase Backgr" > > #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > > ffffffc034c437f0: ffffffc034c43850 6be732e004cf05a4 > > ffffffc034c43800: ffffffe006186108 a0ed07e004cf09c4 > > ffffffc034c43810: ffffff8a1a340000 ffffff8a8d343c00 > > ffffffc034c43820: ffffff89b3eada00 ffffff8b780db540 > > ffffffc034c43830: ffffff89b3eada00 0000000000000000 > > ffffffc034c43840: 0000000000000004 712b828118484a00 > > #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 > > ffffffc034c43850: ffffffc034c438b0 86c54c6004ceff84 > > ffffffc034c43860: 000000708070f000 ffffffc034c43938 > > ffffffc034c43870: ffffff88bd822878 ffffff89b3eada00 > > ... > > > So we check the CONFIG_ARM64_PTR_AUTH and CONFIG_ARM64_PTR_AUTH_KERNEL to double check if pac mechanism been enabled on this ramdump. > > Then we use vabits to figure it out. > > Fix then show the right backtrace below: > > crash> bt 16930 > > PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase Backgr" > > #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > > #1 [ffffffc034c43850] __schedule at ffffffe004cf05a0 > > #2 [ffffffc034c438b0] preempt_schedule_common at ffffffe004ceff80 > > #3 [ffffffc034c43950] unmap_page_range at ffffffe003a7b120 > > #4 [ffffffc034c439f0] unmap_vmas at ffffffe003a80a64 > > #5 [ffffffc034c43ac0] exit_mmap at ffffffe003a945c4 > > #6 [ffffffc034c43b10] __mmput at ffffffe00372c818 > > #7 [ffffffc034c43b40] mmput at ffffffe00372c0d0 > > #8 [ffffffc034c43b90] exit_mm at ffffffe00373d0ac > > #9 [ffffffc034c43c00] do_exit at ffffffe00373bedc > > PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 > > X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 > > X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 > > X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 > > X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 > > X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf > > X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 > > X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 > > X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 > > X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 > > X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 > > ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 > > > Let's use GENMASK to replace the pac pointer to fix it. > > gki related commit as below: > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Author: Amit Daniel Kachhap <amit.kachhap(a)arm.com&gt; > > Date: Fri Mar 13 14:34:58 2020 +0530 > > > arm64: mask PAC bits of __builtin_return_address > > > Functions like vmap() record how much memory has been allocated by their > > callers, and callers are identified using __builtin_return_address(). Once > > the kernel is using pointer-auth the return address will be signed. This > > means it will not match any kernel symbol, and will vary between threads > > even for the same caller. > > > The output of /proc/vmallocinfo in this case may look like, > > 0x(____ptrval____)-0x(____ptrval____) 20480 0x86e28000100e7c60 pages=4 vmalloc N0=4 > > 0x(____ptrval____)-0x(____ptrval____) 20480 0x86e28000100e7c60 pages=4 vmalloc N0=4 > > 0x(____ptrval____)-0x(____ptrval____) 20480 0xc5c78000100e7c60 pages=4 vmalloc N0=4 > > > The above three 64bit values should be the same symbol name and not > > different LR values. > > > Use the pre-processor to add logic to clear the PAC to > > __builtin_return_address() callers. This patch adds a new file > > asm/compiler.h and is transitively included via include/compiler_types.h on > > the compiler command line so it is guaranteed to be loaded and the users of > > this macro will not find a wrong version. > > > Helper macros ptrauth_kernel_pac_mask/ptrauth_clear_pac are created for > > this purpose and added in this file. Existing macro ptrauth_user_pac_mask > > moved from asm/pointer_auth.h. > > > Signed-off-by: Amit Daniel Kachhap <amit.kachhap(a)arm.com&gt; > > Reviewed-by: James Morse <james.morse(a)arm.com&gt; > > Signed-off-by: Catalin Marinas <catalin.marinas(a)arm.com&gt; > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index 87e2cbb76930..115ceea0293e 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -118,6 +118,7 @@ config ARM64 > > select HAVE_ALIGNED_STRUCT_PAGE if SLUB > > select HAVE_ARCH_AUDITSYSCALL > > select HAVE_ARCH_BITREVERSE > > + select HAVE_ARCH_COMPILER_H > > select HAVE_ARCH_HUGE_VMAP > > select HAVE_ARCH_JUMP_LABEL > > select HAVE_ARCH_JUMP_LABEL_RELATIVE > > diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h > > new file mode 100644 > > index 000000000000..eece20d2c55f > > --- /dev/null > > +++ b/arch/arm64/include/asm/compiler.h > > @@ -0,0 +1,24 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __ASM_COMPILER_H > > +#define __ASM_COMPILER_H > > + > > +#if defined(CONFIG_ARM64_PTR_AUTH) > > + > > +/* > > + * The EL0/EL1 pointer bits used by a pointer authentication code. > > + * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply. > > + */ > > +#define ptrauth_user_pac_mask() GENMASK_ULL(54, vabits_actual) > > +#define ptrauth_kernel_pac_mask() GENMASK_ULL(63, vabits_actual) > > + > > +/* Valid for EL0 TTBR0 and EL1 TTBR1 instruction pointers */ > > +#define ptrauth_clear_pac(ptr) \ > > + ((ptr & BIT_ULL(55)) ? (ptr | ptrauth_kernel_pac_mask()) : \ > > + (ptr & ~ptrauth_user_pac_mask())) > > + > > +#define __builtin_return_address(val) \ > > + (void *)(ptrauth_clear_pac((unsigned long)__builtin_return_address(val))) > > + > > +#endif /* CONFIG_ARM64_PTR_AUTH */ > > + > > +#endif /* __ASM_COMPILER_H */ > > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h > > index 833d3f948de0..70c47156e54b 100644 > > --- a/arch/arm64/include/asm/pointer_auth.h > > +++ b/arch/arm64/include/asm/pointer_auth.h > > @@ -68,16 +68,9 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne > > > extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg); > > > -/* > > - * The EL0 pointer bits used by a pointer authentication code. > > - * This is dependent on TBI0 being enabled, or bits 63:56 would also apply. > > - */ > > -#define ptrauth_user_pac_mask() GENMASK(54, vabits_actual) > > - > > -/* Only valid for EL0 TTBR0 instruction pointers */ > > static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr) > > { > > - return ptr & ~ptrauth_user_pac_mask(); > > + return ptrauth_clear_pac(ptr); > > } > > > #define ptrauth_thread_init_user(tsk) > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > Signed-off-by: bevis_chen <bevis_chen(a)asus.com&gt; > > --- > > arm64.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > diff --git a/arm64.c b/arm64.c > > index b3040d7..d8ce98f 100644 > > --- a/arm64.c > > +++ b/arm64.c > > @@ -92,6 +92,7 @@ static void arm64_get_crash_notes(void); > > static void arm64_calc_VA_BITS(void); > > static int arm64_is_uvaddr(ulong, struct task_context *); > > static void arm64_calc_KERNELPACMASK(void); > > +static void arm64_recalc_KERNELPACMASK(void); > > static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base); > > > struct kernel_range { > > @@ -581,6 +582,18 @@ arm64_init(int when) > > if (!machdep->hz) > > machdep->hz = 100; > > > + /* > > + * In the case of using ramdump rather than vmcore, > > + * Will fail to parse out KERNELPAC. > > + * So let's try again from kconfig to ensure if PAC enabled. > > + * If yes, then we use vabits to figure it out. > > + * gki related commit url: > > + * https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@arm.com/ > > + */ > > + if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK) > > + arm64_recalc_KERNELPACMASK(); > > + > > + > > arm64_irq_stack_init(); > > arm64_overflow_stack_init(); > > arm64_stackframe_init(); > > @@ -4921,6 +4934,26 @@ static void arm64_calc_KERNELPACMASK(void) > > } > > } > > > + > > +#define GENMASK_UL(h, l) \ > > + (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) > > + > > +static void arm64_recalc_KERNELPACMASK(void){ > > + if (kt->ikconfig_flags & IKCONFIG_AVAIL){ > > + /* arm64: check pac already enabled yet.*/ > > + if ((get_kernel_config("CONFIG_ARM64_PTR_AUTH", NULL) == IKCONFIG_Y) > > + && (get_kernel_config("CONFIG_ARM64_PTR_AUTH_KERNEL", NULL) == IKCONFIG_Y)){ > > + if (machdep->machspec->VA_BITS_ACTUAL){ > > + machdep->machspec->CONFIG_ARM64_KERNELPACMASK = > > + GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); > > + if (CRASHDEBUG(1)) > > + fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", > > + machdep->machspec->CONFIG_ARM64_KERNELPACMASK); > > + } > > + } > > + } > > +} > > + > > #endif /* ARM64 */ > > > > -- > > 2.27.0 > > > > > > > > > > > > At 2024-07-12 16:08:54, "Lianbo Jiang" <lijiang(a)redhat.com&gt; wrote: > >Hi, bevis_chen > > > >Thank you for the v2. > > > >On 7/11/24 6:42 AM, devel-request(a)lists.crash-utility.osci.io wrote: > >> Date: Wed, 10 Jul 2024 14:19:01 -0000 > >> From:cb126yx@126.com > >> Subject: [Crash-utility] Re: arm64: Fix bt command show wrong > >> stacktrace on ramdump source > >> To:devel@lists.crash-utility.osci.io > >> Message-ID:<20240710141901.30295.7874@lists.crash-utility.osci.io> > >> Content-Type: text/plain; charset="utf-8" > >> > >> >From dd6b187ac15a237cefe863c4e5b432cf13b9883a Mon Sep 17 00:00:00 2001 > >> From: bevis_chen&lt;bevis_chen(a)asus.com&gt; > >> Date: Wed, 3 Jul 2024 15:05:44 +0800 > >> Subject: [PATCH] arm64: Fix bt command show wrong stacktrace on ramdump > >> source > >> > >> If we use crash to parse ramdump(Qcom phone device) rathen than vmcore. > >> Start command should be like: crash vmlinux --kaslr=xxx > >> DDRCS0_0.BIN@0x0000000080000000,... --machdep vabits_actual=39 > >> Then We will see bt command show misleading backtrace information below: > >> > >> crash> bt 16930 > >> PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase > >> Backgr" > >> #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > >> #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 > >> #2 [ffffffc034c438b0] __kvm_nvhe_$d.2314 at 86c54c6004ceff80 > >> #3 [ffffffc034c43950] __kvm_nvhe_$d.2314 at 55d6f96003a7b120 > >> #4 [ffffffc034c439f0] __kvm_nvhe_$d.2314 at 9ccec46003a80a64 > >> #5 [ffffffc034c43ac0] __kvm_nvhe_$d.2314 at 8cf41e6003a945c4 > >> #6 [ffffffc034c43b10] __kvm_nvhe_$d.2314 at a8f181e00372c818 > >> #7 [ffffffc034c43b40] __kvm_nvhe_$d.2314 at 6dedde600372c0d0 > >> #8 [ffffffc034c43b90] __kvm_nvhe_$d.2314 at 62cc07e00373d0ac > >> #9 [ffffffc034c43c00] __kvm_nvhe_$d.2314 at 72fb1de00373bedc > >> ... > >> PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 > >> X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 > >> X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 > >> X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 > >> X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 > >> X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf > >> X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 > >> X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 > >> X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 > >> X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 > >> X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 > >> ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 > >> > >> By checking the raw data below, will see the lr (fp+8) data show the > >> pointer which already been replaced by PAC prefix. > >> > >> crash> bt -f > >> PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase > >> Backgr" > >> #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > >> ffffffc034c437f0: ffffffc034c43850 6be732e004cf05a4 > >> ffffffc034c43800: ffffffe006186108 a0ed07e004cf09c4 > >> ffffffc034c43810: ffffff8a1a340000 ffffff8a8d343c00 > >> ffffffc034c43820: ffffff89b3eada00 ffffff8b780db540 > >> ffffffc034c43830: ffffff89b3eada00 0000000000000000 > >> ffffffc034c43840: 0000000000000004 712b828118484a00 > >> #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 > >> ffffffc034c43850: ffffffc034c438b0 86c54c6004ceff84 > >> ffffffc034c43860: 000000708070f000 ffffffc034c43938 > >> ffffffc034c43870: ffffff88bd822878 ffffff89b3eada00 > >> ... > >> > >> So we check the CONFIG_ARM64_PTR_AUTH and CONFIG_ARM64_PTR_AUTH_KERNEL > >> to double check if pac mechanism been enabled on this ramdump. > >> Then we use vabits to figure it out. > >> Fix then show the right backtrace below: > >> crash> bt 16930 > >> PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase > >> Backgr" > >> #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 > >> #1 [ffffffc034c43850] __schedule at ffffffe004cf05a0 > >> #2 [ffffffc034c438b0] preempt_schedule_common at ffffffe004ceff80 > >> #3 [ffffffc034c43950] unmap_page_range at ffffffe003a7b120 > >> #4 [ffffffc034c439f0] unmap_vmas at ffffffe003a80a64 > >> #5 [ffffffc034c43ac0] exit_mmap at ffffffe003a945c4 > >> #6 [ffffffc034c43b10] __mmput at ffffffe00372c818 > >> #7 [ffffffc034c43b40] mmput at ffffffe00372c0d0 > >> #8 [ffffffc034c43b90] exit_mm at ffffffe00373d0ac > >> #9 [ffffffc034c43c00] do_exit at ffffffe00373bedc > >> PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 > >> X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 > >> X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 > >> X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 > >> X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 > >> X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf > >> X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 > >> X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 > >> X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 > >> X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 > >> X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 > >> ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 > >> > >> Let's use GENMASK to replace the pac pointer to fix it. > >> gki related commit url here: > >> https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@arm.com/ > > > >Can you help to add the kernel commit to patch log? > > > >de1702f65feb ("arm64: move PAC masks to <asm/pointer_auth.h>") > > > > > >> Signed-off-by: bevis_chen&lt;bevis_chen(a)asus.com&gt; > >> --- > >> arm64.c | 33 +++++++++++++++++++++++++++++++++ > >> 1 file changed, 33 insertions(+) > >> > >> diff --git a/arm64.c b/arm64.c > >> index b3040d7..d8ce98f 100644 > >> --- a/arm64.c > >> +++ b/arm64.c > >> @@ -92,6 +92,7 @@ static void arm64_get_crash_notes(void); > >> static void arm64_calc_VA_BITS(void); > >> static int arm64_is_uvaddr(ulong, struct task_context *); > >> static void arm64_calc_KERNELPACMASK(void); > >> +static void arm64_recalc_KERNELPACMASK(void); > >> static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base); > >> > >> struct kernel_range { > >> @@ -581,6 +582,18 @@ arm64_init(int when) > >> if (!machdep->hz) > >> machdep->hz = 100; > >> > >> + /* > >> + * In the case of using ramdump rather than vmcore, > >> + * Will fail to parse out KERNELPAC. > >> + * So let's try again from kconfig to ensure if PAC enabled. > >> + * If yes, then we use vabits to figure it out. > >> + * gki related commit url: > >> + *https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@arm.com/ > >> + */ > >> + if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK) > >> + arm64_recalc_KERNELPACMASK(); > >> + > >> + > >> arm64_irq_stack_init(); > >> arm64_overflow_stack_init(); > >> arm64_stackframe_init(); > >> @@ -4921,6 +4934,26 @@ static void arm64_calc_KERNELPACMASK(void) > >> } > >> } > >> > >> + > >> +#define GENMASK_UL(h, l) \ > >> + (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) > >> + > > > >BTW: I saw a similar version implemented in the kernel, can you help > >double check if that is intentional? > > > >#define __GENMASK(h, l) \ > > (((~_UL(0)) - (_UL(1) << (l)) + 1) & \ > > (~_UL(0) >> (__BITS_PER_LONG - 1 - (h)))) > > > > > >Thanks > > > >Lianbo > > > >> +static void arm64_recalc_KERNELPACMASK(void){ > >> + if (kt->ikconfig_flags & IKCONFIG_AVAIL){ > >> + /* arm64: check pac already enabled yet.*/ > >> + if ((get_kernel_config("CONFIG_ARM64_PTR_AUTH", NULL) == IKCONFIG_Y) > >> + && (get_kernel_config("CONFIG_ARM64_PTR_AUTH_KERNEL", NULL) == IKCONFIG_Y)){ > >> + if (machdep->machspec->VA_BITS_ACTUAL){ > >> + machdep->machspec->CONFIG_ARM64_KERNELPACMASK = > >> + GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); > >> + if (CRASHDEBUG(1)) > >> + fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", > >> + machdep->machspec->CONFIG_ARM64_KERNELPACMASK); > >> + } > >> + } > >> + } > >> +} > >> + > >> #endif /* ARM64 */ > >> > >> > >> -- > >> 2.27.0 > >-- > >Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io > >To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io > >https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ > >Contribution Guidelines: https://github.com/crash-utility/crash/wiki