[Crash-utility] [PATCH] Determine the ARM64 kernel's Pointer Authentication mask value by reading the new KERNELPACMASK vmcoreinfo entry.
Amit Kachhap
amit.kachhap at arm.com
Tue Apr 21 05:19:24 UTC 2020
Hi Dave,
On 4/7/20 9:17 PM, Dave Anderson wrote:
>
>
> Hi Amit,
>
> I have a few suggestions and a question regarding your patch.
>
> First, these two warnings need to be addressed:
>
> $ make warn
> ...
> cc -c -g -DARM64 -DLZO -DSNAPPY -DGDB_7_6 arm64.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
> arm64.c: In function ‘arm64_calc_KERNELPACMASK’:
> arm64.c:4090:2: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘ulong’ [-Wformat=]
> fprintf(fp, " got NUMBER(KERNELPACMASK) =%llx\n", value);
> ^
> arm64.c:4090:9: warning: ‘value’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> fprintf(fp, " got NUMBER(KERNELPACMASK) =%llx\n", value);
> ^
> ...
>
> The message should be moved inside the if statement, and also should be gated
> with CRASHDEBUG(1) to prevent it from being displayed unconditionally:
>
> static void
> arm64_calc_KERNELPACMASK(void)
> {
> ulong value;
> char *string;
>
> if ((string = pc->read_vmcoreinfo("NUMBER(KERNELPACMASK)"))) {
> value = htol(string, QUIET, NULL);
> free(string);
> machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
> if (CRASHDEBUG(1))
> fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", value);
> }
> }
Yes thanks for pointing this out. I just posted v2 version and added
your suggestion.
>
> And the CONFIG_ARM64_KERNELPACMASK value should be displayed in arm64_dump_machdep_table().
Ok.
>
> But given that the patch only modifies text return addresses on the kernel
> stack is here:
>
> @@ -1932,6 +1936,9 @@ arm64_print_stackframe_entry(struct bt_info *bt, int level, struct arm64_stackfr
> * See, for example, "bl schedule" before ret_to_user().
> */
>
> branch_pc = frame->pc - 4;
> + if (ms->CONFIG_ARM64_KERNELPACMASK)
> + branch_pc |= ms->CONFIG_ARM64_KERNELPACMASK;
> +
> name = closest_symbol(branch_pc);
> name_plus_offset = NULL;
>
> I'm wondering how all of the other places that check addresses found
> on the kernel stack will work? For example, all of these places
> check whether an address found on the stack is a kernel text address:
>
> $ grep is_kernel_text arm64.c
> is_kernel_text(regs->pc) &&
> is_kernel_text(regs->regs[30])) {
> if (is_kernel_text(*up)) { <===== from arm64_print_text_symbols()
> if (is_kernel_text(frame->pc) ||
> if (is_kernel_text(frame->pc) &&
> if (is_kernel_text(regs->pc) &&
> if (is_kernel_text(LR) &&
> if (is_kernel_text(regs->pc) && (bt->flags & BT_LINE_NUMBERS)) {
> $
>
> Except for the call arm64_print_text_symbols(), these are checking register
> values found in exception frames. Can you confirm that they will still be
> unmodified kernel text addresses?
>
> The call from arm64_print_text_symbols() is for "bt -[tT]", which just
> scours a kernel stack for text addresses and dumps them. So presumably
> that needs to apply the mask to each stack value as you've done in
> arm64_print_stackframe_entry()? (while still recognizing unmodified text
> addresses found in exception frames)
I added PAC mask changes in most of the is_kernel_text function except
arm64_get_stack_frame as it is the first function so PC should not have
any PAC details. Thanks for your detailed suggestion. It was helpful in
fixing the missing implementation.
Thanks,
Amit Daniel
>
> Thanks,
> Dave
>
>
> ----- Original Message -----
>> This value is used to mask the PAC bits and generate correct backtrace.
>> (amit.kachhap at arm.com)
>> ---
>> The kernel version for the corresponding vmcoreinfo entry is posted here[1].
>>
>> [1]: https://lore.kernel.org/patchwork/patch/1211981/
>>
>> arm64.c | 20 ++++++++++++++++++++
>> defs.h | 1 +
>> 2 files changed, 21 insertions(+)
>>
>> diff --git a/arm64.c b/arm64.c
>> index 09b1b76..55e084f 100644
>> --- a/arm64.c
>> +++ b/arm64.c
>> @@ -84,6 +84,7 @@ static int arm64_get_kvaddr_ranges(struct vaddr_range *);
>> static void arm64_get_crash_notes(void);
>> static void arm64_calc_VA_BITS(void);
>> static int arm64_is_uvaddr(ulong, struct task_context *);
>> +static void arm64_calc_KERNELPACMASK(void);
>>
>>
>> /*
>> @@ -213,6 +214,7 @@ arm64_init(int when)
>> machdep->pagemask = ~((ulonglong)machdep->pageoffset);
>>
>> arm64_calc_VA_BITS();
>> + arm64_calc_KERNELPACMASK();
>> ms = machdep->machspec;
>> if (ms->VA_BITS_ACTUAL) {
>> ms->page_offset = ARM64_PAGE_OFFSET_ACTUAL;
>> @@ -472,6 +474,7 @@ arm64_init(int when)
>> case LOG_ONLY:
>> machdep->machspec = &arm64_machine_specific;
>> arm64_calc_VA_BITS();
>> + arm64_calc_KERNELPACMASK();
>> arm64_calc_phys_offset();
>> machdep->machspec->page_offset = ARM64_PAGE_OFFSET;
>> break;
>> @@ -1925,6 +1928,7 @@ arm64_print_stackframe_entry(struct bt_info *bt, int
>> level, struct arm64_stackfr
>> struct syment *sp;
>> struct load_module *lm;
>> char buf[BUFSIZE];
>> + struct machine_specific *ms = machdep->machspec;
>>
>> /*
>> * if pc comes from a saved lr, it actually points to an instruction
>> @@ -1932,6 +1936,9 @@ arm64_print_stackframe_entry(struct bt_info *bt, int
>> level, struct arm64_stackfr
>> * See, for example, "bl schedule" before ret_to_user().
>> */
>> branch_pc = frame->pc - 4;
>> + if (ms->CONFIG_ARM64_KERNELPACMASK)
>> + branch_pc |= ms->CONFIG_ARM64_KERNELPACMASK;
>> +
>> name = closest_symbol(branch_pc);
>> name_plus_offset = NULL;
>>
>> @@ -4070,6 +4077,19 @@ arm64_swp_offset(ulong pte)
>> return pte;
>> }
>>
>> +static void arm64_calc_KERNELPACMASK(void)
>> +{
>> + ulong value;
>> + char *string;
>> +
>> + if ((string = pc->read_vmcoreinfo("NUMBER(KERNELPACMASK)"))) {
>> + value = htol(string, QUIET, NULL);
>> + free(string);
>> + machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
>> + }
>> + fprintf(fp, " got NUMBER(KERNELPACMASK) =%llx\n", value);
>> +}
>> +
>> #endif /* ARM64 */
>>
>>
>> diff --git a/defs.h b/defs.h
>> index a3f828d..f37a957 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -3269,6 +3269,7 @@ struct machine_specific {
>> ulong machine_kexec_end;
>> ulong VA_BITS_ACTUAL;
>> ulong CONFIG_ARM64_VA_BITS;
>> + ulong CONFIG_ARM64_KERNELPACMASK;
>> ulong VA_START;
>> };
>>
>> --
>> 2.7.4
>>
>>
>
More information about the Crash-utility
mailing list