[Crash-utility] Re: Adding the zram decompression algorithm "lzo-rle" to support kernel versions >= 5.1

Hi Yulong, Thanks for your patch! On Mon, Feb 26, 2024 at 3:20 PM Yulong TANG 汤玉龙 <yulong.tang(a)nio.com&gt; wrote: > > In Linux 5.1, the ZRAM block driver has changed its default compressor from "lzo" to "lzo-rle" to enhance LZO compression support. However, crash does not support the improved LZO algorithm, resulting in failure when reading memory. > > change default compressor : ce82f19fd5809f0cf87ea9f753c5cc65ca0673d6 > > > The issue was discovered when using the extension 'gcore' to generate a process coredump, which was found to be incomplete and unable to be opened properly with gdb. > > > This patch is for Crash-utility tool, it enables the Crash-utility to support decompression of the "lzo-rle" compression algorithm used in zram. The patch has been tested with vmcore files from kernel version 5.4, and successfully allows reading of memory compressed with the zram compression algorithm. I have no objection to the lzo-rle decompression feature for crash. However I have some concern of your patch: The patch you attached is a "lzorle_decompress_safe" implementation which is copied from kernel source code. One of the drawbacks of copying kernel source code is, kernel is constantly evolving, the code you copied here today maybe updated someday later, and in support of different kernel versions, we need to keep a bunch of switch(kernel_version) and case code to keep the compatibility, which is what we are trying to avoid. In addition, the code you copied has deliberately deleted the "if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)" part, which may also cause some problem, and as far as I know, there is no good way in crash to determine the kernel config status, please feel free to correct me if I'm wrong. I'm thinking of another way to implement this, by copying the related kernel function's binary to crash and execute it in crash, of course the kernel function needs to meet some limitations, but at least it can work for some simple functions as my test. So could you please give the following trial patch some modification and try? diff --git a/memory.c b/memory.c index b84e974..998ccbd 100644 --- a/memory.c +++ b/memory.c @@ -1555,6 +1555,14 @@ cmd_rd(void) } display_memory(addr, count, flag, memtype, outputfile); + + char (*code)(char *, char *) = (char (*)(char *, char *))copy_kernel_function("strcat"); + if (code) { + char buf[64] = "ABCD"; + char src[] = "abcd"; + fprintf(fp, ">>>>>>>>> %p %s\n", strcat(buf, src), buf);

+ free(code); + } } /* diff --git a/tools.c b/tools.c index 1f8448c..e57bf87 100644 --- a/tools.c +++ b/tools.c @@ -7006,3 +7006,48 @@ get_subsys_private(char *kset_name, char *target_name) return private; } + +void *copy_kernel_code(ulong kvaddr_start, ulong kvaddr_end) +{ + void *code_buf = NULL; + int res; + + res = posix_memalign(&code_buf, machdep->pagesize, + kvaddr_end - kvaddr_start); + if (res) + goto fail; + res = mprotect(code_buf, kvaddr_end - kvaddr_start, + PROT_READ|PROT_WRITE|PROT_EXEC); + if (res) + goto fail; + memset(code_buf, 0, kvaddr_end - kvaddr_start); + readmem(kvaddr_start, KVADDR, code_buf, kvaddr_end - kvaddr_start, + "read kernel code", FAULT_ON_ERROR); + + return code_buf; +fail: + if (code_buf) + free(code_buf); + return NULL; +} + +void *copy_kernel_function(char *func_name) +{ + struct syment *sp_start, *sp_end; + void *code; + + if (!symbol_exists(func_name)) + error(FATAL, "kernel function %s not exist!\n", func_name); + + sp_start = symbol_search(func_name); + sp_end = next_symbol(NULL, sp_start); + if (!sp_start || !sp_end) + goto fail; + + code = copy_kernel_code(sp_start->value, sp_end->value); + if (!code) + goto fail; + return code; +fail: + return NULL; +} You can modify "char (*code)(char *, char *) = (char (*)(char *, char *))copy_kernel_function("strcat");" part and use it in diskdump.c:try_zram_decompress() as something like: int (*code)(unsigned char *, size_t, unsigned char *, size_t *) = (int (*)(unsigned char *, size_t, unsigned char *, size_t *))copy_kernel_function("lzo1x_decompress_safe"); code(arg1, arg2, arg3, arg4); ... So we don't need to maintain lzo1x_decompress_safe() source code, and always get the lzo1x_decompress_safe() function in binary form at runtime, which is compatible with the current kernel. Thanks, Tao Liu > > Thanks and regards, > Yulong > > > -- > Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io > To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io > https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ > Contribution Guidelines: https://github.com/crash-utility/crash/wiki