[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 Kazu and Yulong,

On Fri, Mar 1, 2024 at 9:33 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
>
> On 2024/03/01 9:29, HAGIO KAZUHITO(萩尾 一仁) wrote:
> > On 2024/02/29 19:08, Tao Liu wrote:
> >> 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?
> >
> > Hi Tao,
> >
> > That's an interesting idea, but I'm very afraid of copying a function
> > from data (vmcore) and executing it, for security.  Maybe it's not
> > impossible that someone sends a crafted vmcore to your support team..
> >
Yeah, I haven't thought of the security issue...

> > Also, with that, we cannot handle a vmcore of another architecture, e.g.
> > arm64 vmcores with x86_64 crash with target=ARM64.

I have thought about this. I'm exploring the possibility of the
feature about "allowing crash to execute some kernel code".

One advantage of this, is we can skip the work of maintaining kernel
version specific code, and can always get the current kernel
compatible functions for use.

And it does have architecture and security issues as you mentioned.
I'm thinking of adding a code emulator to solve this, kind of like the
following pseudocode:

in crash:

char share_mem_buffer[4096];
char code[4096];
copy_kernel_function(code);

pid t = fork();
if (t > 0) {
  wait(pid);
} else if (t == 0) {
  move_child_process_into_cgroup_or_namespace_to_limit_access();
  execute("qemu-aarch64", code, share_mem_buffer);
  exit(0)
}

read_mem(share_mem_buffer);

By cgroup and namespace control, we limit the kernel code access to
the outer resources.
By qemu-aarch64 and similar emulators, we simulate the arch dependent code.

Also, the kernel function itself needs to have some limitations, such
as 1) a stand alone function, 2) no sub function calling or global
variable access etc...

Just my immature idea of solving copy/paste of some pure kernel code.
It is more complex and maybe hard to implement, not sure if there will
be more kernel code being copied to crash in the future. But for this
case, I agree just copying the kernel code is the simplest way.

> >
> > sorry I still haven't check Yulong's patch, but if lzo-rle library is
> > not available, maybe we need to import the code..?
>
> seeing lib/lzo/ in the kernel source, lzo-rle is not far from lzo, the
> lzo library will not support lzo-rle?

Yeah, I have the same question. However by comparing lzo library
function [1] and kernel lib/lzo/, there are differences. I suspect the
current lzo library won't work for lzo-rle, but I agree it's worth a
try of confirmation.

[1]: https://github.com/nemequ/lzo/blob/0083878c235a89ef96a009d1ff0b500f3a364e4b/minilzo/minilzo.c#L5395

Thanks,
Tao Liu
>
> Thanks,
> Kazu
--
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