Hi lianbo attach patch v2.
lijiang <liji...@redhat.com> 于2025年1月15日周三 16:42写道：
> Thank you for the patch, Guanyou.
>
> On Tue, Dec 31, 2024 at 7:22 PM <devel-requ...@lists.crash-utility.osci.io>
> wrote:
>
>> Date: Thu, 26 Dec 2024 00:08:50 +0800
>> From: Guanyou Chen <chenguanyou9...@gmail.com>
>> Subject: [Crash-utility] [PATCH] arm64: add pac mask to better support
>> gdb stack unwind
>> To: Lianbo <liji...@redhat.com>, Tao Liu <l...@redhat.com>,
>> devel@lists.crash-utility.osci.io
>> Message-ID:
>> <
>> cahs3rmxjg6oxb_zmgnr60kripoxo9tnb_63k+rjjhfk1t7a...@mail.gmail.com>
>> 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 <chenguan...@xiaomi.com>
>> ---
>> 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
>>
>
0001-arm64-add-pac-mask-to-better-support-gdb-stack-unwin.patch.v2
Description: Binary data
-- Crash-utility mailing list -- devel@lists.crash-utility.osci.io To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
