[Crash-utility] Re: [PATCH v5 07/24] arm64: Fix bt command show wrong stacktrace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi bevis_chen,

The patch you send is mis-formatted, which we cannot apply by "git
am". Could you please re-send your patch by:

$ git send-email -1 --to="devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx" <commit hash id>

On Wed, Jul 24, 2024 at 2:29 PM cb126yx <cb126yx@xxxxxxx> wrote:
>
> If we use crash tool to parse ramdump(Qcom phone device) rather 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.
>
The above message is informal for a patch. How about:

This patch will check if pac mechanism is enabled by checking whether
struct ptrauth_keys_kernel exists. If it does, then
CONFIG_ARM64_PTR_AUTH_KERNEL must be enabled, see the following kernel
code:

struct thread_struct {
...
  ifdef CONFIG_ARM64_PTR_AUTH
    struct ptrauth_keys_user keys_user;
  #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
   struct ptrauth_keys_kernel keys_kernel;
  #endif
}


>
>   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:
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
I think there is no need to embed the kernel patch into this patch's
commit log, just one url which can direct us to the kernel patch will
be fine.

> Author: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>
>
> Date:   Fri Mar 13 14:34:58 2020 +0530
>
>
>     arm64: mask PAC bits of __builtin_return_address
>
>
>     Functions like vmap() record how much memory has been allocated by their
>
>     callers, and callers are identified using __builtin_return_address(). Once
>
>     the kernel is using pointer-auth the return address will be signed. This
>
>     means it will not match any kernel symbol, and will vary between threads
>
>     even for the same caller.
>
>
>     The output of /proc/vmallocinfo in this case may look like,
>
>     0x(____ptrval____)-0x(____ptrval____)   20480 0x86e28000100e7c60 pages=4 vmalloc N0=4
>
>     0x(____ptrval____)-0x(____ptrval____)   20480 0x86e28000100e7c60 pages=4 vmalloc N0=4
>
>     0x(____ptrval____)-0x(____ptrval____)   20480 0xc5c78000100e7c60 pages=4 vmalloc N0=4
>
>
>     The above three 64bit values should be the same symbol name and not
>
>     different LR values.
>
>
>     Use the pre-processor to add logic to clear the PAC to
>
>     __builtin_return_address() callers. This patch adds a new file
>
>     asm/compiler.h and is transitively included via include/compiler_types.h on
>
>     the compiler command line so it is guaranteed to be loaded and the users of
>
>     this macro will not find a wrong version.
>
>
>     Helper macros ptrauth_kernel_pac_mask/ptrauth_clear_pac are created for
>
>     this purpose and added in this file. Existing macro ptrauth_user_pac_mask
>
>     moved from asm/pointer_auth.h.
>
>
>     Signed-off-by: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>
>
>     Reviewed-by: James Morse <james.morse@xxxxxxx>
>
>     Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
>
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>
> index 87e2cbb76930..115ceea0293e 100644
>
> --- a/arch/arm64/Kconfig
>
> +++ b/arch/arm64/Kconfig
>
> @@ -118,6 +118,7 @@ config ARM64
>
>         select HAVE_ALIGNED_STRUCT_PAGE if SLUB
>
>         select HAVE_ARCH_AUDITSYSCALL
>
>         select HAVE_ARCH_BITREVERSE
>
> +       select HAVE_ARCH_COMPILER_H
>
>         select HAVE_ARCH_HUGE_VMAP
>
>         select HAVE_ARCH_JUMP_LABEL
>
>         select HAVE_ARCH_JUMP_LABEL_RELATIVE
>
> diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h
>
> new file mode 100644
>
> index 000000000000..eece20d2c55f
>
> --- /dev/null
>
> +++ b/arch/arm64/include/asm/compiler.h
>
> @@ -0,0 +1,24 @@
>
> +/* SPDX-License-Identifier: GPL-2.0 */
>
> +#ifndef __ASM_COMPILER_H
>
> +#define __ASM_COMPILER_H
>
> +
>
> +#if defined(CONFIG_ARM64_PTR_AUTH)
>
> +
>
> +/*
>
> + * The EL0/EL1 pointer bits used by a pointer authentication code.
>
> + * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply.
>
> + */
>
> +#define ptrauth_user_pac_mask()                GENMASK_ULL(54, vabits_actual)
>
> +#define ptrauth_kernel_pac_mask()      GENMASK_ULL(63, vabits_actual)
>
> +
>
> +/* Valid for EL0 TTBR0 and EL1 TTBR1 instruction pointers */
>
> +#define ptrauth_clear_pac(ptr)                                         \
>
> +       ((ptr & BIT_ULL(55)) ? (ptr | ptrauth_kernel_pac_mask()) :      \
>
> +                              (ptr & ~ptrauth_user_pac_mask()))
>
> +
>
> +#define __builtin_return_address(val)                                  \
>
> +       (void *)(ptrauth_clear_pac((unsigned long)__builtin_return_address(val)))
>
> +
>
> +#endif /* CONFIG_ARM64_PTR_AUTH */
>
> +
>
> +#endif /* __ASM_COMPILER_H */
>
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
>
> index 833d3f948de0..70c47156e54b 100644
>
> --- a/arch/arm64/include/asm/pointer_auth.h
>
> +++ b/arch/arm64/include/asm/pointer_auth.h
>
> @@ -68,16 +68,9 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne
>
>
>  extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg);
>
>
> -/*
>
> - * The EL0 pointer bits used by a pointer authentication code.
>
> - * This is dependent on TBI0 being enabled, or bits 63:56 would also apply.
>
> - */
>
> -#define ptrauth_user_pac_mask()        GENMASK(54, vabits_actual)
>
> -
>
> -/* Only valid for EL0 TTBR0 instruction pointers */
>
>  static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
>
>  {
>
> -       return ptr & ~ptrauth_user_pac_mask();
>
> +       return ptrauth_clear_pac(ptr);
>
>  }
>
>
>  #define ptrauth_thread_init_user(tsk)
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> Signed-off-by: bevis_chen <bevis_chen@xxxxxxxx>
>
> ---
>
>  arm64.c | 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);
>
> +       }
>
> +}
>
I'm OK with the code change above. For the code, ack.

Thanks,
Tao Liu
> +
>
>  #endif  /* ARM64 */
>
>
>
> --
>
> 2.27.0
>
>
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux