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

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

 



Thank you for the update.

On Mon, Jul 15, 2024 at 11:59 AM cb126yx <cb126yx@xxxxxxx> wrote:

Sorry, as the origin patch has aleardy been added to patch log,

so the below comment in arm64.c is Inappropriate and inconsistent:

             * gki related commit url:

             * https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@xxxxxxx/

 so delete these url link  comment in V4 version.


patch v4 here:

===>


From ab61e60987e4a835e41e4c19822174045888b84e Mon Sep 17 00:00:00 2001

From: bevis_chen <bevis_chen@xxxxxxxx>

Date: Wed, 3 Jul 2024 15:05:44 +0800

Subject: [PATCH] arm64: Fix bt command show wrong stacktrace on ramdump source


If we use crash to parse ramdump(Qcom phone device) rathen than vmcore.

Start command should be like: crash vmlinux --kaslr=xxx DDRCS0_0.BIN@0x0000000080000000,... --machdep vabits_actual=39

Then We will see bt command show misleading backtrace information below:


crash> bt 16930

PID: 16930    TASK: ffffff89b3eada00  CPU: 2    COMMAND: "Firebase Backgr"

 #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4

 #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0

   #2 [ffffffc034c438b0] __kvm_nvhe_$d.2314 at 86c54c6004ceff80

 #3 [ffffffc034c43950] __kvm_nvhe_$d.2314 at 55d6f96003a7b120

 #4 [ffffffc034c439f0] __kvm_nvhe_$d.2314 at 9ccec46003a80a64

 #5 [ffffffc034c43ac0] __kvm_nvhe_$d.2314 at 8cf41e6003a945c4

 #6 [ffffffc034c43b10] __kvm_nvhe_$d.2314 at a8f181e00372c818

 #7 [ffffffc034c43b40] __kvm_nvhe_$d.2314 at 6dedde600372c0d0

 #8 [ffffffc034c43b90] __kvm_nvhe_$d.2314 at 62cc07e00373d0ac

 #9 [ffffffc034c43c00] __kvm_nvhe_$d.2314 at 72fb1de00373bedc

...

     PC: 00000073f5294840   LR: 00000070d8f39ba4   SP: 00000070d4afd5d0

    X29: 00000070d4afd600  X28: b4000071efcda7f0  X27: 00000070d4afe000

    X26: 0000000000000000  X25: 00000070d9616000  X24: 0000000000000000

    X23: 0000000000000000  X22: 0000000000000000  X21: 0000000000000000

    X20: b40000728fd27520  X19: b40000728fd27550  X18: 000000702daba000

    X17: 00000073f5294820  X16: 00000070d940f9d8  X15: 00000000000000bf

    X14: 0000000000000000  X13: 00000070d8ad2fac  X12: b40000718fce5040

    X11: 0000000000000000  X10: 0000000000000070   X9: 0000000000000001

     X8: 0000000000000062   X7: 0000000000000020   X6: 0000000000000000

     X5: 0000000000000000   X4: 0000000000000000   X3: 0000000000000000

     X2: 0000000000000002   X1: 0000000000000080   X0: b40000728fd27550

    ORIG_X0: b40000728fd27550  SYSCALLNO: ffffffff  PSTATE: 40001000


By checking the raw data below, will see the lr (fp+8) data show the pointer which already been replaced by PAC prefix.


crash> bt -f

PID: 16930    TASK: ffffff89b3eada00  CPU: 2    COMMAND: "Firebase Backgr"

 #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4

    ffffffc034c437f0: ffffffc034c43850 6be732e004cf05a4

    ffffffc034c43800: ffffffe006186108 a0ed07e004cf09c4

    ffffffc034c43810: ffffff8a1a340000 ffffff8a8d343c00

    ffffffc034c43820: ffffff89b3eada00 ffffff8b780db540

    ffffffc034c43830: ffffff89b3eada00 0000000000000000

    ffffffc034c43840: 0000000000000004 712b828118484a00

 #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0

    ffffffc034c43850: ffffffc034c438b0 86c54c6004ceff84

    ffffffc034c43860: 000000708070f000 ffffffc034c43938

    ffffffc034c43870: ffffff88bd822878 ffffff89b3eada00

...


So we check the CONFIG_ARM64_PTR_AUTH and CONFIG_ARM64_PTR_AUTH_KERNEL to double check if pac mechanism been enabled on this ramdump.

Then we use vabits to figure it out.

Fix then show the right backtrace below:

