[Crash-utility] Re: [PATCH] arm64: add pac mask to better support gdb stack unwind

Hi lianbo attached pacth v3. value 'pc|mask' is always checked in crash code as below: crash_get_kernel_pac_mask(), and then deal with this one. /* arm64 kernel lr maybe has patuh */ void crash_decode_ptrauth_pc(ulong *pc); void crash_decode_ptrauth_pc(ulong *pc) { #ifdef ARM64 struct machine_specific *ms = machdep->machspec; if (is_kernel_text(*pc | ms->CONFIG_ARM64_KERNELPACMASK)) *pc |= ms->CONFIG_ARM64_KERNELPACMASK; #endif /* !ARM64 */ } Guanyou Chen <chenguanyou9338(a)gmail.com&gt; 于2025年1月15日周三 17:55写道： > Hi lianbo > > attach patch v2. > > lijiang <lijiang(a)redhat.com&gt; 于2025年1月15日周三 16:42写道： > >> Thank you for the patch, Guanyou. >> >> On Tue, Dec 31, 2024 at 7:22 PM < >> devel-request(a)lists.crash-utility.osci.io&gt; wrote: >> >>> Date: Thu, 26 Dec 2024 00:08:50 +0800 >>> From: Guanyou Chen <chenguanyou9338(a)gmail.com&gt; >>> Subject: [Crash-utility] [PATCH] arm64: add pac mask to better support >>> gdb stack unwind >>> To: Lianbo <lijiang(a)redhat.com&gt;, Tao Liu <ltao(a)redhat.com&gt;, >>> devel(a)lists.crash-utility.osci.io >>> Message-ID: >>> < >>> CAHS3RMXJG6OxB_zmgnr60KriPOxo9tnb_63K+rjJHfk1t7aT0A(a)mail.gmail.com&gt; >>> Content-Type: multipart/mixed; boundary="0000000000009f35d6062a1a727c" >>> >>> --0000000000009f35d6062a1a727c >>> Content-Type: multipart/alternative; >>> boundary="0000000000009f35d5062a1a727a" >>> >>> --0000000000009f35d5062a1a727a >>> Content-Type: text/plain; charset="UTF-8" >>> >>> Hi Lianbo & Tao >>> >>> Currently, gdb passthroughs of 'bt', 'frame', 'up', 'down', >>> 'info, locals' don't work on arm64 machine enabled pauth. >>> This is due to gdb not knowing the lr register real values >>> to unwind the stack frames. >>> >>> ---------------------------- >>> gdb passthrough (eg. "bt") | | >>> crash -------------------------> | | >>> | gdb_interface | >>> | | >>> | | >>> | ---------------------- | >>> get_kernel_pac_mask | | | | >>> crash_target<-------------------------+--| gdb | | >>> --------------------------+->| | | >>> arm64: CONFIG_ARM64_KERNELPACMASK| | | | >>> other: ~0UL | | | | >>> | ---------------------- | >>> ---------------------------- >>> >>> With the patch: >>> crash> gdb bt >>> #0 __switch_to (prev=prev@entry=0xffffff8001af92c0, >>> next=next@entry=0xffffff889da7a580) >>> at /proc/self/cwd/common/arch/arm64/kernel/process.c:569 >>> #1 0xffffffd3602132c0 in context_switch (rq=0xffffff8a7295a080, >>> prev=0xffffff8001af92c0, next=0xffffff889da7a580, rf=<optimized out>) at >>> /proc/self/cwd/common/kernel/sched/core.c:5515 >>> #2 __schedule (sched_mode=<optimized out>, sched_mode@entry >>> =2147859424) >>> at /proc/self/cwd/common/kernel/sched/core.c:6843 >>> #3 0xffffffd3602136d8 in schedule () at >>> /proc/self/cwd/common/kernel/sched/core.c:6917 >>> ... >>> >>> Without the patch: >>> crash> gdb bt >>> #0 __switch_to (prev=0xffffff8001af92c0, next=0xffffff889da7a580) at >>> /proc/self/cwd/common/arch/arm64/kernel/process.c:569 >>> #1 0x9fc5c5d3602132c0 in ?? () >>> Backtrace stopped: previous frame identical to this frame (corrupt >>> stack?) >>> >>> Signed-off-by: Guanyou.Chen <chenguanyou(a)xiaomi.com&gt; >>> --- >>> gdb-10.2.patch | 24 ++++++++++++++++++++++++ >>> gdb_interface.c | 11 +++++++++++ >>> 2 files changed, 35 insertions(+) >>> >>> diff --git a/gdb-10.2.patch b/gdb-10.2.patch >>> index c867660..4c13a6b 100644 >>> --- a/gdb-10.2.patch >>> +++ b/gdb-10.2.patch >>> @@ -16216,3 +16216,27 @@ exit 0 >>> printf_filtered (_("Backtrace stopped: %s\n"), >>> frame_stop_reason_string (trailing)); >>> } >>> +--- gdb-10.2/gdb/frame.c.orig >>> ++++ gdb-10.2/gdb/frame.c >>> +@@ -944,6 +944,10 @@ frame_find_by_id (struct frame_id id) >>> + return NULL; >>> + } >>> + >>> ++#ifdef CRASH_MERGE >>> ++extern "C" unsigned long crash_get_kernel_pac_mask(void); >>> ++#endif >>> ++ >>> + static CORE_ADDR >>> + frame_unwind_pc (frame_info_ptr this_frame) >>> + { >>> +@@ -974,6 +978,10 @@ frame_unwind_pc (struct frame_info_ptr *this_frame) >>> + try >>> + { >>> + pc = gdbarch_unwind_pc (prev_gdbarch, this_frame); >>> ++#ifdef CRASH_MERGE >>> ++ CORE_ADDR mask = crash_get_kernel_pac_mask(); >>> ++ pc |= mask; >>> >> >>> value 'pc|mask' is always checked in crash code as below: >> ... >> LR = regs->regs[30]; >> if (is_kernel_text (LR | ms->CONFIG_ARM64_KERNELPACMASK)) >> LR |= ms->CONFIG_ARM64_KERNELPACMASK; >> ... >> if yes, the value pc can be passed as an argument in >> crash_get_kernel_pac_mask(), and then deal with this one. >> >> >>> ++#endif >>> + pc_p = true; >>> + } >>> + catch (const gdb_exception_error &ex) >>> diff --git a/gdb_interface.c b/gdb_interface.c >>> index 315711e..765dafe 100644 >>> --- a/gdb_interface.c >>> +++ b/gdb_interface.c >>> @@ -1083,3 +1083,14 @@ int crash_get_current_task_reg (int regno, const >>> char *regname, >>> return machdep->get_current_task_reg(regno, regname, regsize, value); >>> } >>> >>> +/* arm64 kernel lr pac mask */ >>> +unsigned long crash_get_kernel_pac_mask(void); >>> +unsigned long crash_get_kernel_pac_mask(void) >>> +{ >>> +#ifdef ARM64 >>> + struct machine_specific *ms = machdep->machspec; >>> + return ms->CONFIG_ARM64_KERNELPACMASK; >>> +#else >>> + return ~0UL; >>> >> >> The "~0UL" is 0xFFFFFFFFFFFFFFFF, is this expected? Or do you want it to >> overflow? >> ++ pc |= mask; >> >> It probably has the same result, but the "return 0UL" should be more >> readable. Please see kernel code: >> vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n", >> >> system_supports_address_auth() ? >> ptrauth_kernel_pac_mask() >> : 0); >> >> >> BTW: I cannot apply your patch with git command, and ran into some >> issues, probably it is a coding issue. >> >> Thanks >> Lianbo >> >> >>> +#endif /* !ARM64 */ >>> +} >>> -- >>> 2.34.1 >>> >>> Thanks, >>> Guanyou >>> >>