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

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

Hi lijiang and tao, Thanks, the case where IKCONFIG does not exist was indeed not considered before. In file arch/arm64/include/asm/processor.h We can see the "struct ptrauth_keys_kernel" have already been embeded into arm's thread structure. 136 struct thread_struct { 137 struct cpu_context cpu_context; /* cpu context */ ... 159 #ifdef CONFIG_ARM64_PTR_AUTH 160 struct ptrauth_keys_user keys_user; 161 #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL 162 struct ptrauth_keys_kernel keys_kernel; 163 #endif ... 169 }; So your suggestion [1] is a very good idead to dealing with the case that IKCONFIG is not available.

[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); } ---< BTW, most of the mobile phone already have IKCONFIG setting as using the google's gki kernel. So we may check IKconfig first , if the checking fail then we try to check if the struct ptrauth_keys_kernel exists should be a much compatible methods?

What's more, the CONFIG_ARM64_PTR_AUTH_KERNEL does depend on CONFIG_ARM64_PTR_AUTH, just checking the CONFIG_ARM64_PTR_AUTH_KERNEL should enough. Origin KCONFIG: 1616 config ARM64_PTR_AUTH_KERNEL 1617 bool "Use pointer authentication for kernel" 1618 default y 1619 depends on ARM64_PTR_AUTH I will change the patch according to our discussion and do the local test first. If any other suggestion, please feel free to let me know. At 2024-07-16 19:20:33, "Tao Liu" <ltao(a)redhat.com&gt; wrote: >Hi Lijiang, > >Thanks for the info. > >On Tue, Jul 16, 2024 at 9:51 PM lijiang <lijiang(a)redhat.com&gt; wrote: >> >> 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) { > >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(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