crash> bt 16930

PID: 16930    TASK: ffffff89b3eada00  CPU: 2    COMMAND: "Firebase Backgr"

 #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4

 #1 [ffffffc034c43850] __schedule at ffffffe004cf05a0

 #2 [ffffffc034c438b0] preempt_schedule_common at ffffffe004ceff80

 #3 [ffffffc034c43950] unmap_page_range at ffffffe003a7b120

 #4 [ffffffc034c439f0] unmap_vmas at ffffffe003a80a64

 #5 [ffffffc034c43ac0] exit_mmap at ffffffe003a945c4

 #6 [ffffffc034c43b10] __mmput at ffffffe00372c818

 #7 [ffffffc034c43b40] mmput at ffffffe00372c0d0

 #8 [ffffffc034c43b90] exit_mm at ffffffe00373d0ac

 #9 [ffffffc034c43c00] do_exit at ffffffe00373bedc

     PC: 00000073f5294840   LR: 00000070d8f39ba4   SP: 00000070d4afd5d0

    X29: 00000070d4afd600  X28: b4000071efcda7f0  X27: 00000070d4afe000

    X26: 0000000000000000  X25: 00000070d9616000  X24: 0000000000000000

    X23: 0000000000000000  X22: 0000000000000000  X21: 0000000000000000

    X20: b40000728fd27520  X19: b40000728fd27550  X18: 000000702daba000

    X17: 00000073f5294820  X16: 00000070d940f9d8  X15: 00000000000000bf

    X14: 0000000000000000  X13: 00000070d8ad2fac  X12: b40000718fce5040

    X11: 0000000000000000  X10: 0000000000000070   X9: 0000000000000001

     X8: 0000000000000062   X7: 0000000000000020   X6: 0000000000000000

     X5: 0000000000000000   X4: 0000000000000000   X3: 0000000000000000

     X2: 0000000000000002   X1: 0000000000000080   X0: b40000728fd27550

    ORIG_X0: b40000728fd27550  SYSCALLNO: ffffffff  PSTATE: 40001000


Let's use GENMASK to replace the pac pointer to fix it.

gki related commit as below:


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Author: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>

Date:   Fri Mar 13 14:34:58 2020 +0530


    arm64: mask PAC bits of __builtin_return_address


    Functions like vmap() record how much memory has been allocated by their

    callers, and callers are identified using __builtin_return_address(). Once

    the kernel is using pointer-auth the return address will be signed. This

    means it will not match any kernel symbol, and will vary between threads

    even for the same caller.


    The output of /proc/vmallocinfo in this case may look like,

    0x(____ptrval____)-0x(____ptrval____)   20480 0x86e28000100e7c60 pages=4 vmalloc N0=4

    0x(____ptrval____)-0x(____ptrval____)   20480 0x86e28000100e7c60 pages=4 vmalloc N0=4

    0x(____ptrval____)-0x(____ptrval____)   20480 0xc5c78000100e7c60 pages=4 vmalloc N0=4


    The above three 64bit values should be the same symbol name and not

    different LR values.


    Use the pre-processor to add logic to clear the PAC to

    __builtin_return_address() callers. This patch adds a new file

    asm/compiler.h and is transitively included via include/compiler_types.h on

    the compiler command line so it is guaranteed to be loaded and the users of

    this macro will not find a wrong version.


    Helper macros ptrauth_kernel_pac_mask/ptrauth_clear_pac are created for

    this purpose and added in this file. Existing macro ptrauth_user_pac_mask

    moved from asm/pointer_auth.h.


    Signed-off-by: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>

    Reviewed-by: James Morse <james.morse@xxxxxxx>

    Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>


diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig

index 87e2cbb76930..115ceea0293e 100644

--- a/arch/arm64/Kconfig

+++ b/arch/arm64/Kconfig

@@ -118,6 +118,7 @@ config ARM64

        select HAVE_ALIGNED_STRUCT_PAGE if SLUB

        select HAVE_ARCH_AUDITSYSCALL

        select HAVE_ARCH_BITREVERSE

+       select HAVE_ARCH_COMPILER_H

        select HAVE_ARCH_HUGE_VMAP

        select HAVE_ARCH_JUMP_LABEL

        select HAVE_ARCH_JUMP_LABEL_RELATIVE

diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h

new file mode 100644

index 000000000000..eece20d2c55f

--- /dev/null

+++ b/arch/arm64/include/asm/compiler.h

@@ -0,0 +1,24 @@

