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

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

 



Hi Yulong,

Thanks for your patch!

On Mon, Feb 26, 2024 at 3:20 PM Yulong TANG 汤玉龙 <yulong.tang@xxxxxxx> 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@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
--
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