coredump: add zstandard support for coredumps

[nolange]

This adds support for coredumps being compressed (and decompressed) with zstd. It steals parts from #8207, particularly the meson bits.

I tested compression/decompression of coredumps on a live system.

The other PR also added zstd support for journals, but for me this leaves too many questions open (first and foremost if this actually makes a useful difference).

[nolange]

Reading through the previous PR compression, seems like zstd compression for the journal itself was compressed.
So this will be some critism/questions about the journal format and implementation in general, mixed with some specifics of zstd. Probably should end into multiple Issues or TODO list.

Compressed data in the Journal

• The journal format itself seems rather unfit to tack on compression. Likely alot better results can be archived if the FS just does compression transparently.
• the key is in the compressed data (?!?), and decompress_startswith will be repeatedly called until the right key is found.
• if found, the decompression is restarted from scratch.

To address the above:
The decompression context is not that cheap to throw it away (LZ4 might be different). Ideally one would reuse a context atleast through the whole search. Thats of course not that easy if by now there are 3 formats that could all appear in one journal.
Easier would be to save the context between decompress_startswith/decompress_blob. Quite frankly just use the "streaming decompression" used for external files instead. The uncompressed size is stored in the journal data too, so that's no loss in information.

Avoiding dependencies to now 3 additional dso's

Might make sense to dlopen the fitting library only if is going to be used. In my case this would be just for compressing/decompressing coredumps.
Or, even crazier, allow adding a script in (/etc,/run,/lib)/systemd/coredump/storage. if existing,
it will be passed the open fd and called with storage -o /proc/self/fd/N.
Might need to figure out a way to report back a fitting file-extension (just dump it on stdout after successfully compressing?)

Decompression would be similar with storage -d -o /proc/self/fd/N

