Date: Tue, 7 Feb 2023 14:02:27 +0000
From: "Aureau, Georges (Kernel Tools ERT)" <georges.aureau@hpe.com>
To: HAGIO KAZUHITO(?????) <k-hagio-ab@nec.com>, "Discussion list for
crash utility usage, maintenance and development"
<crash-utility@redhat.com>
Subject: Re: [Crash-utility] kmem -s/-S not working properly on
RHEL8.6/8.7
Message-ID:
<SJ0PR84MB1482D6E7A2540ED4640603A69FDB9@SJ0PR84MB1482.NAMPRD84.PROD.OUTLOOK.COM>
Content-Type: text/plain; charset="utf-8"
Hello Kazu,
Thanks tons for your valuable comments, below is round #2.
With respect to the RHEL8 crash patch, it consisted of simply removing the 'if (THIS_KERNEL_VERSION >= LINUX(5,7,0))' from freelist_ptr(), suited for RHEL8.6/RHEL8.7, but obviously not for upstream.
You are right, Georges.
Usually I would recommend using the upstream crash tool to analyze upstream kernel vmcore. Correspondingly, RHEL crash tool analyzes the RHEL vmcore.
But anyway, this patch is a good improvement for the situation. I like the idea of searching for the bswap instruction from the asm code.
Cheers,
Georges
--
diff --git a/defs.h b/defs.h
index 33a823b..56d6cf4 100644
--- a/defs.h
+++ b/defs.h
@@ -2638,6 +2638,7 @@ struct vm_table { /* kernel VM-related data */
#define SLAB_OVERLOAD_PAGE (0x8000000)
#define SLAB_CPU_CACHE (0x10000000)
#define SLAB_ROOT_CACHES (0x20000000)
+#define FREELIST_PTR_BSWAP (0x40000000)
I would suggest adding the prefix "SLAB_" for the macro definition "FREELIST_PTR_BSWAP".
#define IS_FLATMEM() (vt->flags & FLATMEM)
#define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM)
diff --git a/memory.c b/memory.c
index 5141fbe..b235b7c 100644
--- a/memory.c
+++ b/memory.c
@@ -320,6 +320,7 @@ static void dump_per_cpu_offsets(void);
static void dump_page_flags(ulonglong);
static ulong kmem_cache_nodelists(ulong);
static void dump_hstates(void);
+static void freelist_ptr_init(void);
static ulong freelist_ptr(struct meminfo *, ulong, ulong);
static ulong handle_each_vm_area(struct handle_each_vm_area_args *);
@@ -370,7 +371,6 @@ mem_init(void)
DISPLAY_DEFAULT = (sizeof(long) == 8) ? DISPLAY_64 : DISPLAY_32;
}
-
/*
* Stash a few popular offsets and some basic kernel virtual memory
* items used by routines in this file.
@@ -789,6 +789,8 @@ vm_init(void)
MEMBER_OFFSET_INIT(kmem_cache_name, "kmem_cache", "name");
MEMBER_OFFSET_INIT(kmem_cache_flags, "kmem_cache", "flags");
MEMBER_OFFSET_INIT(kmem_cache_random, "kmem_cache", "random");
+ if (VALID_MEMBER(kmem_cache_random))
+ freelist_ptr_init();
MEMBER_OFFSET_INIT(kmem_cache_cpu_freelist, "kmem_cache_cpu", "freelist");
MEMBER_OFFSET_INIT(kmem_cache_cpu_page, "kmem_cache_cpu", "page");
if (INVALID_MEMBER(kmem_cache_cpu_page))
@@ -19519,13 +19521,65 @@ count_free_objects(struct meminfo *si, ulong freelist)
return c;
}
+/*
+ * With CONFIG_SLAB_FREELIST_HARDENED, freelist_ptr's are crypted with xor's,
+ * and for recent release with an additionnal bswap. Some releases prio to 5.7.0
+ * may be using the additionnal bswap. The only easy and reliable way to tell is
+ * to inspect assembly code (eg. "__slab_free") for a bswap instruction.
+ */
+static int
+freelist_ptr_bswap_x86(void)
+{
+ struct syment *sp, *spn;
+ char buf1[BUFSIZE];
+ char buf2[BUFSIZE];
+ char *arglist[MAXARGS];
+ ulong vaddr;
+ int found;
+
+ if (!(sp = symbol_search("__slab_free")) ||
+ !(spn = next_symbol(NULL, sp)))
+ return FALSE;
+
+ sprintf(buf1, "x/%ldi 0x%lx", spn->value - sp->value, sp->value);
The number of instructions disassembled by gdb is: spn->value - sp->value, which is much more than actually needed.
Can you please replace it with this one?
+ sprintf(buf1, "disassemble 0x%lx, 0x%lx", sp->value, spn->value);
+
+ found = FALSE;
+ vaddr = 0;
+ open_tmpfile();
+ gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR);
+ rewind(pc->tmpfile);
+ while (fgets(buf2, BUFSIZE, pc->tmpfile)) {
With the above change, need to add it as below:
+ if (STRNEQ(buf, "Dump of assembler code"))
+ continue;
Thanks.
Lianbo
+ if (parse_line(buf2, arglist) < 3)
+ continue;
+
+ if ((vaddr = htol(strip_ending_char(arglist[0], ':'),
+ RETURN_ON_ERROR|QUIET, NULL)) >= spn->value)
+ continue;
+
+ if (STREQ(arglist[2], "bswap")) {
+ found = TRUE;
+ break;
+ }
+ }
+ close_tmpfile();
+ return found;
+}
+
+static void
+freelist_ptr_init(void)
+{
+ if (THIS_KERNEL_VERSION >= LINUX(5,7,0) ||
+ ((machine_type("X86_64") || machine_type("X86")) && freelist_ptr_bswap_x86()))
+ vt->flags |= FREELIST_PTR_BSWAP;
+}
+
static ulong
freelist_ptr(struct meminfo *si, ulong ptr, ulong ptr_addr)
{
if (VALID_MEMBER(kmem_cache_random)) {
/* CONFIG_SLAB_FREELIST_HARDENED */
- if (THIS_KERNEL_VERSION >= LINUX(5,7,0))
+ if (vt->flags & FREELIST_PTR_BSWAP)
ptr_addr = (sizeof(long) == 8) ? bswap_64(ptr_addr)
: bswap_32(ptr_addr);
return (ptr ^ si->random ^ ptr_addr);
------------------------------