ckolivas / lrzip

Long Range Zip
http://lrzip.kolivas.org
GNU General Public License v2.0
615 stars 76 forks source link

Archives created with -n cannot be decompressed #94

Closed DavidCWGA closed 4 years ago

DavidCWGA commented 6 years ago

An archive created with -n causes the error "Unable to malloc buffer of size 9332470592 in this environment" when attempting to decompress.

Creating an archive from the same source with no arguments can be extracted fine.

lrzip version 0.631 Uncompressed source size: 31G Archive size with -n: 16G Archive size with no args: 8.3G

Aeny202 commented 4 years ago

I wish I would have found out about this before I went and used -n on a bunch of my tar archives. The limit seems to be somewhere between 10GB and 11GB of source file.

I created 2 files using dd if=/dev/urandom of=sample.txt bs=1G count=10 iflag=fullblock, this one decompressed just fine. However, a file created with dd if=/dev/urandom of=sample.txt bs=1G count=11 iflag=fullblock failed with

[aeny@butterfly downloads]$ /usr/bin/lrzip -d sample.txt.lrz Output filename is: sample.txt Decompressing... Unable to malloc buffer of size 11105056085 in this environment No such file or directory Fatal error - exiting

Both files were compressed with lrzip -nUp 1 sample.txt, with versionlrzip version 0.631

Output of lrzip -i sample.txt.lrz

lrzip --i sample.txt.lrz Reading profile /etc/firejail/lrzip.profile sample.txt.lrz: lrzip version: 0.6 file Compression: rzip alone Decompressed file size: 11811160064 Compressed file size: 11811700879 Compression ratio: 1.000 MD5 used for integrity testing MD5: 17067f6e92b09e65bb70506b48d21962

pete4abw commented 4 years ago

Some system info would be useful. lrzip is 32 bit or 64 bit: Architecture: Processor: Total Ram: Swap Size:

Also, when creating a test file, don't use urandom. It's useless. Your compressed file is larger than the decompressed data.

Decompressed file size: 11811160064
Compressed file size: 11811700879

I'll take a look at the unzip code.

Aeny202 commented 4 years ago

Thanks for taking a look at this. I'll do my best to fill in the data you requested, if anything is missing feel free to reach out again: lrzip bitness:

[aeny@butterfly ~]$ file /usr/bin/lrzip /usr/bin/lrzip: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=4cc80bd91b698fda948450c48de8230182418032, for GNU/Linux 3.2.0, stripped

Archtecture:

[aeny@butterfly ~]$ uname -a Linux butterfly 5.3.13-arch1-1 SMP PREEMPT Sun, 24 Nov 2019 10:15:50 +0000 x86_64 GNU/Linux

Processor:

[aeny@butterfly ~]$ lscpu | grep Model Model: 142 Model name: Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz

Total ram + swap size:

[aeny@butterfly ~]$ free -h total used free shared buff/cache available Mem: 31Gi 3.9Gi 26Gi 270Mi 643Mi 26Gi Swap: 20Gi 1.1Gi 19Gi

I can redo the test with better test data if that would help. My idea was to use /dev/urandom so it would be reproducible. If you have a suggestion for better testdata, then please let me know.

pete4abw commented 4 years ago

Would you please do an lrzip -vvi sample.txt.lrz? This will provide additional file information about the file. Also, and this is for @DavidCWGA too, it's very likely the output file is fine. I would like you to try the decompress again with this command: lrzip -vvd sample.txt.lrz I want to see what happens in this part of stream.c, where the memory allocation occurs. Also, please extract the first 64 bytes of the lrzip header of the failing file and attach. Something like head -c 64 sample.txt.lrz > lrzip.header where bytes 0-23 are the magic header, and chunk and stream data headers follow.

Aeny202 commented 4 years ago

Sure thing! lrzip -vvi

[aeny@butterfly downloads]$ /usr/bin/lrzip -vvi sample.txt.lrz Detected lrzip version 0.6 file. Rzip chunk 1: Chunk byte width: 5 Chunk size: 11811160064 Stream: 0 Offset: 31 Block Comp Percent Size 1 none 100.0% 540688 / 540688 Offset: 0 Head: 0 Stream: 1 Offset: 31 Block Comp Percent Size 1 none 100.0% 11105056085 / 11105056085 Offset: 0 Head: 11105596837 2 none 100.0% 706103979 / 706103979 Offset: 0 Head: 0 Rzip compression: 100.0% 11811700752 / 11811160064 Back end compression: 100.0% 11811700752 / 11811700752 Overall compression: 100.0% 11811700752 / 11811160064 sample.txt.lrz: lrzip version: 0.6 file Compression: rzip alone Decompressed file size: 11811160064 Compressed file size: 11811700879 Compression ratio: 1.000 MD5 used for integrity testing MD5: 156d32ebf6a9eb03660cef3cb200ebbd