(Of course that's nothing you want to do for data embedded in the journal)

Supporting sparse files

coredumps are a whole bunch of zeros, using sparse files gives insane savings here. My single test involved uncompressing a zstd compressed coredump with the zstd tool. the file was 1.2GB in size, but took up only ~60MB on disk.

not storing the coredump before compressing

Currently the uncompressed coredump will be used to create a stacktrace.
Skipping that, and feeding the stream directly into the compressor would preferable IMHO, as the stacktrace can be recreated later (potentially after fetching debug info, on another system).
Even if not, would it be possible to create the stacktrace from still existing mappings from the /proc filesystem?

On that note, since coredump does access the /proc directory of the deceased procress..
Is it taken into account, that this might not work?

Quote about /proc/sys/kernel/core_pipe_limit from the manpage:

A value of 0 in this file is special. It indicates that unlimited
processes may be captured in parallel, but that no waiting will take
place (i.e., the collecting program is not guaranteed access to
/proc/). The default value for this file is 0.

[keszybz]

Looks quite nice. I think that with the mmap calls gone, the code will be pleasantly simpler.

I was wary of adding another dependency, but since this is opt-in at compile time, it is OK. Some distributions might choose not to enable it.

[nolange]

@keszybz Also tested the revised patch, works fine here. Build failures seem unrelated.

Generally I have a problem though with everything in systemd now depending on libzstd, when only two tools use it (coredumpd and coredumpctl). What would be your direction to go?

IMHO, the de(compression) stuff should be put into src/basic, and libraries be dlopened on demand.

[nolange]

added a line in README for the libzstd dependency,
and guessed the lines in the mkosi files in another commit.

[poettering]

Looks pretty good, just some nitpicks

[evverx]

@nolange as far as I can tell, neither compress_stream_zstd nor decompress_stream_zstd is tested anywhere. Could you update src/journal/test-compress.c (where test_compress_stream is used to make sure at least the basic stuff is covered).

@terrelln I linked a fuzzer against libzstd I built with ASan+UBsan and fuzzer-no-link. It failed as soon as it started with

legacy/zstd_v07.c:4056:47: runtime error: addition of unsigned offset to 0x61d000003280 overflowed to 0x61d00000326c
    #0 0x7f5eeb892707 in ZSTDv07_decompressContinue (/home/vagrant/zstd/lib/libzstd.so.1+0x76e707)
    #1 0x7f5eeb898343 in ZBUFFv07_decompressContinue (/home/vagrant/zstd/lib/libzstd.so.1+0x774343)
    #2 0x7f5eeb79c097 in ZSTD_decompressLegacyStream (/home/vagrant/zstd/lib/libzstd.so.1+0x678097)
    #3 0x7f5eeb79a482 in ZSTD_decompressStream (/home/vagrant/zstd/lib/libzstd.so.1+0x676482)
    #4 0x7f5eecc18acf in decompress_stream_zstd /home/vagrant/systemd/build/../src/journal/compress.c:853:39
    #5 0x536c04 in LLVMFuzzerTestOneInput /home/vagrant/systemd/build/../src/fuzz/fuzz-compress.c:36:16
    #6 0x441988 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/vagrant/systemd/out/fuzz-compress+0x441988)
    #7 0x4425b0 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/home/vagrant/systemd/out/fuzz-compress+0x4425b0)
    #8 0x44367e in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/vagrant/systemd/out/fuzz-compress+0x44367e)
    #9 0x44510c in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/vagrant/systemd/out/fuzz-compress+0x44510c)
    #10 0x42fabb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/vagrant/systemd/out/fuzz-compress+0x42fabb)
    #11 0x41fa46 in main (/home/vagrant/systemd/out/fuzz-compress+0x41fa46)
    #12 0x7f5eebba71a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #13 0x41fa9d in _start (/home/vagrant/systemd/out/fuzz-compress+0x41fa9d)

As far as I can see, pointer-overflows are ignored (unless ubsan_pointer_overflow is passed explicitly) and I can't seem to figure out why. Are you planning to fix this and https://bugs.chromium.org/p/oss-fuzz/issues/list?sort=-opened&q=proj%3Azstd%20pointer-overflow&can=1 eventually?

[evverx]

Other than that systemd seems to be failing to compile on Debian Buster with

7@@journal-client@sta/compress.c.o' -MF 'src/journal/da05bd7@@journal-client@sta/compress.c.o.d' -o 'src/journal/da05bd7@@journal-client@sta/compress.c.o' -c ../src/journal/compress.c
../src/journal/compress.c: In function ‘compress_stream_zstd’:
../src/journal/compress.c:720:21: error: implicit declaration of function ‘ZSTD_CCtx_setPledgedSrcSize’ [-Werror=implicit-function-declaration]
                 z = ZSTD_CCtx_setPledgedSrcSize(cctx, in_totalsize);
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/journal/compress.c:720:21: error: nested extern declaration of ‘ZSTD_CCtx_setPledgedSrcSize’ [-Werror=nested-externs]
../src/journal/compress.c:725:13: error: implicit declaration of function ‘ZSTD_CCtx_setParameter’ [-Werror=implicit-function-declaration]
         z = ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1);
             ^~~~~~~~~~~~~~~~~~~~~~
../src/journal/compress.c:725:13: error: nested extern declaration of ‘ZSTD_CCtx_setParameter’ [-Werror=nested-externs]
../src/journal/compress.c:725:42: error: ‘ZSTD_c_checksumFlag’ undeclared (first use in this function); did you mean ‘ZSTD_error_checksum_wrong’?
         z = ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1);
                                          ^~~~~~~~~~~~~~~~~~~
                                          ZSTD_error_checksum_wrong
../src/journal/compress.c:725:42: note: each undeclared identifier is reported only once for each function it appears in
../src/journal/compress.c:755:37: error: implicit declaration of function ‘ZSTD_compressStream2’; did you mean ‘ZSTD_compressStream’? [-Werror=implicit-function-declaration]
                         remaining = ZSTD_compressStream2(
                                     ^~~~~~~~~~~~~~~~~~~~
                                     ZSTD_compressStream
../src/journal/compress.c:755:37: error: nested extern declaration of ‘ZSTD_compressStream2’ [-Werror=nested-externs]
../src/journal/compress.c:756:72: error: ‘ZSTD_e_end’ undeclared (first use in this function); did you mean ‘ZSTD_DDict’?
                                 cctx, &output, &input, is_last_chunk ? ZSTD_e_end : ZSTD_e_continue);
                                                                        ^~~~~~~~~~
                                                                        ZSTD_DDict
../src/journal/compress.c:756:85: error: ‘ZSTD_e_continue’ undeclared (first use in this function)
                                 cctx, &output, &input, is_last_chunk ? ZSTD_e_end : ZSTD_e_continue);
                                                                                     ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
