With CONFIG_KASAN_HW_TAGS enabled kvaddr can be tagged and this results in readmem, vtop etc. fail like below. " please wait... (gathering kmem slab cache data) crash: invalid kernel virtual address: f0ffff878000201c type: "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(a)gmail.com&gt; --- 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; 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)"))) {

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(a)redhat.com https://listman.redhat.com/mailman/listinfo/crash-utility

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"

Thanks, Pingfan >> >> " >> please wait... (gathering kmem slab cache data) >> crash: invalid kernel virtual address: f0ffff878000201c type: >> "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(a)gmail.com&gt; >> --- >> 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; >> 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? > > 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(a)redhat.com >> https://listman.redhat.com/mailman/listinfo/crash-utility >> > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://listman.redhat.com/mailman/listinfo/crash-utility

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(a)redhat.com&gt; 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/l... 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_Mem..., 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(a)gmail.com&gt; >>>> --- >>>> 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(a)redhat.com >>>> https://listman.redhat.com/mailman/listinfo/crash-utility >>>> >>> >>> -- >>> Crash-utility mailing list >>> Crash-utility(a)redhat.com >>> https://listman.redhat.com/mailman/listinfo/crash-utility >> > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://listman.redhat.com/mailman/listinfo/crash-utility >

On 5/3/21 2:16 PM, vinayak menon wrote: > Hi Pingfan > > On Thu, Apr 29, 2021 at 3:35 PM piliu <piliu(a)redhat.com&gt; wrote: [...] > > Okay I think I got your concern now. Actually when I started > implementing the patchset PAC mask was > already present in crash utility. But the issue with that was it > worked only with vmcoreinfo and not raw ramdumps. > So the first patch created a machdep option tasg_mask. When doing that > I made it a generic tag_mask and not > pac_mask to avoid the need for 2 different machdep options, one later > for MTE, essentially doing similar things. > So that's the reason why LR/PC modifications are seen, which is for > PAC. Do you really think I need to split into > 2 different machdep options or we do it later when we find an absolute > need for it ? AFAICT it does not break anything > and takes care of MTE, PAC, MTE+PAC scenarios. > Yes, please split the patch as each patch has a dedicated purpose. Also please limit terms to PAC instead of MTE, since this patch tries to resolves PAC issue. >>>> 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(a)gmail.com&gt; >>>>>> --- >>>>>> 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. > > Trying to understand this better. Without MTE or PAC or both, it is > just a matter of > using the proper tasg_mask value ? Ideally it should work for any of > these combinations. > Sorry that I can not totally agree with you. The tag_mask subfileds should be applied to different things, E.g. here PC/LR can not accept MTE subfield.

Thanks, Pingfan