lrzip -vvd

[aeny@butterfly downloads]$ /usr/bin/lrzip -vvd sample.txt.lrz The following options are in effect for this DECOMPRESSION. Threading is ENABLED. Number of CPUs detected: 8 Detected 33315168256 bytes ram Compression level 7 Nice Value: 19 Show Progress Max Verbose Temporary Directory set as: ./ Output filename is: sample.txt Detected lrzip version 0.6 file. MD5 being used for integrity testing. Decompressing... Reading chunk_bytes at 24 Expected size: 11811160064 Chunk byte width: 5 Reading eof flag at 25 EOF: 1 Reading expected chunksize at 26 Chunk size: 11811160064 Reading stream 0 header at 32 Reading stream 1 header at 48 Reading ucomp header at 11105056165 Fill_buffer stream 0 c_len 540688 u_len 540688 last_head 0 Starting thread 0 to decompress 540688 bytes from stream 0 Thread 0 decompressed 540688 bytes from stream 0 Taking decompressed data from thread 0 Reading ucomp header at 64 Fill_buffer stream 1 c_len 11105056085 u_len 11105056085 last_head 11105596837 Unable to malloc buffer of size 11105056085 in this environment No such file or directory Fatal error - exiting

I ran head -c 64 sample.txt.lrz > lrzip.header and attached the resulting file with .log extension because GitHub seems very picky about those things.

I had to recreate the file because I had removed it, so the md5 hash will differ from previous output, I'll try to keep the sample file persistent from now on.

lrzip.log

pete4abw commented 4 years ago

Thank you. @Aeny202 . Maybe @DavidCWGA could provide the same output? This will require some interesting debugging. lrzip was never expected to produce larger files than fed. The issue between 10G and 11G is interesting, but I think coincidental with the way lrzip allocates working ram during the early phase. It has more to do with this test in stream.c line 1649.

    if (unlikely(u_len > control->maxram))
        fatal_return(("Unable to malloc buffer of size %lld in this environment\n", u_len), -1);

The differences between the way -n is handled and other compressions is the threadding. Only one thread is used for -n pre-pass compression. Multiple threads are used for others thus, total required ram is divided by the number of threads.

I'll look at this, but am going to wait for @DavidCWGA to provide details by running lrzip -vvi and -vvd and the first 64 bytes of his lrzip header file to see what his experience looks like. Thank you for reporting.

pete4abw commented 4 years ago

@Aeny202 . Please patch stream.c or edit as follows, recompile, and try. lrzip will still fail, but I want to see the output when you run lrzip -vvd file

diff --git a/stream.c b/stream.c
index 4dd2d82..0541df7 100644
--- a/stream.c
+++ b/stream.c
@@ -1637,7 +1637,8 @@ fill_another:
                fatal_return(("Invalid data compressed len %lld uncompressed %lld last_head %lld\n",
                             c_len, u_len, last_head), -1);
        }
-       print_maxverbose("Fill_buffer stream %d c_len %lld u_len %lld last_head %lld\n", streamno, c_len, u_len, last_head);
+       print_maxverbose("Fill_buffer stream %d c_len %lld u_len %lld last_head %lld max avail ram %lld\n", 
+                       streamno, c_len, u_len, last_head, control->maxram);

        padded_len = MAX(c_len, MIN_SIZE);
        sinfo->total_read += padded_len;

I just want to see what maxram shows for your system. I believe because the output of lrzip -n resulted in a larger file and the chunk size was larger than ever anticipated, this caused the error. However, actually, since the fatal abort is not caused by a malloc, but by a simple numerical comparison, if (unlikely(u_len > control->maxram)) fatal_return(("Unable to malloc buffer of size %lld in this environment\n", u_len), -1);

a fix may be easy. Just want to see what maxram shows.

Aeny202 commented 4 years ago

Hey Pete, thanks for the patch. I thought I'd start out with compiling master but I'm afraid it doesn't anymore since the latest merges. I don't think I messed anything up since installing it from the AUR also ends with the same error, maybe I'm missing a new dependency that hasn't been added to the readme yet?:

/usr/bin/ld: ./.libs/libtmplrzip.a(7zCrcT8.o): in function CrcUpdate': 7zCrcT8.c:(.text+0x79): undefined reference toCrcUpdateT8' /usr/bin/ld: ./.libs/libtmplrzip.a(7zCrcT8.o): in function CrcCalc': 7zCrcT8.c:(.text+0x98): undefined reference toCrcUpdateT8' collect2: error: ld returned 1 exit status make[2]: [Makefile:739: lrzip] Error 1 make[2]: Waiting for unfinished jobs.... make[2]: Leaving directory '/home/aeny/.cache/yay/lrzip-git/src/lrzip' make[1]: [Makefile:958: all-recursive] Error 1 make[1]: Leaving directory '/home/aeny/.cache/yay/lrzip-git/src/lrzip' make: [Makefile:556: all] Error 2