[206/1596] Compiling C object 'src/journal/da05bd7@@journal-client@sta/catalog.c.o'.
ninja: build stopped: subcommand failed.
make: *** [Makefile:2: all] Error 1

$ dpkg -s libzstd-dev
Package: libzstd-dev
Status: install ok installed
Priority: optional
Section: libdevel
Installed-Size: 924
Maintainer: Debian Med Packaging Team <debian-med-packaging@lists.alioth.debian.org>
Architecture: i386
Multi-Arch: same
Source: libzstd
Version: 1.3.8+dfsg-3
Depends: libzstd1 (= 1.3.8+dfsg-3)
Description: fast lossless compression algorithm -- development files
 Zstd, short for Zstandard, is a fast lossless compression algorithm, targeting
 real-time compression scenarios at zlib-level compression ratio.
 .
 This package contains the headers and static library.
Homepage: https://github.com/facebook/zstd

[terrelln]

As far as I can see, pointer-overflows are ignored (unless ubsan_pointer_overflow is passed explicitly) and I can't seem to figure out why. Are you planning to fix this and https://bugs.chromium.org/p/oss-fuzz/issues/list?sort=-opened&q=proj%3Azstd%20pointer-overflow&can=1 eventually?

No, zstd uses pointer arithmetic for efficiency, and that can occasionally overflow. All platforms that we've tested support wrapping arithmetic for pointers. This is heavily baked into the code.

The alternative would be to do pointer -> cast to uintptr_t -> do arithmitic & validation -> cast to pointer -> dereference. But that is also undefined behavior, since you can't do arithmetic on uintptr_t and cast back to a pointer and dereference.

If you can demonstrate a platform that breaks because of this, please report a bug.

However, this should be the only type of UB that we ignore, and we never ignore any ASAN/MSAN bugs. So if you see other types of errors, please let us know!

[nolange]

@evverx I did not know those were recent additions (or recently moved to stable). I provide 1.3.x compatibility with another commit, raising the dependency if of course the easiest/cleanest option.

For Debian Buster, libzstd-dev is at version 1.4.4 in the backports, so that would not be a showstopper.

I only tested compilation with 1.3.1 so far. Would test some more, if that's the preferred solution.

@terrelln You would need to find a cpu with something different than 2-complement integers. Maybe there are some obscure DSPs around, but you would run in alot other issues before you will be able to run zstd or systemd on them ;)

[evverx]

@terrelln

All platforms that we've tested support wrapping arithmetic for pointers. This is heavily baked into the code.

It seems it isn't baked into the build system though. The "usan" and "uasan" targets (I looked at to figure out how libzstd should be built to avoid known issues) pass "CC=clang MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize-recover=signed-integer-overflow -fsanitize=undefined -Werror"" as far as I can see.

@nolange

I only tested compilation with 1.3.1 so far. Would test some more, if that's the preferred solution.

I'm not sure whether it's necessary to support 1.3.*. I'm fine with just bumping libzstd in meson.build. Though I'd wait for someone else to chime in :-)

What's more important I think is that the new functions are still untested in the sense they aren't covered by the testsuite automatically. I think it would be great to update src/journal/test-compress.c.

[evverx]

FWIW now it's failing to compile on Debian Buster with

../src/journal/compress.c:81:20: error: redundant redeclaration of ‘ZSTD_compressStream’ [-Werror=redundant-decls]
 ZSTDLIB_API size_t ZSTD_compressStream(ZSTD_CStream *zcs, ZSTD_outBuffer *output, ZSTD_inBuffer *input);
                    ^~~~~~~~~~~~~~~~~~~
In file included from ../src/journal/compress.c:20:
/usr/include/zstd.h:351:20: note: previous declaration of ‘ZSTD_compressStream’ was here
 ZSTDLIB_API size_t ZSTD_compressStream(ZSTD_CStream* zcs, ZSTD_outBuffer* output, ZSTD_inBuffer* input);
                    ^~~~~~~~~~~~~~~~~~~
../src/journal/compress.c:82:20: error: redundant redeclaration of ‘ZSTD_flushStream’ [-Werror=redundant-decls]
 ZSTDLIB_API size_t ZSTD_flushStream(ZSTD_CStream *zcs, ZSTD_outBuffer *output);
                    ^~~~~~~~~~~~~~~~
