AgentD / squashfs-tools-ng

A new set of tools and libraries for working with SquashFS images
Other
194 stars 30 forks source link

Creating image with certain input data & options - "packing file data: data corrupted" #110

Closed benkohler closed 1 year ago

benkohler commented 1 year ago

We began getting this gensquashfs error on a large (~4GB) squashfs image for the rootfs of our livecd. I've reduced the test case down to a specific subdir, to be able to reproduce & test much more quickly. Example run:

$ gensquashfs -fqD gensquashfs-test-case/ test.img man3/ASN1_OBJECT_new.3ssl.bz2: packing file data: data corrupted.

Strangely, it seems to only fail with the default block size, I tried a bunch of other block sizes and it succeeds:

$ gensquashfs -fqD gensquashfs-test-case/ test.img -b 4096 $ gensquashfs -fqD gensquashfs-test-case/ test.img -b 8192 $ gensquashfs -fqD gensquashfs-test-case/ test.img -b 16384 $ gensquashfs -fqD gensquashfs-test-case/ test.img -b 32768 $ gensquashfs -fqD gensquashfs-test-case/ test.img -b 65536 $ gensquashfs -fqD gensquashfs-test-case/ test.img -b 131072 man3/BIO_should_retry.3ssl.bz2: packing file data: data corrupted. $ gensquashfs -fqD gensquashfs-test-case/ test.img -b 262144 $

Also I'm able to get it to succeed with default block size if I turn up the job count high enough:

$ gensquashfs -fqD gensquashfs-test-case/ test.img man3/ASN1_OBJECT_new.3ssl.bz2: packing file data: data corrupted. gensquashfs -fqD gensquashfs-test-case/ test.img -j2 man2/pipe.2.bz2: packing file data: data corrupted. $ gensquashfs -fqD gensquashfs-test-case/ test.img -j0 man2/pciconfig_iobase.2: packing file data: data corrupted. $ gensquashfs -fqD gensquashfs-test-case/ test.img -j20 man3/CMSG_ALIGN.3: packing file data: data corrupted. $ gensquashfs -fqD gensquashfs-test-case/ test.img -j200 $

One more interesting thing is that even though I am routinely seeing 'data corrupted' in specific dirs like gensquashfs-test-case/man3/, if I point gensquashfs at that specific dir, it doesn't fail.

I'm going to try to attach a tarball of the test data you can use to reproduce.

benkohler commented 1 year ago

The current smallest test tarball I can make won't fit in the 25MB limit, here's a link to download it from google drive, I will work to make an even more reduced test case: https://drive.google.com/file/d/1FvKLUHFbquyFTqkPztKl8f8AbWRTbPs0/view?usp=share_link

benkohler commented 1 year ago

gensquashfs-test-case.tar.gz

With this test tarball, I can reproduce: gensquashfs -fqD gensquashfs-test-case/ test.image

benkohler commented 1 year ago

And on all 3 of my test machines, with either the reduced test case OR the whole 4GB test case, the cutoff seems to be 22->23 jobs.

-j21 -> fail -j22 -> fail -j23 -> success -j24 -> success

benkohler commented 1 year ago

Slightly mixed results depending on compressor:

gzip - SUCCESS lzma - FAIL lzo - SEGFAULT xz - FAIL lz4 - FAIL zstd - FAIL

AgentD commented 1 year ago

Segfault in the LZO compressor

This one was easy to fix: For legal reasons, the libsquashfs library cannot implement LZO, so the application does. The compressor is created in a fallback path, in case the library does implement it some day, and tried to clean up whatever the library returned, but the library returned an error and left the output pointer uninitialized.

This is fixed in 3bf38f07f0172b4698c205f72d923e9a37e53da7.

Data corruption error

Following the error code backwards, it originates in the fragment de-duplication code.

If a fragment block has already been written to disk, it needs to be read back and unpacked. Turns out, the code doing this contains an overzealous bounds check, that compared the on-disk size against the block size with '>=' instead of '>', reporting that the on-disk data is corrupted. This should be fixed in commit 9d189b6091a7011bfac8a9c09af750614d82a10d.

You managed to trigger an edge case, where the fragment block needs to be read back and is exactly 128k in size. That's why the problem goes away when using a gzip instead, or literally any other block size than 128k.

Relation to the thread pool processor

Why did it go away when cranking up the number of jobs?

It turns out, when compiling with the configure option --without-pthread (i.e. forcing libsquashfs to use the reference serial implementation), the problem also goes away when setting -j200, so what's happening here?

The block processor has an internal queue for pending and completed blocks. The length can be tuned when giving gensquashfs the option -Q. If not doing that, it automatically scales the queue length according to the number of jobs.

When holding the job count constant and cranking up the queue length (~500 or so), the problem goes away. When forcing a short queue and cranking up the job count, the problem stays.

What happens is, that the queue is so long, the entire file ends up in the queue until finally flushing them out. The fragment de-duplication reads the fragment data from the backlog queue, instead of the output file and the code mentioned above is not touched, not triggering the overzealous bounds check.

AgentD commented 1 year ago

Thanks for the reproducing samples by the way! I'll see if I can integrate them into my test suite on my end.

benkohler commented 1 year ago

Wow thanks for the fixes and the writeup. I can confirm that with these latest fixes, I am unable to trigger any failure with any of the test scenarios I can throw at it.

Dr-Emann commented 1 year ago

Wow, what a great, thorough bug report, and a nice in-depth response explaining not only that the bug was fixed, but what exactly was happening. This is a great thread, thanks to all involved!