If you can guide me past this then I'd gladly apply the patch and answer your questions :)

pete4abw commented 4 years ago

Oh boy. Since I have nasm compiler, I made a typo in lzma/C/Makefile.am. I'm pushing a hotfix for this. I am sorry. You can just edit line 19 of the Makefile.am and change the 7zCrcT8.c to 7zCrc.c.

--- a/lzma/C/Makefile.am
+++ b/tmp/lrzip/lzma/C/Makefile.am
@@ -16,7 +16,7 @@ if USE_ASM
   ASM_H += @top_srcdir@/lzma/ASM/x86/7zAsm.asm
   C_S += 7zCrcT8.c
 else
-  C_S += 7zCrcT8.c 
+  C_S += 7zCrc.c 
 endif
Aeny202 commented 4 years ago

Thanks for the quick fix, I applied both patches and this seems to be the the generated output. Hopefully this is helpful to you:

[aeny@butterfly lrzip]$ ./lrzip -vvd /shared/downloads/sample.txt.lrz The following options are in effect for this DECOMPRESSION. Threading is ENABLED. Number of CPUs detected: 8 Detected 33315168256 bytes ram Compression level 7 Nice Value: 19 Show Progress Max Verbose Temporary Directory set as: ./ Output filename is: /shared/downloads/sample.txt Detected lrzip version 0.6 file. MD5 being used for integrity testing. Decompressing... Reading chunk_bytes at 24 Expected size: 11811160064 Chunk byte width: 5 Reading eof flag at 25 EOF: 1 Reading expected chunksize at 26 Chunk size: 11811160064 Reading stream 0 header at 32 Reading stream 1 header at 48 Reading ucomp header at 11105056165 Fill_buffer stream 0 c_len 540688 u_len 540688 last_head 0 max avail ram 11105054720 Starting thread 0 to decompress 540688 bytes from stream 0 Thread 0 decompressed 540688 bytes from stream 0 Taking decompressed data from thread 0 Reading ucomp header at 64 Fill_buffer stream 1 c_len 11105056085 u_len 11105056085 last_head 11105596837 max avail ram 11105054720 Unable to malloc buffer of size 11105056085 in this environment No such file or directory Fatal error - exiting

pete4abw commented 4 years ago

Sure does help! Here's the issue. Fill_buffer stream 1 c_len 11105056085 u_len 11105056085 last_head 11105596837 max avail ram 11105054720 c_len is compressed length, u_len is uncompressed length, and max available ram is less than the uncompressed length. This is what is tested. There's a couple of problems here.

  1. As previously discussed, having an uncompressible file should never have been created! A warning should have been issued.
  2. While this will take a bit of work to figure out why the so-called max available ram is so close to the ram needed by u_len, the max available is a somewhat contrived value. But in a decompression, this should not be a barrier.
  3. @ckolivas is the maintainer and designer of all things with regards to memory and scheduling and the guru on everything else, so any fix will have to run through him and have some extensive testing. Fussing with ram allocation in a rushed manner is ill-advised. This is because more than just -n depends on this, and rzip layer is run before all other compression/decompression.

THAT said, the output you provided gives me an idea which I've been mulling over since this thread opened. My solution would look like this. Make that fatal error into a warning. If the malloc fails, abort at that point, which it does. Something like this:

diff --git a/stream.c b/stream.c
index 7196863..c2a75d0 100644
--- a/stream.c
+++ b/stream.c
@@ -1647,8 +1647,10 @@ fill_another:
        sinfo->total_read += padded_len;
        fsync(control->fd_out);

+       /* is this really necessary? malloc may fail below if a problem
        if (unlikely(u_len > control->maxram))
                fatal_return(("Unable to malloc buffer of size %lld in this environment\n", u_len), -1);
+       */
        max_len = MAX(u_len, MIN_SIZE);
        max_len = MAX(max_len, c_len);
        s_buf = malloc(max_len);

The malloc fail is here:

    if (unlikely(u_len && !s_buf))
        fatal_return(("Unable to malloc buffer of size %lld in fill_buffer\n", u_len), -1);

Try this and see what happens. Until @ckolivas reviews, though, this is as far as I will go for now. No guarantees, but it is worth a try. @DavidCWGA too.

ckolivas commented 4 years ago

Fixed in master, thanks very much for debugging, Peter!