In file included from ../src/journal/compress.c:20:
/usr/include/zstd.h:352:20: note: previous declaration of ‘ZSTD_flushStream’ was here
 ZSTDLIB_API size_t ZSTD_flushStream(ZSTD_CStream* zcs, ZSTD_outBuffer* output);
                    ^~~~~~~~~~~~~~~~
../src/journal/compress.c:83:20: error: redundant redeclaration of ‘ZSTD_endStream’ [-Werror=redundant-decls]
 ZSTDLIB_API size_t ZSTD_endStream(ZSTD_CStream *zcs, ZSTD_outBuffer *output);
                    ^~~~~~~~~~~~~~
In file included from ../src/journal/compress.c:20:
/usr/include/zstd.h:353:20: note: previous declaration of ‘ZSTD_endStream’ was here
 ZSTDLIB_API size_t ZSTD_endStream(ZSTD_CStream* zcs, ZSTD_outBuffer* output);
                    ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
[207/1596] Compiling C object 'src/journal/da05bd7@@journal-client@sta/journal-file.c.o'.
ninja: build stopped: subcommand failed.
make: *** [Makefile:2: all] Error 1

As far as I know, Debian Buster is still used on Semaphore CI so to catch this automatically (and to make sure it's gone once it's fixed) could you add libzstd-dev to the list of dependencies installed there:

diff --git a/semaphoreci/semaphore-runner.sh b/semaphoreci/semaphore-runner.sh
index 04f6ee2605..b15c043847 100755
--- a/semaphoreci/semaphore-runner.sh
+++ b/semaphoreci/semaphore-runner.sh
@@ -36,7 +36,7 @@ apt-get -q --allow-releaseinfo-change update
 apt-get -y dist-upgrade
 apt-get install -y eatmydata
 # The following four are needed as long as these deps are not covered by Debian's own packaging
-apt-get install -y libfdisk-dev libp11-kit-dev libssl-dev libpwquality-dev
+apt-get install -y libfdisk-dev libp11-kit-dev libssl-dev libpwquality-dev libzstd-dev
 apt-get purge --auto-remove -y unattended-upgrades
 systemctl unmask systemd-networkd
 systemctl enable systemd-networkd

?

[evverx]

It would be interesting to see how it fares on Ubuntu CI because it's the only CI where non-amd64 architectures are covered. @ddstreet would it be possible to install libzstd-dev there at least temporarily?

[nolange]

Lotsa changes... rebase because of formatting in meson.build, +-ZSTDwill be displayed in the features, added ZSTD to test-compress, fixed test-compress to build without warning if LZ4 is disabled, check returnvalue of ZSTD_CCtx_setParameter with ZSTD_isError, fixed for 1.3.x (tested coredump with 1.3.1), added libstd-dev for the CI builds

[nolange]

Any comments on the 2nds post meanwhile? Those are the bigger things IMHO.

1. lots of tools depend on liblz4, libxz, .... I guess this is for static linking, as meson wont handle transitive dependencies?
   Currently only coredumpd and coredumpctl would really need libzstd.
2. What about using dlopen to resolve the compression libraries when needed?
3. Perhaps allow [true, false, dlopen, auto] as option for compressors?
4. Those compressors would be better moved out of the journal into basic (or shared)

[shawnl]

not storing the coredump before compressing

I looked into this, and I get the feeling that coredumps are done very quite silly by the kernel. As long as everything is PIE we can just launch the coredump handler in the same address space as the program was in----all inside of the signal handler (which is safe because we never return from a core dump triggering signal handler), and this would make the kernel's job much easier too.

[nolange]

not storing the coredump before compressing

I looked into this, and I get the feeling that coredumps are done very quite silly by the kernel. As long as everything is PIE we can just launch the coredump handler in the same address space as the program was in----all inside of the signal handler (which is safe because we never return from a core dump triggering signal handler), and this would make the kernel's job much easier too.

"As long as everything is PIE"... that's already a steep requirement.
What I would expect is taking the code from elfutils, doing the same as eu-stack -p PID would do (uses PTRACE, so that capability is necessary).

ie.:

• use the /proc filesystem for stacktraces (fork a process to guard against crashes)
• store the coredump either compressed or uncompressed (sparse!). Or not at all.

[ddstreet]

@ddstreet thank you! Could you also install zstd there? I didn't know at the time that the test is skipped if zstdcat isn't installed. Sorry about that.

sure, just added 'zstd' to the list of packages to install for the 'upstream' and 'root-unittests' autopkgtests, as well as build-time deps, since the tests are run at build time too.

