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

Hi lianbo only test step. rm -rf gdb-10.2/ make target=arm64 Thanks Guanyou lijiang <lijiang(a)redhat.com&gt; 于2025年1月16日周四 14:58写道： > > > 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 >>>>> >>>>