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, [email protected] 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 <[email protected]>
> ---
> 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/crashFrom bd3ce999aec28981c269024b6dcfa39d9aa7cfb9 Mon Sep 17 00:00:00 2001
From: "[email protected]" <[email protected]>
Date: Fri, 22 Dec 2023 03:30:33 +0000
Subject: [PATCH] arm64: support HW Tag-Based KASAN (MTE) mode
Kernel commit 2e903b914797 ("kasan, arm64: implement HW_TAGS runtime")
introduced Hardware Tag-Based KASAN (MTE) mode for ARMv8.5 and 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 an
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 replaces the orignal generic_is_kvaddr() with arm64_is_kvaddr(),
which checks 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().
Signed-off-by: chenqiwu <[email protected]>
Signed-off-by: Kazuhito Hagio <[email protected]>
---
arm64.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
defs.h | 1 +
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/arm64.c b/arm64.c
index 57965c6cb3c8..6ab10ca9b5be 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);
+/* 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) (unsigned char)((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))
+
+static inline bool is_mte_kvaddr(ulong addr)
+{
+ /* check for ARM64_MTE enabled */
+ if (!(machdep->flags & ARM64_MTE))
+ return false;
+
+ /* 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;
@@ -182,6 +217,9 @@ arm64_init(int when)
if (kernel_symbol_exists("kimage_voffset"))
machdep->flags |= NEW_VMEMMAP;
+ if (kernel_symbol_exists("cpu_enable_mte"))
+ machdep->flags |= ARM64_MTE;
+
if (!machdep->pagesize && arm64_get_vmcoreinfo(&value,
"PAGESIZE", NUM_DEC))
machdep->pagesize = (unsigned int)value;
@@ -262,7 +300,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 */
@@ -975,6 +1013,8 @@ arm64_dump_machdep_table(ulong arg)
fprintf(fp, "%sFLIPPED_VM", others++ ? "|" : "");
if (machdep->flags & HAS_PHYSVIRT_OFFSET)
fprintf(fp, "%sHAS_PHYSVIRT_OFFSET", others++ ? "|" : "");
+ if (machdep->flags & ARM64_MTE)
+ fprintf(fp, "%sARM64_MTE", others++ ? "|" : "");
fprintf(fp, ")\n");
fprintf(fp, " kvbase: %lx\n", machdep->kvbase);
@@ -1023,7 +1063,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 +1673,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 +4605,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 20237b72a10b..aa8eba83b7f4 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)
/*
* Get kimage_voffset from /dev/crash
--
2.31.1
--
Crash-utility mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