+/* SPDX-License-Identifier: GPL-2.0 */

+#ifndef __ASM_COMPILER_H

+#define __ASM_COMPILER_H

+

+#if defined(CONFIG_ARM64_PTR_AUTH)

+

+/*

+ * The EL0/EL1 pointer bits used by a pointer authentication code.

+ * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply.

+ */

+#define ptrauth_user_pac_mask()                GENMASK_ULL(54, vabits_actual)

+#define ptrauth_kernel_pac_mask()      GENMASK_ULL(63, vabits_actual)

+

+/* Valid for EL0 TTBR0 and EL1 TTBR1 instruction pointers */

+#define ptrauth_clear_pac(ptr)                                         \

+       ((ptr & BIT_ULL(55)) ? (ptr | ptrauth_kernel_pac_mask()) :      \

+                              (ptr & ~ptrauth_user_pac_mask()))

+

+#define __builtin_return_address(val)                                  \

+       (void *)(ptrauth_clear_pac((unsigned long)__builtin_return_address(val)))

+

+#endif /* CONFIG_ARM64_PTR_AUTH */

+

+#endif /* __ASM_COMPILER_H */

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h

index 833d3f948de0..70c47156e54b 100644

--- a/arch/arm64/include/asm/pointer_auth.h

+++ b/arch/arm64/include/asm/pointer_auth.h

@@ -68,16 +68,9 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne


 extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg);


-/*

- * The EL0 pointer bits used by a pointer authentication code.

- * This is dependent on TBI0 being enabled, or bits 63:56 would also apply.

- */

-#define ptrauth_user_pac_mask()        GENMASK(54, vabits_actual)

-

-/* Only valid for EL0 TTBR0 instruction pointers */

 static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)

 {

-       return ptr & ~ptrauth_user_pac_mask();

+       return ptrauth_clear_pac(ptr);

 }


 #define ptrauth_thread_init_user(tsk)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Signed-off-by: bevis_chen <bevis_chen@xxxxxxxx>

---

 arm64.c | 31 +++++++++++++++++++++++++++++++

 1 file changed, 31 insertions(+)


diff --git a/arm64.c b/arm64.c

index b3040d7..c83a91a 100644

--- a/arm64.c

+++ b/arm64.c

@@ -92,6 +92,7 @@ static void arm64_get_crash_notes(void);

 static void arm64_calc_VA_BITS(void);

 static int arm64_is_uvaddr(ulong, struct task_context *);

 static void arm64_calc_KERNELPACMASK(void);

+static void arm64_recalc_KERNELPACMASK(void);

 static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base);


 struct kernel_range {

@@ -581,6 +582,16 @@ arm64_init(int when)

                if (!machdep->hz)

                        machdep->hz = 100;


+               /*

+               * In the case of using ramdump rather than vmcore,

+               * Will fail to parse out KERNELPAC.

+               * So let's try again from kconfig to ensure if PAC enabled.

+               * If yes, then we use vabits to figure it out.

+               */

+               if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK)

+                       arm64_recalc_KERNELPACMASK();

+

+

                arm64_irq_stack_init();

                arm64_overflow_stack_init();

                arm64_stackframe_init();

@@ -4921,6 +4932,26 @@ static void arm64_calc_KERNELPACMASK(void)

        }

 }


+

+#define GENMASK_UL(h, l) \

