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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/25/23 15:15, HAGIO KAZUHITO(萩尾 一仁) wrote:

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.

Thank you for the information, Kazu.

I did not get the machine(armv8.5 later), I can not test this one, but I haven't seen any regression issues.

Anyway, the attached patch looks good to me. So: Ack


Thanks.

Lianbo


On 2023/12/22 12:30, qiwu.chen@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx>
---
   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
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux