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> wrote:
On Wed, Jul 17, 2024 at 2:40 PM lijiang <lijiang(a)redhat.com>
wrote:
>
> On Tue, Jul 16, 2024 at 7:27 PM cb126yx <cb126yx(a)126.com> 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> wrote:
>> >Hi Lijiang,
>> >
>> >Thanks for the info.
>> >
>> >On Tue, Jul 16, 2024 at 9:51 PM lijiang <lijiang(a)redhat.com> wrote:
>> >>
>> >> Thank you for the update.
>> >>
>> >> On Mon, Jul 15, 2024 at 11:59 AM cb126yx <cb126yx(a)126.com> 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>
>> >>>
>> >>> 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>
>> >>>
>> >>> 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>
>> >>>
>> >>> Reviewed-by: James Morse <james.morse(a)arm.com>
>> >>>
>> >>> Signed-off-by: Catalin Marinas <catalin.marinas(a)arm.com>
>> >>>
>> >>>
>> >>> 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>
>> >>>
>> >>> ---
>> >>>
>> >>> 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>
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>
>> >>>
>> >>> 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>
>> >>>
>> >>> 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>
>> >>>
>> >>> Reviewed-by: James Morse <james.morse(a)arm.com>
>> >>>
>> >>> Signed-off-by: Catalin Marinas <catalin.marinas(a)arm.com>
>> >>>
>> >>>
>> >>> 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>
>> >>>
>> >>> ---
>> >>>
>> >>> 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> 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<bevis_chen(a)asus.com>
>> >>> >> 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<bevis_chen(a)asus.com>
>> >>> >> ---
>> >>> >> 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