+    (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

+



Think it again, the IKCONFIG is not available in most distributions, so it may not work well in such cases.

Can we use some symbols to determine if the PTR AUTH mechanism is enabled? For example([1] or [2]):
#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL

struct ptrauth_keys_kernel {
        struct ptrauth_key apia;
};

[1] check if the struct ptrauth_keys_kernel exists, eg:

STRUCT_SIZE_INIT(ptrauth_keys_kernel, "ptrauth_keys_kernel");

if (VALID_SIZE(ptrauth_keys_kernel) > 0) {

                       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@xxxxxxx> wrote:


Hi Lianbo


I have already added the origin commit information to patch log, as V3 below.


About the macro __GENMASK and GENMASK_UL.

Both the two macros achieve the same effect, but __GENMASK uses more addition and subtraction operations and is usually called in an internal kernel core bit operation function,.

Whereas GENMASK_UL uses shift operations directly, which are more efficient and been exposed to external users as api, so I indeed use the GENMASK_UL on purpose.


If you have any other suggestions, please feel free to tell me ! 



From 1da252fcba90af00a42328f2a5e2ec5058ff4f7e Mon Sep 17 00:00:00 2001

From: bevis_chen <bevis_chen@xxxxxxxx>

Date: Wed, 3 Jul 2024 15:05:44 +0800

Subject: [PATCH] arm64: Fix bt command show wrong stacktrace on ramdump source


If we use crash to parse ramdump(Qcom phone device) rathen than vmcore.

Start command should be like: crash vmlinux --kaslr=xxx DDRCS0_0.BIN@0x0000000080000000,... --machdep vabits_actual=39

Then We will see bt command show misleading backtrace information below:


crash> bt 16930

PID: 16930    TASK: ffffff89b3eada00  CPU: 2    COMMAND: "Firebase Backgr"

 #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4

 #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0

   #2 [ffffffc034c438b0] __kvm_nvhe_$d.2314 at 86c54c6004ceff80

 #3 [ffffffc034c43950] __kvm_nvhe_$d.2314 at 55d6f96003a7b120

 #4 [ffffffc034c439f0] __kvm_nvhe_$d.2314 at 9ccec46003a80a64

 #5 [ffffffc034c43ac0] __kvm_nvhe_$d.2314 at 8cf41e6003a945c4

 #6 [ffffffc034c43b10] __kvm_nvhe_$d.2314 at a8f181e00372c818

 #7 [ffffffc034c43b40] __kvm_nvhe_$d.2314 at 6dedde600372c0d0

 #8 [ffffffc034c43b90] __kvm_nvhe_$d.2314 at 62cc07e00373d0ac

 #9 [ffffffc034c43c00] __kvm_nvhe_$d.2314 at 72fb1de00373bedc

...

     PC: 00000073f5294840   LR: 00000070d8f39ba4   SP: 00000070d4afd5d0

    X29: 00000070d4afd600  X28: b4000071efcda7f0  X27: 00000070d4afe000

    X26: 0000000000000000  X25: 00000070d9616000  X24: 0000000000000000

    X23: 0000000000000000  X22: 0000000000000000  X21: 0000000000000000

    X20: b40000728fd27520  X19: b40000728fd27550  X18: 000000702daba000

    X17: 00000073f5294820  X16: 00000070d940f9d8  X15: 00000000000000bf

    X14: 0000000000000000  X13: 00000070d8ad2fac  X12: b40000718fce5040

    X11: 0000000000000000  X10: 0000000000000070   X9: 0000000000000001

     X8: 0000000000000062   X7: 0000000000000020   X6: 0000000000000000

     X5: 0000000000000000   X4: 0000000000000000   X3: 0000000000000000

     X2: 0000000000000002   X1: 0000000000000080   X0: b40000728fd27550

    ORIG_X0: b40000728fd27550  SYSCALLNO: ffffffff  PSTATE: 40001000


By checking the raw data below, will see the lr (fp+8) data show the pointer which already been replaced by PAC prefix.


crash> bt -f

PID: 16930    TASK: ffffff89b3eada00  CPU: 2    COMMAND: "Firebase Backgr"

 #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4

    ffffffc034c437f0: ffffffc034c43850 6be732e004cf05a4

    ffffffc034c43800: ffffffe006186108 a0ed07e004cf09c4

    ffffffc034c43810: ffffff8a1a340000 ffffff8a8d343c00

    ffffffc034c43820: ffffff89b3eada00 ffffff8b780db540

    ffffffc034c43830: ffffff89b3eada00 0000000000000000

    ffffffc034c43840: 0000000000000004 712b828118484a00

 #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0

    ffffffc034c43850: ffffffc034c438b0 86c54c6004ceff84

    ffffffc034c43860: 000000708070f000 ffffffc034c43938

    ffffffc034c43870: ffffff88bd822878 ffffff89b3eada00

...


So we check the CONFIG_ARM64_PTR_AUTH and CONFIG_ARM64_PTR_AUTH_KERNEL to double check if pac mechanism been enabled on this ramdump.

Then we use vabits to figure it out.

Fix then show the right backtrace below:

crash> bt 16930

PID: 16930    TASK: ffffff89b3eada00  CPU: 2    COMMAND: "Firebase Backgr"

 #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4

 #1 [ffffffc034c43850] __schedule at ffffffe004cf05a0

 #2 [ffffffc034c438b0] preempt_schedule_common at ffffffe004ceff80

 #3 [ffffffc034c43950] unmap_page_range at ffffffe003a7b120

 #4 [ffffffc034c439f0] unmap_vmas at ffffffe003a80a64

 #5 [ffffffc034c43ac0] exit_mmap at ffffffe003a945c4

 #6 [ffffffc034c43b10] __mmput at ffffffe00372c818

 #7 [ffffffc034c43b40] mmput at ffffffe00372c0d0

 #8 [ffffffc034c43b90] exit_mm at ffffffe00373d0ac

 #9 [ffffffc034c43c00] do_exit at ffffffe00373bedc

     PC: 00000073f5294840   LR: 00000070d8f39ba4   SP: 00000070d4afd5d0

    X29: 00000070d4afd600  X28: b4000071efcda7f0  X27: 00000070d4afe000

    X26: 0000000000000000  X25: 00000070d9616000  X24: 0000000000000000

    X23: 0000000000000000  X22: 0000000000000000  X21: 0000000000000000

    X20: b40000728fd27520  X19: b40000728fd27550  X18: 000000702daba000

    X17: 00000073f5294820  X16: 00000070d940f9d8  X15: 00000000000000bf

    X14: 0000000000000000  X13: 00000070d8ad2fac  X12: b40000718fce5040

    X11: 0000000000000000  X10: 0000000000000070   X9: 0000000000000001

     X8: 0000000000000062   X7: 0000000000000020   X6: 0000000000000000

     X5: 0000000000000000   X4: 0000000000000000   X3: 0000000000000000

     X2: 0000000000000002   X1: 0000000000000080   X0: b40000728fd27550

    ORIG_X0: b40000728fd27550  SYSCALLNO: ffffffff  PSTATE: 40001000


Let's use GENMASK to replace the pac pointer to fix it.

gki related commit as below:


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Author: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>

Date:   Fri Mar 13 14:34:58 2020 +0530


    arm64: mask PAC bits of __builtin_return_address


    Functions like vmap() record how much memory has been allocated by their

    callers, and callers are identified using __builtin_return_address(). Once

    the kernel is using pointer-auth the return address will be signed. This

    means it will not match any kernel symbol, and will vary between threads

    even for the same caller.


    The output of /proc/vmallocinfo in this case may look like,

    0x(____ptrval____)-0x(____ptrval____)   20480 0x86e28000100e7c60 pages=4 vmalloc N0=4

    0x(____ptrval____)-0x(____ptrval____)   20480 0x86e28000100e7c60 pages=4 vmalloc N0=4

    0x(____ptrval____)-0x(____ptrval____)   20480 0xc5c78000100e7c60 pages=4 vmalloc N0=4


    The above three 64bit values should be the same symbol name and not

    different LR values.


    Use the pre-processor to add logic to clear the PAC to

    __builtin_return_address() callers. This patch adds a new file

    asm/compiler.h and is transitively included via include/compiler_types.h on

    the compiler command line so it is guaranteed to be loaded and the users of

    this macro will not find a wrong version.


    Helper macros ptrauth_kernel_pac_mask/ptrauth_clear_pac are created for

    this purpose and added in this file. Existing macro ptrauth_user_pac_mask

    moved from asm/pointer_auth.h.


    Signed-off-by: Amit Daniel Kachhap <amit.kachhap@xxxxxxx>

    Reviewed-by: James Morse <james.morse@xxxxxxx>

    Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>


diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig

index 87e2cbb76930..115ceea0293e 100644

--- a/arch/arm64/Kconfig

+++ b/arch/arm64/Kconfig

@@ -118,6 +118,7 @@ config ARM64

        select HAVE_ALIGNED_STRUCT_PAGE if SLUB

        select HAVE_ARCH_AUDITSYSCALL

        select HAVE_ARCH_BITREVERSE

+       select HAVE_ARCH_COMPILER_H

        select HAVE_ARCH_HUGE_VMAP

        select HAVE_ARCH_JUMP_LABEL

        select HAVE_ARCH_JUMP_LABEL_RELATIVE

diff --git a/arch/arm64/include/asm/compiler.h b/arch/arm64/include/asm/compiler.h

new file mode 100644

index 000000000000..eece20d2c55f

--- /dev/null

+++ b/arch/arm64/include/asm/compiler.h

@@ -0,0 +1,24 @@

+/* SPDX-License-Identifier: GPL-2.0 */

+#ifndef __ASM_COMPILER_H

+#define __ASM_COMPILER_H

+

+#if defined(CONFIG_ARM64_PTR_AUTH)

+

+/*

+ * The EL0/EL1 pointer bits used by a pointer authentication code.

+ * This is dependent on TBI0/TBI1 being enabled, or bits 63:56 would also apply.

+ */

+#define ptrauth_user_pac_mask()                GENMASK_ULL(54, vabits_actual)

+#define ptrauth_kernel_pac_mask()      GENMASK_ULL(63, vabits_actual)

+

+/* Valid for EL0 TTBR0 and EL1 TTBR1 instruction pointers */

+#define ptrauth_clear_pac(ptr)                                         \

+       ((ptr & BIT_ULL(55)) ? (ptr | ptrauth_kernel_pac_mask()) :      \

+                              (ptr & ~ptrauth_user_pac_mask()))

+

+#define __builtin_return_address(val)                                  \

+       (void *)(ptrauth_clear_pac((unsigned long)__builtin_return_address(val)))

+

+#endif /* CONFIG_ARM64_PTR_AUTH */

+

+#endif /* __ASM_COMPILER_H */

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h

index 833d3f948de0..70c47156e54b 100644

--- a/arch/arm64/include/asm/pointer_auth.h

+++ b/arch/arm64/include/asm/pointer_auth.h

@@ -68,16 +68,9 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne


 extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg);


-/*

- * The EL0 pointer bits used by a pointer authentication code.

- * This is dependent on TBI0 being enabled, or bits 63:56 would also apply.

- */

-#define ptrauth_user_pac_mask()        GENMASK(54, vabits_actual)

-

-/* Only valid for EL0 TTBR0 instruction pointers */

 static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)

 {

-       return ptr & ~ptrauth_user_pac_mask();

+       return ptrauth_clear_pac(ptr);

 }


 #define ptrauth_thread_init_user(tsk)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Signed-off-by: bevis_chen <bevis_chen@xxxxxxxx>