On Wed, Jul 17, 2024 at 2:40 PM lijiang <lijiang(a)redhat.com&gt; wrote: > > On Tue, Jul 16, 2024 at 7:27 PM cb126yx <cb126yx(a)126.com&gt; wrote: >> >> Hi lijiang and tao, >> >> >> Thanks, the case where IKCONFIG does not exist was indeed not considered before. >> >> >> In file arch/arm64/include/asm/processor.h >> >> We can see the "struct ptrauth_keys_kernel" have already been embeded into arm's thread structure. >> >> >> >> 136 struct thread_struct { >> >> 137 struct cpu_context cpu_context; /* cpu context */ >> >> ... >> >> 159 #ifdef CONFIG_ARM64_PTR_AUTH >> >> 160 struct ptrauth_keys_user keys_user; >> >> 161 #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL >> >> 162 struct ptrauth_keys_kernel keys_kernel; >> >> 163 #endif >> >> ... >> >> 169 }; >> >> >> So your suggestion [1] is a very good idead to dealing with the case that IKCONFIG is not available. >> >> > > Seems we all agree that [1] is the best solution. Let's go with this. > >> ---> >> >> [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); >> >> } >> >> ---< >> >> >> BTW, most of the mobile phone already have IKCONFIG setting as using the google's gki kernel. >> >> So we may check IKconfig first , if the checking fail then we try to check if the struct ptrauth_keys_kernel exists should be a much compatible methods? > > > No, checking IKconfig may be redundant according to the current kernel code once we use the solution [1]. > Agreed, there is no need to do the redundant IKconfig check. We do IKconfig checks only when there is no better choice. Thanks, Tao Liu > Thanks > Lianbo > >> >> >> >> What's more, the CONFIG_ARM64_PTR_AUTH_KERNEL does depend on CONFIG_ARM64_PTR_AUTH, just checking the CONFIG_ARM64_PTR_AUTH_KERNEL should enough. >> >> >> Origin KCONFIG: >> >> 1616 config ARM64_PTR_AUTH_KERNEL >> >> 1617 bool "Use pointer authentication for kernel" >> >> 1618 default y >> >> 1619 depends on ARM64_PTR_AUTH >> >> >> I will change the patch according to our discussion and do the local test first. >> >> If any other suggestion, please feel free to let me know. >> >> >> At 2024-07-16 19:20:33, "Tao Liu" <ltao(a)redhat.com&gt; wrote: >> >Hi Lijiang, >> > >> >Thanks for the info. >> > >> >On Tue, Jul 16, 2024 at 9:51 PM lijiang <lijiang(a)redhat.com&gt; wrote: >> >> >> >> 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) { >> > >> >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(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

Hi lijiang and tao， I have already test the new patch, and it works well. The new patch list below(maybe v5) accompany with some small modification on commit log side. Please help to check again and see if any other suggestion here. much thanks. Patch V5: ~~~~~~~~~~~~~~~~~~ From 16b7effaf541e1a8a79e012b5b2f62ed2ddf5031 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 as: 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 may search the CONFIG_ARM64_PTR_AUTH_KERNEL to double check if pac mechanism has been enabled on this ramdump. Luckily, lijiang and tao give some valuable suggestion: As IKCONFIG may not available in most distributions. So we could check if the "struct ptrauth_keys_kernel" has already been embeded into arm's thread structure. struct thread_struct { struct cpu_context cpu_context; /* cpu context */ ... #ifdef CONFIG_ARM64_PTR_AUTH struct ptrauth_keys_user keys_user; #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL struct ptrauth_keys_kernel keys_kernel; #endif ... }; PAC feature should be enabled when "struct ptrauth_keys_kernel" exist, and we can use vabits to figure out pac bitmask. The fix backtrace is shown 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 Here is gki's PAC related original commit for reference: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 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 | 30 ++++++++++++++++++++++++++++++ defs.h | 1 + symbols.c | 1 + 3 files changed, 32 insertions(+) diff --git a/arm64.c b/arm64.c index b3040d7..78c4b2f 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,17 @@ 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 we check if the "sturct ptrauth_keys_kernel" exits + * as a basis for whether PAC feature is enabled or not. + * If yes, then we use vabits to figure out pac bitmask. + */ + if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK) + arm64_recalc_KERNELPACMASK(); + + arm64_irq_stack_init(); arm64_overflow_stack_init(); arm64_stackframe_init(); @@ -4921,6 +4933,24 @@ 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){ + /* arm64: check if pac already enabled yet from related structure.*/ + STRUCT_SIZE_INIT(ptrauth_keys_kernel, "ptrauth_keys_kernel"); + if (VALID_STRUCT(ptrauth_keys_kernel)) { + if (machdep->machspec->VA_BITS_ACTUAL){

+ 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 */ diff --git a/defs.h b/defs.h index 49e6923..030cb90 100644 --- a/defs.h +++ b/defs.h @@ -2419,6 +2419,7 @@ struct size_table { /* stash of commonly-used sizes */ long module_memory; long fred_frame; long vmap_node; + long ptrauth_keys_kernel; }; struct array_table { diff --git a/symbols.c b/symbols.c index 69a1fbb..5d5830c 100644 --- a/symbols.c +++ b/symbols.c @@ -12090,6 +12090,7 @@ dump_offset_table(char *spec, ulong makestruct) fprintf(fp, " blk_mq_tags: %ld\n", SIZE(blk_mq_tags)); fprintf(fp, " maple_tree: %ld\n", SIZE(maple_tree)); fprintf(fp, " maple_node: %ld\n", SIZE(maple_node)); + fprintf(fp, " ptrauth_keys_kernel: %ld\n", SIZE(ptrauth_keys_kernel)); fprintf(fp, " percpu_counter: %ld\n", SIZE(percpu_counter)); -- 2.27.0 At 2024-07-17 11:09:02, "cb126yx" <cb126yx(a)126.com&gt; wrote: Sure, I will make a new patch and do local test first. Thanks for both of your suggestion. At 2024-07-17 10:57:02, "Tao Liu" <ltao(a)redhat.com&gt; wrote: >On Wed, Jul 17, 2024 at 2:40 PM lijiang <lijiang(a)redhat.com&gt; wrote: >> >> On Tue, Jul 16, 2024 at 7:27 PM cb126yx <cb126yx(a)126.com&gt; wrote: >>> >>> Hi lijiang and tao, >>> >>> >>> Thanks, the case where IKCONFIG does not exist was indeed not considered before. >>> >>> >>> In file arch/arm64/include/asm/processor.h >>> >>> We can see the "struct ptrauth_keys_kernel" have already been embeded into arm's thread structure. >>> >>> >>> >>> 136 struct thread_struct { >>> >>> 137 struct cpu_context cpu_context; /* cpu context */ >>> >>> ... >>> >>> 159 #ifdef CONFIG_ARM64_PTR_AUTH >>> >>> 160 struct ptrauth_keys_user keys_user; >>> >>> 161 #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL >>> >>> 162 struct ptrauth_keys_kernel keys_kernel; >>> >>> 163 #endif >>> >>> ... >>> >>> 169 }; >>> >>> >>> So your suggestion [1] is a very good idead to dealing with the case that IKCONFIG is not available. >>> >>> >> >> Seems we all agree that [1] is the best solution. Let's go with this. >> >>> ---> >>> >>> [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); >>> >>> } >>> >>> ---< >>> >>> >>> BTW, most of the mobile phone already have IKCONFIG setting as using the google's gki kernel. >>> >>> So we may check IKconfig first , if the checking fail then we try to check if the struct ptrauth_keys_kernel exists should be a much compatible methods? >> >> >> No, checking IKconfig may be redundant according to the current kernel code once we use the solution [1]. >> >Agreed, there is no need to do the redundant IKconfig check. We do >IKconfig checks only when there is no better choice. > >Thanks, >Tao Liu > >> Thanks >> Lianbo >> >>> >>> >>> >>> What's more, the CONFIG_ARM64_PTR_AUTH_KERNEL does depend on CONFIG_ARM64_PTR_AUTH, just checking the CONFIG_ARM64_PTR_AUTH_KERNEL should enough. >>> >>> >>> Origin KCONFIG: >>> >>> 1616 config ARM64_PTR_AUTH_KERNEL >>> >>> 1617 bool "Use pointer authentication for kernel" >>> >>> 1618 default y >>> >>> 1619 depends on ARM64_PTR_AUTH >>> >>> >>> I will change the patch according to our discussion and do the local test first. >>> >>> If any other suggestion, please feel free to let me know. >>> >>> >>> At 2024-07-16 19:20:33, "Tao Liu" <ltao(a)redhat.com&gt; wrote: >>> >Hi Lijiang, >>> > >>> >Thanks for the info. >>> > >>> >On Tue, Jul 16, 2024 at 9:51 PM lijiang <lijiang(a)redhat.com&gt; wrote: >>> >> >>> >> 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) { >>> > >>> >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(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

Signed-off-by: bevis_chen <bevis_chen(a)asus.com --- arm64.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/arm64.c b/arm64.c index b3040d7..50dd458 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,17 @@ 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 we check if the "sturct ptrauth_keys_kernel" exits + * as a basis for whether PAC feature is enabled or not. + * If yes, then we use vabits to figure out pac bitmask. + */ + if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK) + arm64_recalc_KERNELPACMASK(); + + arm64_irq_stack_init(); arm64_overflow_stack_init(); arm64_stackframe_init(); @@ -4921,6 +4933,21 @@ 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){ + /* arm64: check if pac already enabled yet from related structure.*/ + if (STRUCT_EXISTS("ptrauth_keys_kernel") && 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); + } +} +