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

On Wed, Jan 15, 2025 at 9:29 PM Guanyou Chen <chenguanyou9338(a)gmail.com&gt; wrote: > Hi lianbo > > attached pacth v3. > Thank you for the update. Have you tested patch v3? I got an error and failed to apply the gdb patch: ... patching file gdb-10.2/gdb/stack.c Hunk #1 succeeded at 2130 (offset 3 lines). patching file gdb-10.2/gdb/frame.c Hunk #1 FAILED at 944. 1 out of 2 hunks FAILED make[6]: Nothing to be done for 'all'. ... Thanks Lianbo > > Can you help double check if a sanity check needs to be added? I saw > the 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. > > /* 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; >>>> >>> >>> Can you help double check if a sanity check needs to be added? I saw >>> the 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 >>>> >>>