---

 arm64.c | 33 +++++++++++++++++++++++++++++++++

 1 file changed, 33 insertions(+)


diff --git a/arm64.c b/arm64.c

index b3040d7..d8ce98f 100644

--- a/arm64.c

+++ b/arm64.c

@@ -92,6 +92,7 @@ static void arm64_get_crash_notes(void);

 static void arm64_calc_VA_BITS(void);

 static int arm64_is_uvaddr(ulong, struct task_context *);

 static void arm64_calc_KERNELPACMASK(void);

+static void arm64_recalc_KERNELPACMASK(void);

 static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base);


 struct kernel_range {

@@ -581,6 +582,18 @@ arm64_init(int when)

                if (!machdep->hz)

                        machdep->hz = 100;


+               /*

+               * In the case of using ramdump rather than vmcore,

+               * Will fail to parse out KERNELPAC.

+               * So let's try again from kconfig to ensure if PAC enabled.

+               * If yes, then we use vabits to figure it out.

+               * gki related commit url:

+               * https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@xxxxxxx/

+               */

+               if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK)

+                       arm64_recalc_KERNELPACMASK();

+

+

                arm64_irq_stack_init();

                arm64_overflow_stack_init();

                arm64_stackframe_init();

@@ -4921,6 +4934,26 @@ static void arm64_calc_KERNELPACMASK(void)

        }

 }


