[Crash-utility] Re: [PATCH v2] arm64: support ramdump for HW Tag-Based KASAN(MTE) mode

Hi chenqiwu, Thank you for the update. The patch can be applied but there are several comments below, I tweaked the patch on my end. Could you test the attached patch? Lianbo, please review the attached one.

On 2023/12/22 12:30, qiwu.chen(a)transsion.com wrote: > Kernel commit 2e903b914797("kasan, arm64: implement HW_TAGS runtime") > introduce a Hardware Tag-Based KASAN(MTE) mode for ARMv8.5 later CPUs, > which uses the Top Byte Ignore (TBI) feature of arm64 CPUs to store a > pointer tag in the top byte of kernel pointers. > > Currently, crash utility cannot load MTE ramdump due to access the invalid > HW Tag-Based kvaddr. Here's the example error message: > please wait... (gathering kmem slab cache data) > crash: invalid kernel virtual address: f1ffff80c000201c type: "kmem_cache objsize/object_size" > please wait... (gathering task table data) > crash: invalid kernel virtual address: f9ffff8239c2cde0 type: "xa_node shift" > > This patch replace the orignal generic_is_kvaddr() with arm64_is_kvaddr(), > which check the validity for a HW Tag-Based kvaddr. mte_tag_reset() is > used to convert a Tag-Based kvaddr to untaggged kvaddr in arm64_VTOP() > and arm64_IS_VMALLOC_ADDR(). > > --- > changes in v2: > - Introduce a new flag (ARM64_MTE) and initialize it on PRE_SYMTAB stage. > - Create this patch based on latest tag. > > Signed-off-by: chenqiwu <qiwu.chen(a)transsion.com&gt; > --- > arm64.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- > defs.h | 1 + > 2 files changed, 47 insertions(+), 4 deletions(-) > > diff --git a/arm64.c b/arm64.c > index 57965c6..ba1a4f5 100644 > --- a/arm64.c > +++ b/arm64.c > @@ -102,6 +102,41 @@ struct kernel_range { > static struct kernel_range *arm64_get_va_range(struct machine_specific *ms); > static void arm64_get_struct_page_size(struct machine_specific *ms); > > +typedef unsigned char u8; This seems used only one time, can be removed? > +/* mte tag shift bit */ > +#define MTE_TAG_SHIFT 56 > +/* native kernel pointers tag */ > +#define KASAN_TAG_KERNEL 0xFF > +/* minimum value for random tags */ > +#define KASAN_TAG_MIN 0xF0 > +/* right shift the tag to MTE_TAG_SHIFT bit */ > +#define mte_tag_shifted(tag) ((ulong)(tag) << MTE_TAG_SHIFT) > +/* get the top byte value of the original kvaddr */ > +#define mte_tag_get(addr) (u8)((ulong)(addr) >> MTE_TAG_SHIFT) > +/* reset the top byte to get an untaggged kvaddr */ > +#define mte_tag_reset(addr) ((ulong)addr & ~mte_tag_shifted(KASAN_TAG_KERNEL) | mte_tag_shifted(KASAN_TAG_KERNEL)) Some warnings are emitted: arm64.c: In function 'arm64_is_kvaddr': arm64.c:117:48: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses] #define mte_tag_reset(addr) ((ulong)addr & ~mte_tag_shifted(KASAN_TAG_KERNEL) | mte_tag_shifted(KASAN_TAG_KERNEL)) ^ ... I added: -#define mte_tag_reset(addr) ((ulong)addr & ~mte_tag_shifted(KASAN_TAG_KERNEL) | mte_tag_shifted(KASAN_TAG_KERNEL)) +#define mte_tag_reset(addr) (((ulong)addr & ~mte_tag_shifted(KASAN_TAG_KERNEL)) | \ + mte_tag_shifted(KASAN_TAG_KERNEL)) > + > +static inline bool is_mte_kvaddr(ulong addr) > +{ > + /* check for ARM64_MTE enabled */ > + if (machdep->flags & ARM64_MTE) > + return false; if (!(machdep->flags & ARM64_MTE)) is correct? > + > + /* check the validity of HW Tag-Based kvaddr */ > + if (mte_tag_get(addr) >= KASAN_TAG_MIN && mte_tag_get(addr) < KASAN_TAG_KERNEL) > + return true; > + > + return false; > +} > + > +static int arm64_is_kvaddr(ulong addr) > +{ > + if (is_mte_kvaddr(addr)) > + return (mte_tag_reset(addr) >= (ulong)(machdep->kvbase)); > + > + return (addr >= (ulong)(machdep->kvbase)); > +} > + > static void arm64_calc_kernel_start(void) > { > struct machine_specific *ms = machdep->machspec; > @@ -146,7 +181,8 @@ arm64_init(int when) > if (machdep->cmdline_args[0]) > arm64_parse_cmdline_args(); > machdep->flags |= MACHDEP_BT_TEXT; > - > + if (symbol_exists("cpu_enable_mte")) > + machdep->flags |= ARM64_MTE; The PRE_SYMTAB phase has no symbol table yet, so the flag should be always not set. I moved this to PRE_GDB with changing it to kernel_symbol_exists(), I think it can't be a module symbol. > ms = machdep->machspec; > > /* > @@ -262,7 +298,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; > > /* The defaults */ > @@ -1023,7 +1059,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"); > @@ -1633,6 +1669,9 @@ ulong arm64_PTOV(ulong paddr) > ulong > arm64_VTOP(ulong addr) > { > + if (is_mte_kvaddr(addr)) > + addr = mte_tag_reset(addr); > + > if (machdep->flags & NEW_VMEMMAP) { > if (machdep->machspec->VA_START && > (addr >= machdep->machspec->kimage_text) && > @@ -4562,7 +4601,10 @@ int > arm64_IS_VMALLOC_ADDR(ulong vaddr) > { > struct machine_specific *ms = machdep->machspec; > - > + > + if (is_mte_kvaddr(vaddr)) > + vaddr = mte_tag_reset(vaddr); > + > if ((machdep->flags & NEW_VMEMMAP) && > (vaddr >= machdep->machspec->kimage_text) && > (vaddr <= machdep->machspec->kimage_end)) > diff --git a/defs.h b/defs.h > index 20237b7..aa8eba8 100644 > --- a/defs.h > +++ b/defs.h > @@ -3348,6 +3348,7 @@ typedef signed int s32; > #define FLIPPED_VM (0x400) > #define HAS_PHYSVIRT_OFFSET (0x800) > #define OVERFLOW_STACKS (0x1000) > +#define ARM64_MTE (0x2000) ah, I should have said this too, the flag "ARM64_MTE" should be printed by "help -m" if set, in arm64_dump_machdep_table(). I added: + if (machdep->flags & ARM64_MTE) + fprintf(fp, "%sARM64_MTE", others++ ? "|" : ""); Thanks, Kazu > > /* > * Get kimage_voffset from /dev/crash