Hi Lijiang, Thanks for the info. On Tue, Jul 16, 2024 at 9:51 PM lijiang <lijiang@xxxxxxxxxx> wrote: > > Thank you for the update. > > On Mon, Jul 15, 2024 at 11:59 AM cb126yx <cb126yx@xxxxxxx> 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@xxxxxxx/ >> >> 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@xxxxxxxx> >> >> 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@xxxxxxx> >> >> 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@xxxxxxx> >> >> Reviewed-by: James Morse <james.morse@xxxxxxx> >> >> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx> >> >> >> 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@xxxxxxxx> >> >> --- >> >> 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) { I think if (VALID_SIZE(ptrauth_keys_kernel)) is enough... > > if (machdep->machspec->VA_BITS_ACTUAL){ > > machdep->machspec->CONFIG_ARM64_KERNELPACMASK = > > GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); > > } I'm OK with this check, it looks reasonable and worth a try... Hi @cb126yx, could you please have a try on this modification, to see if it can work for you? > > ------------------------------------------------------------------------------------------------------------------ > 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")) { I guess this check will not work, since the ptrauth_keys_init_kernel is inlined, there might be no such symbol found in kernel. Please correct me if I'm wrong... Thanks, Tao Liu > 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@xxxxxxx> 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@xxxxxxxx> >> >> 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@xxxxxxx> >> >> 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@xxxxxxx> >> >> Reviewed-by: James Morse <james.morse@xxxxxxx> >> >> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx> >> >> >> 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@xxxxxxxx> >> >> --- >> >> 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@xxxxxxx/ >> >> + */ >> >> + 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@xxxxxxxxxx> wrote: >> >Hi, bevis_chen >> > >> >Thank you for the v2. >> > >> >On 7/11/24 6:42 AM, devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx wrote: >> >> Date: Wed, 10 Jul 2024 14:19:01 -0000 >> >> From:cb126yx@xxxxxxx >> >> Subject: Re: arm64: Fix bt command show wrong >> >> stacktrace on ramdump source >> >> To:devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx >> >> Message-ID:<20240710141901.30295.7874@xxxxxxxxxxxxxxxxxxxxxxxxxxx> >> >> Content-Type: text/plain; charset="utf-8" >> >> >> >> >From dd6b187ac15a237cefe863c4e5b432cf13b9883a Mon Sep 17 00:00:00 2001 >> >> From: bevis_chen<bevis_chen@xxxxxxxx> >> >> 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@xxxxxxx/ >> > >> >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<bevis_chen@xxxxxxxx> >> >> --- >> >> 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@xxxxxxx/ >> >> + */ >> >> + 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx >> >To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx >> >https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ >> >Contribution Guidelines: https://github.com/crash-utility/crash/wiki -- Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