+

+#define GENMASK_UL(h, l) \

+    (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

+

+static void arm64_recalc_KERNELPACMASK(void){

+       if (kt->ikconfig_flags & IKCONFIG_AVAIL){

+               /* arm64: check pac already enabled yet.*/

+               if ((get_kernel_config("CONFIG_ARM64_PTR_AUTH", NULL) == IKCONFIG_Y)

+                       && (get_kernel_config("CONFIG_ARM64_PTR_AUTH_KERNEL", NULL) == IKCONFIG_Y)){

+                       if (machdep->machspec->VA_BITS_ACTUAL){

+                               machdep->machspec->CONFIG_ARM64_KERNELPACMASK =

+                                       GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL);

+                               if (CRASHDEBUG(1))

+                                       fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n",

+                                               machdep->machspec->CONFIG_ARM64_KERNELPACMASK);

+                       }

+               }

+       }

+}

+

 #endif  /* ARM64 */



--

2.27.0




 






At 2024-07-12 16:08:54, "Lianbo Jiang" <lijiang@xxxxxxxxxx> wrote: >Hi,  bevis_chen > >Thank you for the v2. > >On 7/11/24 6:42 AM, devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx wrote: >> Date: Wed, 10 Jul 2024 14:19:01 -0000 >> From:cb126yx@xxxxxxx >> Subject: Re: arm64: Fix bt command show wrong >> stacktrace on ramdump source >> To:devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx >> Message-ID:<20240710141901.30295.7874@xxxxxxxxxxxxxxxxxxxxxxxxxxx> >> Content-Type: text/plain; charset="utf-8" >> >> >From dd6b187ac15a237cefe863c4e5b432cf13b9883a Mon Sep 17 00:00:00 2001 >> From: bevis_chen<bevis_chen@xxxxxxxx> >> Date: Wed, 3 Jul 2024 15:05:44 +0800 >> Subject: [PATCH] arm64: Fix bt command show wrong stacktrace on ramdump >> source >> >> If we use crash to parse ramdump(Qcom phone device) rathen than vmcore. >> Start command should be like: crash vmlinux --kaslr=xxx >> DDRCS0_0.BIN@0x0000000080000000,... --machdep vabits_actual=39 >> Then We will see bt command show misleading backtrace information below: >> >> crash> bt 16930 >> PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase >> Backgr" >> #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 >> #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 >> #2 [ffffffc034c438b0] __kvm_nvhe_$d.2314 at 86c54c6004ceff80 >> #3 [ffffffc034c43950] __kvm_nvhe_$d.2314 at 55d6f96003a7b120 >> #4 [ffffffc034c439f0] __kvm_nvhe_$d.2314 at 9ccec46003a80a64 >> #5 [ffffffc034c43ac0] __kvm_nvhe_$d.2314 at 8cf41e6003a945c4 >> #6 [ffffffc034c43b10] __kvm_nvhe_$d.2314 at a8f181e00372c818 >> #7 [ffffffc034c43b40] __kvm_nvhe_$d.2314 at 6dedde600372c0d0 >> #8 [ffffffc034c43b90] __kvm_nvhe_$d.2314 at 62cc07e00373d0ac >> #9 [ffffffc034c43c00] __kvm_nvhe_$d.2314 at 72fb1de00373bedc >> ... >> PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 >> X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 >> X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 >> X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 >> X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 >> X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf >> X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 >> X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 >> X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 >> X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 >> X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 >> ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 >> >> By checking the raw data below, will see the lr (fp+8) data show the >> pointer which already been replaced by PAC prefix. >> >> crash> bt -f >> PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase >> Backgr" >> #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 >> ffffffc034c437f0: ffffffc034c43850 6be732e004cf05a4 >> ffffffc034c43800: ffffffe006186108 a0ed07e004cf09c4 >> ffffffc034c43810: ffffff8a1a340000 ffffff8a8d343c00 >> ffffffc034c43820: ffffff89b3eada00 ffffff8b780db540 >> ffffffc034c43830: ffffff89b3eada00 0000000000000000 >> ffffffc034c43840: 0000000000000004 712b828118484a00 >> #1 [ffffffc034c43850] __kvm_nvhe_$d.2314 at 6be732e004cf05a0 >> ffffffc034c43850: ffffffc034c438b0 86c54c6004ceff84 >> ffffffc034c43860: 000000708070f000 ffffffc034c43938 >> ffffffc034c43870: ffffff88bd822878 ffffff89b3eada00 >> ... >> >> So we check the CONFIG_ARM64_PTR_AUTH and CONFIG_ARM64_PTR_AUTH_KERNEL >> to double check if pac mechanism been enabled on this ramdump. >> Then we use vabits to figure it out. >> Fix then show the right backtrace below: >> crash> bt 16930 >> PID: 16930 TASK: ffffff89b3eada00 CPU: 2 COMMAND: "Firebase >> Backgr" >> #0 [ffffffc034c437f0] __switch_to at ffffffe0036832d4 >> #1 [ffffffc034c43850] __schedule at ffffffe004cf05a0 >> #2 [ffffffc034c438b0] preempt_schedule_common at ffffffe004ceff80 >> #3 [ffffffc034c43950] unmap_page_range at ffffffe003a7b120 >> #4 [ffffffc034c439f0] unmap_vmas at ffffffe003a80a64 >> #5 [ffffffc034c43ac0] exit_mmap at ffffffe003a945c4 >> #6 [ffffffc034c43b10] __mmput at ffffffe00372c818 >> #7 [ffffffc034c43b40] mmput at ffffffe00372c0d0 >> #8 [ffffffc034c43b90] exit_mm at ffffffe00373d0ac >> #9 [ffffffc034c43c00] do_exit at ffffffe00373bedc >> PC: 00000073f5294840 LR: 00000070d8f39ba4 SP: 00000070d4afd5d0 >> X29: 00000070d4afd600 X28: b4000071efcda7f0 X27: 00000070d4afe000 >> X26: 0000000000000000 X25: 00000070d9616000 X24: 0000000000000000 >> X23: 0000000000000000 X22: 0000000000000000 X21: 0000000000000000 >> X20: b40000728fd27520 X19: b40000728fd27550 X18: 000000702daba000 >> X17: 00000073f5294820 X16: 00000070d940f9d8 X15: 00000000000000bf >> X14: 0000000000000000 X13: 00000070d8ad2fac X12: b40000718fce5040 >> X11: 0000000000000000 X10: 0000000000000070 X9: 0000000000000001 >> X8: 0000000000000062 X7: 0000000000000020 X6: 0000000000000000 >> X5: 0000000000000000 X4: 0000000000000000 X3: 0000000000000000 >> X2: 0000000000000002 X1: 0000000000000080 X0: b40000728fd27550 >> ORIG_X0: b40000728fd27550 SYSCALLNO: ffffffff PSTATE: 40001000 >> >> Let's use GENMASK to replace the pac pointer to fix it. >> gki related commit url here: >> https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@xxxxxxx/ > >Can you help to add the kernel commit to patch log? > >de1702f65feb ("arm64: move PAC masks to <asm/pointer_auth.h>") > > >> Signed-off-by: bevis_chen<bevis_chen@xxxxxxxx> >> --- >> arm64.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/arm64.c b/arm64.c >> index b3040d7..d8ce98f 100644 >> --- a/arm64.c >> +++ b/arm64.c >> @@ -92,6 +92,7 @@ static void arm64_get_crash_notes(void); >> static void arm64_calc_VA_BITS(void); >> static int arm64_is_uvaddr(ulong, struct task_context *); >> static void arm64_calc_KERNELPACMASK(void); >> +static void arm64_recalc_KERNELPACMASK(void); >> static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, int base); >> >> struct kernel_range { >> @@ -581,6 +582,18 @@ arm64_init(int when) >> if (!machdep->hz) >> machdep->hz = 100; >> >> + /* >> + * In the case of using ramdump rather than vmcore, >> + * Will fail to parse out KERNELPAC. >> + * So let's try again from kconfig to ensure if PAC enabled. >> + * If yes, then we use vabits to figure it out. >> + * gki related commit url: >> + *https://lore.kernel.org/all/20230412160134.306148-4-mark.rutland@xxxxxxx/ >> + */ >> + if(!machdep->machspec->CONFIG_ARM64_KERNELPACMASK) >> + arm64_recalc_KERNELPACMASK(); >> + >> + >> arm64_irq_stack_init(); >> arm64_overflow_stack_init(); >> arm64_stackframe_init(); >> @@ -4921,6 +4934,26 @@ static void arm64_calc_KERNELPACMASK(void) >> } >> } >> >> + >> +#define GENMASK_UL(h, l) \ >> + (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) >> + > >BTW: I saw a similar version implemented in the kernel, can you help >double check if that is intentional? > >#define __GENMASK(h, l) \ >         (((~_UL(0)) - (_UL(1) << (l)) + 1) & \ >          (~_UL(0) >> (__BITS_PER_LONG - 1 - (h)))) > > >Thanks > >Lianbo > >> +static void arm64_recalc_KERNELPACMASK(void){ >> + if (kt->ikconfig_flags & IKCONFIG_AVAIL){ >> + /* arm64: check pac already enabled yet.*/ >> + if ((get_kernel_config("CONFIG_ARM64_PTR_AUTH", NULL) == IKCONFIG_Y) >> + && (get_kernel_config("CONFIG_ARM64_PTR_AUTH_KERNEL", NULL) == IKCONFIG_Y)){ >> + if (machdep->machspec->VA_BITS_ACTUAL){ >> + machdep->machspec->CONFIG_ARM64_KERNELPACMASK = >> + GENMASK_UL(63, machdep->machspec->VA_BITS_ACTUAL); >> + if (CRASHDEBUG(1)) >> + fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", >> + machdep->machspec->CONFIG_ARM64_KERNELPACMASK); >> + } >> + } >> + } >> +} >> + >> #endif /* ARM64 */ >> >> >> -- >> 2.27.0 >-- >Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx >To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx >https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ >Contribution Guidelines: https://github.com/crash-utility/crash/wiki
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

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

 

Powered by Linux