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 >

Hi lianbo attached pacth v3.

> 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 >>> >>

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 >>>>> >>>>