On Wed, Jan 15, 2025 at 9:29 PM Guanyou Chen <chenguanyou9...@gmail.com>
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 <chenguanyou9...@gmail.com> 于2025年1月15日周三 17:55写道：
>
>> 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
>>>>
>>>
--
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
