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

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 >