[Crash-utility] [PATCH 2/2] arm64: make crash CONFIG_KASAN_HW_TAGS aware
piliu
piliu at redhat.com
Thu Apr 29 10:05:24 UTC 2021
Hi Vinayak,
After paying some time on MTE and PAC material, I still have concerns on
your 2/2.
Please see the comment inline.
On 4/27/21 9:47 PM, vinayak menon wrote:
> Hi Pingfan,
>
> On Mon, Apr 26, 2021 at 8:06 PM piliu <piliu at redhat.com> wrote:
>>
>>
>>
>> On 4/26/21 7:17 PM, piliu wrote:
>>>
>>> Hi Vinayak,
>>>
>>> On 3/30/21 9:52 PM, Vinayak Menon wrote:
>>>> With CONFIG_KASAN_HW_TAGS enabled kvaddr can be tagged
>>>> and this results in readmem, vtop etc. fail like below.
>> Sorry for the limited knowledge of this field. I have more question
>> during the reviewing.
>>
>> -1.is this reproducible with SW TAGS?
>> -2. HW TAGS aims for malloc, while here I see the handling LR, PC.
>> It seems that they are more related to "Pointer Authentication"
>
> I have not tried with SW tags. CONFIG_KASAN_HW_TAGS can be found here
> in kernel https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/Kconfig.kasan?h=v5.12#n102
Yeah, I know that. Since our lab has no MTE machine to verify your
patch, so I wonder whether soft tagged kasan can work.
>
> The MTE based kasan is used for tagging malloc too. With
> CONFIG_KASAN_HW_TAGS it is used to tag kernel memory, replacing the
> regular kasan.
>
According to
https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Arm_Memory_Tagging_Extension_Whitepaper.pdf,
MTE is used to resolve out-of-range and use-after-free. I can not see
any relation with PC and LR register, which occurs in this patch.
>>
>> Thanks,
>> Pingfan
>>>>
>>>> "
>>>> please wait... (gathering kmem slab cache data)
>>>> crash: invalid kernel virtual address: f0ffff878000201c type:
This bug should be caused by MTE, i.e. a tagged pointer.
>>>> "kmem_cache objsize/object_size"
>>>> crash: get_active_set: no tasks found?
>>>> please wait... (gathering task table data)
>>>> crash: invalid kernel virtual address: f1ffff87f51e3530 type:
>>>> "xa_node shift"
>>>> "
>>>>
>>>> Make the mask introduced for pointer authentication generic
>>>> and use it in vtop and kvaddr validation.
>>>>
>>>> Signed-off-by: Vinayak Menon <vinayakm.list at gmail.com>
>>>> ---
>>>> arm64.c | 50 +++++++++++++++++++++++++++++++-------------------
>>>> defs.h | 2 +-
>>>> 2 files changed, 32 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/arm64.c b/arm64.c
>>>> index 5b59972..bb41cbb 100644
>>>> --- a/arm64.c
>>>> +++ b/arm64.c
>>>> @@ -85,7 +85,8 @@ 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);
>>>> +static int arm64_is_kvaddr(ulong);
>>>> +static void arm64_calc_KERNELTAGMASK(void);
>>>> /*
>>>> @@ -215,7 +216,7 @@ arm64_init(int when)
>>>> machdep->pagemask = ~((ulonglong)machdep->pageoffset);
>>>> arm64_calc_VA_BITS();
>>>> - arm64_calc_KERNELPACMASK();
>>>> + arm64_calc_KERNELTAGMASK();
>>>> ms = machdep->machspec;
>>>> if (ms->VA_BITS_ACTUAL) {
>>>> ms->page_offset = ARM64_PAGE_OFFSET_ACTUAL;
>>>> @@ -228,7 +229,7 @@ arm64_init(int when)
>>>> machdep->kvbase = ARM64_VA_START;
>>>> ms->userspace_top = ARM64_USERSPACE_TOP;
>>>> }
>>>> - machdep->is_kvaddr = generic_is_kvaddr;
>>>> + machdep->is_kvaddr = arm64_is_kvaddr;
>>>> machdep->kvtop = arm64_kvtop;
>>>> if (machdep->flags & NEW_VMEMMAP) {
>>>> struct syment *sp;
>>>> @@ -477,7 +478,7 @@ arm64_init(int when)
>>>> case LOG_ONLY:
>>>> machdep->machspec = &arm64_machine_specific;
>>>> arm64_calc_VA_BITS();
>>>> - arm64_calc_KERNELPACMASK();
>>>> + arm64_calc_KERNELTAGMASK();
>>>> arm64_calc_phys_offset();
>>>> machdep->machspec->page_offset = ARM64_PAGE_OFFSET;
>>>> arm64_calc_physvirt_offset();
>>>> @@ -608,7 +609,7 @@ arm64_dump_machdep_table(ulong arg)
>>>> fprintf(fp, " dis_filter: arm64_dis_filter()\n");
>>>> fprintf(fp, " cmd_mach: arm64_cmd_mach()\n");
>>>> fprintf(fp, " get_smp_cpus: arm64_get_smp_cpus()\n");
>>>> - fprintf(fp, " is_kvaddr: generic_is_kvaddr()\n");
>>>> + fprintf(fp, " is_kvaddr: arm64_is_kvaddr()\n");
>>>> fprintf(fp, " is_uvaddr: arm64_is_uvaddr()\n");
>>>> fprintf(fp, " value_to_symbol:
>>>> generic_machdep_value_to_symbol()\n");
>>>> fprintf(fp, " init_kernel_pgd: arm64_init_kernel_pgd\n");
>>>> @@ -668,9 +669,9 @@ arm64_dump_machdep_table(ulong arg)
>>>> fprintf(fp, "%ld\n", ms->VA_BITS_ACTUAL);
>>>> else
>>>> fprintf(fp, "(unused)\n");
>>>> - fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: ");
>>>> - if (ms->CONFIG_ARM64_KERNELPACMASK)
>>>> - fprintf(fp, "%lx\n", ms->CONFIG_ARM64_KERNELPACMASK);
>>>> + fprintf(fp, "CONFIG_ARM64_KERNELTAGMASK: ");
>>>> + if (ms->CONFIG_ARM64_KERNELTAGMASK)
>>>> + fprintf(fp, "%lx\n", ms->CONFIG_ARM64_KERNELTAGMASK);
>>>> else
>>>> fprintf(fp, "(unused)\n");
>>>> fprintf(fp, " userspace_top: %016lx\n", ms->userspace_top);
>>>> @@ -1208,6 +1209,9 @@ arm64_kvtop(struct task_context *tc, ulong
>>>> kvaddr, physaddr_t *paddr, int verbos
>>>> if (!IS_KVADDR(kvaddr))
>>>> return FALSE;
>>>> + if (kvaddr & (1UL << 63))
>>>> + kvaddr |= machdep->machspec->CONFIG_ARM64_KERNELTAGMASK;
>>>> +
>>>> if (!vt->vmalloc_start) {
>>>> *paddr = VTOP(kvaddr);
>>>> return TRUE;
>>>> @@ -1828,7 +1832,7 @@ arm64_is_kernel_exception_frame(struct bt_info
>>>> *bt, ulong stkptr)
>>>> if (INSTACK(regs->sp, bt) && INSTACK(regs->regs[29], bt) &&
>>>> !(regs->pstate & (0xffffffff00000000ULL | PSR_MODE32_BIT)) &&
>>>> is_kernel_text(regs->pc) &&
>>>> - is_kernel_text(regs->regs[30] |
>>>> ms->CONFIG_ARM64_KERNELPACMASK)) {
>>>> + is_kernel_text(regs->regs[30] |
>>>> ms->CONFIG_ARM64_KERNELTAGMASK)) {
>>>> switch (regs->pstate & PSR_MODE_MASK)
>>>> {
>>>> case PSR_MODE_EL1t:
>>>> @@ -2198,8 +2202,8 @@ arm64_unwind_frame(struct bt_info *bt, struct
>>>> arm64_stackframe *frame)
>>>> frame->sp = fp + 0x10;
>>>> frame->fp = GET_STACK_ULONG(fp);
>>>> frame->pc = GET_STACK_ULONG(fp + 8);
>>>> - if (is_kernel_text(frame->pc | ms->CONFIG_ARM64_KERNELPACMASK))
>>>> - frame->pc |= ms->CONFIG_ARM64_KERNELPACMASK;
>>>> + if (is_kernel_text(frame->pc | ms->CONFIG_ARM64_KERNELTAGMASK))
>>>> + frame->pc |= ms->CONFIG_ARM64_KERNELTAGMASK;
>>>> if ((frame->fp == 0) && (frame->pc == 0))
>>>> return FALSE;
>>>> @@ -2869,8 +2873,8 @@ arm64_print_text_symbols(struct bt_info *bt,
>>>> struct arm64_stackframe *frame, FIL
>>>> for (i = (start - bt->stackbase)/sizeof(ulong); i <
>>>> LONGS_PER_STACK; i++) {
>>>> up = (ulong *)(&bt->stackbuf[i*sizeof(ulong)]);
>>>> val = *up;
>>>> - if (is_kernel_text(val | ms->CONFIG_ARM64_KERNELPACMASK)) {
>>>> - val |= ms->CONFIG_ARM64_KERNELPACMASK;
>>>> + if (is_kernel_text(val | ms->CONFIG_ARM64_KERNELTAGMASK)) {
>>>> + val |= ms->CONFIG_ARM64_KERNELTAGMASK;
>>>> name = closest_symbol(val);
>>>> fprintf(ofp, " %s[%s] %s at %lx",
>>>> bt->flags & BT_ERROR_MASK ?
>>>> @@ -3205,8 +3209,8 @@ arm64_print_exception_frame(struct bt_info *bt,
>>>> ulong pt_regs, int mode, FILE *o
>>>> rows = 4;
>>>> } else {
>>>> LR = regs->regs[30];
>>>> - if (is_kernel_text (LR | ms->CONFIG_ARM64_KERNELPACMASK))
>>>> - LR |= ms->CONFIG_ARM64_KERNELPACMASK;
>>>> + if (is_kernel_text (LR | ms->CONFIG_ARM64_KERNELTAGMASK))
>>>> + LR |= ms->CONFIG_ARM64_KERNELTAGMASK;
This looks alike to the scenario on page 17 of
https://events.static.linuxfound.org/sites/events/files/slides/slides_23.pdf
When LR is saved on frame, it should not be MTE tagged, instead, it is
PAed. Because it is not an object pointer, and a tagged pointer is
generated by explicit code, e.g.
__kasan_slab_alloc()
{
tag = assign_tag(cache, object, false);
tagged_object = set_tag(object, tag);
}
I do not see any opportunity for PC/LR to handle by similar code.
So here, I think it should be something like LR |=
ms->CONFIG_ARM64_KERNELTAGMASK.PAC_SUBMASK.
>>>> SP = regs->sp;
>>>> top_reg = 29;
>>>> is_64_bit = TRUE;
>>>> @@ -4102,6 +4106,14 @@ arm64_calc_virtual_memory_ranges(void)
>>>> }
>>>> static int
>>>> +arm64_is_kvaddr(ulong addr)
>>>> +{
>>>> + if (addr & (1UL << 63))
>>>> + addr |= machdep->machspec->CONFIG_ARM64_KERNELTAGMASK;
>>>> + return generic_is_kvaddr(addr);
>>>> +}
>>>> +
>>>> +static int
>>>> arm64_is_uvaddr(ulong addr, struct task_context *tc)
>>>> {
>>>> return (addr < machdep->machspec->userspace_top);
>>>> @@ -4129,21 +4141,21 @@ arm64_swp_offset(ulong pte)
>>>> return pte;
>>>> }
>>>> -static void arm64_calc_KERNELPACMASK(void)
>>>> +static void arm64_calc_KERNELTAGMASK(void)
>>>> {
>>>> ulong value = 0;
>>>> char *string;
>>>> - if ((string = pc->read_vmcoreinfo("NUMBER(KERNELPACMASK)"))) {
>>>> + if ((string = pc->read_vmcoreinfo("NUMBER(KERNELTAGMASK)"))) {
>>>
>>> I git grep 5.12 kernel, but did not find this. Do you plan to do it in
>>> kernel firstly?
>>>
Again, this does occur in v5.12 kernel. But I guess you plan to export
both MTE and PAC mask through one interface.
But I think the usage should be separated. As above mentioned.
Sincerely hope you can address my concerns, due to the unavailable MTE
machine to test and verify my thoughts.
Thanks,
Pingfan
>>>> value = htol(string, QUIET, NULL);
>>>> free(string);
>>>> } else if (machdep->machspec->tag_mask) {
>>>> value = machdep->machspec->tag_mask;
>>>> }
>>>> - machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
>>>> + machdep->machspec->CONFIG_ARM64_KERNELTAGMASK = value;
>>>> if (CRASHDEBUG(1))
>>>> - fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", value);
>>>> + fprintf(fp, "CONFIG_ARM64_KERNELTAGMASK: %lx\n", value);
>>>> }
>>>> #endif /* ARM64 */
>>>> diff --git a/defs.h b/defs.h
>>>> index d406f5f..770c335 100644
>>>> --- a/defs.h
>>>> +++ b/defs.h
>>>> @@ -3329,7 +3329,7 @@ struct machine_specific {
>>>> ulong VA_BITS_ACTUAL;
>>>> ulong CONFIG_ARM64_VA_BITS;
>>>> ulong VA_START;
>>>> - ulong CONFIG_ARM64_KERNELPACMASK;
>>>> + ulong CONFIG_ARM64_KERNELTAGMASK;
>>>> ulong physvirt_offset;
>>>> ulong tag_mask;
>>>> };
>>>>
>>>> --
>>>> Crash-utility mailing list
>>>> Crash-utility at redhat.com
>>>> https://listman.redhat.com/mailman/listinfo/crash-utility
>>>>
>>>
>>> --
>>> Crash-utility mailing list
>>> Crash-utility at redhat.com
>>> https://listman.redhat.com/mailman/listinfo/crash-utility
>>
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility
>
More information about the Crash-utility
mailing list