[evverx]

Looks like https://github.com/systemd/systemd/pull/8207/files#diff-969b60ad3d206fd45c208e266ccfed38L3106-L3111 got lost along the way. @nolange could you bring it back?

[evverx]

Interestingly, test-compress fails with

/* testing ZSTD compression */
/* create source from /dev/fd/63 */
/* test compression */
ZSTD compression finished (0 -> 13 bytes, inf%)
/* test decompression */
ZSTD decompression finished (13 -> 0 bytes, 0.0%)
/* test faulty decompression */
ZSTD decoder failed: Unknown frame descriptor
ZSTD decompression finished (13 -> 0 bytes, 0.0%)
Assertion 'r == -EFBIG' failed at src/journal/test-compress.c:223, function test_compress_stream(). Aborting.
Aborted (core dumped)

if an empty file is passed to it. I'm not sure whether it has anything to do with this PR though.

[nolange]

if an empty file is passed to it. I'm not sure whether it has anything to do with this PR though.

Faulty testcase (with file len 0), max_size will wrap to a huge number (max of size_t).

        r = decompress(dst, dst2, st.st_size - 1);
        assert_se(r == -EFBIG);

[nolange]

rebased verson pushed, support for zstd 1.3.x was kicked out. Please either give me concrete information for adding further CI stuff or handle this in another PR/commit, playing ping/pong to get this done ain't an efficient use of anyones time.

[evverx]

playing ping/pong to get this done ain't an efficient use of anyones time.

I'm not sure what "ping/pong" is in this context. Based on one of your comments my understanding was that you knew how to install libzstd-1.4* on Debian Buster and I refrained from giving detailed instructions. I seem to have misread that comment. Sorry about that.

Please either give me concrete information for adding further CI stuff or handle this in another PR/commit

I'll fix it later.

[nolange]

playing ping/pong to get this done ain't an efficient use of anyones time.

I'm not sure what "ping/pong" is in this context. Based on one of your comments my understanding was that you knew how to install libzstd-1.4* on Debian Buster and I refrained from giving detailed instructions. I seem to have misread that comment. Sorry about that.

I know how to do it in Buster, I dont know what happens in semaphore (which also drops all debian repos later and hooks in ubuntu ppas).
I dont know how I would test it either, other than pushing a commit so Id get a response someday tomorrow.

[evverx]

In case you're interested https://github.com/systemd/systemd/blob/master/semaphoreci/semaphore-runner.sh controls what happens on Semaphore CI. The idea was to put the 100-line bash script in the repository and keep it here so that anyone could read and edit it should the need arise. It appears it can get mixed up with Ubuntu CI. I'll try to add more comments there to make it clear that it has nothing to with Ubuntu CI.

[poettering]

looks excellent, one typo though

[evverx]

The unit tests passed on Ubuntu CI before the typo was fixed so it looks like it should be good to go.

[trentbuck]

• The journal format itself seems rather unfit to tack on compression. Likely alot better results can be archived if the FS just does compression transparently.

FYI, here's some actual numbers for the current status quo, since I didn't see anyone else posting them.

The systemd-journal is running with its default settings as at Debian 12 / systemd 252. The parent /var/log dataset is traditional text/plain log files generated by rsyslogd, nginx, &c. A single system.journal shows 40MiB uncompressed / 9MiB compressed. Most other datasets see compressratio of 1.01x (movies, photos) to 2.00x (OS, office docs, SQL DBs). collectd/rrdtool databases can go as high as 12.00x.

IIRC systemd-journal compresses per-journal-event, whereas filesystem-level transparent compression compresses per-filesystem-block. So it makes sense that OS-level compression will always show much better compression ratios.

cyber@obese:~$ zfs list -o compression,compressratio,referenced,logicalreferenced,name | sed -n '1p; /log/p'
COMPRESS        RATIO     REFER  LREFER  NAME
zstd            8.07x     1.47G   14.5G  obese/heavy/var/log
zstd            6.69x     58.2M    453M  obese/heavy/var/log/journal
zstd            4.97x     26.0M   92.4M  obese/light/var/log
zstd            5.08x      737M   2.56G  obese/light/var/log/journal
zstd            5.12x     6.09M   17.6M  obese/obese/var/log
zstd            5.24x      410M   2.12G  obese/obese/var/log/journal