Re: [PATCH] AArch64 pauth: Indicate addresses in backtrace for kernel

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

Hi,

On 10/26/21 9:22 AM, Kuan-Ying Lee wrote:
> On Mon, 2021-10-25 at 20:07 +0800, Luis Machado wrote:
>> On 10/25/21 8:47 AM, Kuan-Ying Lee via Gdb-patches wrote:
>>> Armv8.3-a Pointer Authentication cause the function return address
>>> to
>>> be changed. GDB need to use address bit[55] to know which mode is
>>> active
>>> and mask/unmask the link register in order to get backtrace.
>>>
>>> If address is in kernel mode, we mask the address. If address is in
>>> user mode,
>>> we need to unmask the address.
>>> ---
>>>    gdb/aarch64-tdep.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>> index 4b5af4616af..d4bb4305cea 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -257,7 +257,10 @@ aarch64_frame_unmask_lr (struct gdbarch_tdep
>>> *tdep,
>>>        {
>>>          int cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep-
>>>> pauth_reg_base);
>>>          CORE_ADDR cmask = frame_unwind_register_unsigned
>>> (this_frame, cmask_num);
>>> -      addr = addr & ~cmask;
>>> +      if (addr & 0x0080000000000000ULL)
>>> +        addr = addr | cmask;
>>> +      else
>>> +        addr = addr & ~cmask;
>>>    
>>>          /* Record in the frame that the link register required
>>> unmasking.  */
>>>          set_frame_previous_pc_masked (this_frame);
>>>
>>
> 
> Hi Luis,
> 
> Thanks for your reply.
> 
> I think I use the misunderstanding words 'mask' and 'unmask'.
> I will use 'unmangle' in the v2 commit message instead.

No problem. That part was OK.

> 
> Backtrace needs to be printed as unmangle address not only in kernel
> mode but also user mode.
> 
> However, when we use arm64, kernel address will start with 0xffff....
> and user mode address will start with 0x0000....
> High bits in kernel address and user mode address are different.
> 
> Thus, I think we should do the following two things.
> 1. If address is in kernel mode. Unmangle the kernel address by 'addr |
> cmask'.
> 2. If address is in user mode. Unmangle the user mode address by 'addr
> & ~cmask'
> 
> We can see this kernel patch [1] use different unmangle method for user
> mode and kernel mode.

Thanks for the explanation. That makes sense now, and the current code 
is only handling usermode addresses.

I was slightly confused with bit 55, but I see it is a VA range select bit.

> 
> After my patch, we can parse kernel backtrace as below. (address starts
> with 0xffff...)
> (gdb)bt
> #0  0xffffffdc252a5c44 in crash_setup_regs (newregs=0xffffffdc252b54a8,
> oldregs=0x0)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/arch/arm64/include/asm/kexec.h:45
> #1  ipanic (this=<optimized out>, event=<optimized out>, ptr=<optimized
> out>)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-
> 5.10/drivers/misc/mediatek/aee/mrdump/mrdump_panic.c:259
> #2  0xffffffdc291acbe8 [PAC] in notifier_call_chain (nl=<optimized
> out>, val=<optimized out>, v=<optimized out>, nr_to_call=<optimized
> out>,
>      nr_calls=0x0) at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/kernel/notifier.c:83
> #3  atomic_notifier_call_chain (nh=<optimized out>, val=0,
> v=0xffffffdc2c3aa1c8 <panic.buf>)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/kernel/notifier.c:217
> #4  0xffffffdc291acbe8 [PAC] in notifier_call_chain (nl=<optimized
> out>, val=<optimized out>, v=<optimized out>, nr_to_call=<optimized
> out>,
>      nr_calls=0x0) at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/kernel/notifier.c:83
> #5  atomic_notifier_call_chain (nh=<optimized out>,
> val=18446743919823523840, v=0x0)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/kernel/notifier.c:217
> #6  0xffffffdc2914eb90 [PAC] in panic (fmt=<optimized out>)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/kernel/panic.c:272
> #7  0xffffffdc29c2eb78 [PAC] in sysrq_handle_crash (key=<optimized
> out>)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/drivers/tty/sysrq.c:158
> #8  0xffffffdc29c2fab8 [PAC] in __handle_sysrq (key=99,
> check_mask=<optimized out>)
>      at /mfs/mtkslt1092/mtk21026/CAS_REAL/alps-mp-s0_mp1
> --2021_09_28_11_00/merged/kernel-5.10/drivers/tty/sysrq.c:602
> 
> This user mode backtrace as below is from [2].
> (gdb) bt
> 0  0x0000000000400490 in puts@plt ()
> 1  0x00000000004005dc in foo ("hello") at cbreak-lib.c:6
> 2  0x0000000000400604 [PAC] in bar () at cbreak-lib.c:12
> 3  0x0000000000400620 [PAC] in main2 () at cbreak.c:17
> 4  0x00000000004005b4 in main () at cbreak-3.c:10
> 
>> Could you please share more information about this problem? Why is
>> it
>> GDB needs to do things differently for a kernel mode and user mode
>> address? What is the test setup?
> 
> Explanation is on the above.
> 
>>
>> If we entered the above conditional block, that means DWARF has told
>> GDB
>> that LR is masked (ra_state_regnum), and so it needs to be unmasked.
> 
> Yes, it needs to be 'unmangled'.
> 
> However, kernel address and user mode address need different unmangle
> methods.
>>
>> Given this is generic AArch64 code, we don't want to risk breaking
>> existing use cases, and I'd like to understand what is not being
>> handled
>> properly.
>>
> I am not sure if changing code like this has risk or not.
> Need some gdb experts for review.

That's what I'm trying to assess with your help. I'll go through the 
patch once again with a better understanding of your use case.

> 
> Any suggestion and reviews are welcome.
> Thanks. :)
>> We have another gdbarch method that handles sign-extending kernel
>> mode
>> addresses (gdbarch_significant_addr_bit), but it is not clear if
>> that
>> could be used here without some examples.
> 
> I think we cannot use significant bit when enable PAC feature.
> If enable PAC feature, we need to use bit[55] to know if address is in
> kernel address. (Can refer to [3])
> 
> [1] arm64: mask PAC bits of __builtin_return_address
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=689eae42afd7a916634146edca38463769969184
> 
> [2] AArch64 pauth: Indicate unmasked addresses in backtrace
> 
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3d31bc39e655ea39721754fa0ea539a8a0c9b26c
> 
> [3]
> https://developer.arm.com/documentation/102433/0100/Return-oriented-programming
> 
> Thanks,
> Kuan-Ying Lee
>>
>> Thanks,
>> Luis
> 
> 
>