Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suspiciously large seek offset results in failed test #143

Open
saethlin opened this issue Nov 23, 2021 · 10 comments
Open

Suspiciously large seek offset results in failed test #143

saethlin opened this issue Nov 23, 2021 · 10 comments
Labels
bug Something isn't working

Comments

@saethlin
Copy link

Description

This crate does not build with the latest beta, which means it will not in its current state build with the next stable. This is due to the use of #[deny(dead_code)], which in upcoming releases will no longer consider a derived Debug or Clone as usage, though they will consider a manually-implemented impl of those same traits as usage.

Reproducing

cargo +beta check

The fact that this crate doesn't build on nightly for this exact reason is the real obstacle for me, but the fact that the build will soon fail on stable is probably more relevant to you :p

I've just deleted the member on my end. I'd send a PR, but I also can't run your test suite, because I get this test failure:

✗ Unexpected IO Error: "tests/images/invalid/openexr/Damaged/asan_heap-oob_7f4d5072b39d_561_5f5e4ef49a581edaf7bf0858fbfcfdd1_exr", Os { code: 22, kind: InvalidInput, message: "Invalid argument" }
thread 'damaged' panicked at 'A damaged file was not handled correctly', tests/fuzz.rs:125:5

The responsible syscall appears to be

lseek(6, 216172782113789576, SEEK_SET) = -1 EINVAL (Invalid argument)

No idea what's going on there.

@saethlin saethlin added the bug Something isn't working label Nov 23, 2021
@johannesvollmer
Copy link
Owner

Thanks for the efforts. If you think the tests are only a problem on your local machine, you can open the pr anyways, because GitHub will run the tests for each pr.

@johannesvollmer
Copy link
Owner

johannesvollmer commented Nov 23, 2021

The error you saw is an IO Error. What OS are you running? Any kind of special hardware? Do you have the exr project in some special folder?

Were you able to pinpoint the source code location in the project where the error occurred?

@saethlin
Copy link
Author

Ah, took me a while to notice you've overridden the default dev/test profiles. I've deleted those so I can get useful stack traces. Maybe this helps?

[pid 2351494] lseek(5, 216172782113789576, SEEK_SET <unfinished ...>
[pid 2351494] <... lseek resumed>)      = -1 EINVAL (Invalid argument)
 > /usr/lib/libpthread-2.33.so(lseek64+0xb) [0x12d6b]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(<std::fs::File as std::io::Seek>::seek+0x19) [0x258b09]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(<std::io::buffered::bufreader::BufReader<R> as std::io::Seek>::seek+0xbd) [0xbc1dd]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::io::Tracking<T>::seek_read_to+0x2eb) [0x3502b]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::io::PeekRead<exr::io::Tracking<T>>::skip_to+0x21) [0x355a1]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(<exr::block::reader::FilteredChunksReader<R> as core::iter::traits::iterator::Iterator>::next::{{closure}}+0x85) [0x908a5]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(core::option::Option<T>::map+0x88) [0x101158]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(<exr::block::reader::FilteredChunksReader<R> as core::iter::traits::iterator::Iterator>::next+0x51) [0x907b1]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(<exr::block::reader::OnProgressChunksReader<R,F> as core::iter::traits::iterator::Iterator>::next+0x27) [0x90e27]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::block::reader::ParallelBlockDecompressor<R>::decompress_next_block+0x164) [0x9fda4]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(<exr::block::reader::ParallelBlockDecompressor<R> as core::iter::traits::iterator::Iterator>::next+0x16) [0x90c96]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::block::reader::ChunksReader::decompress_parallel+0x1a7) [0x93d67]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::image::read::image::ReadImage<F,L>::from_chunks+0x6a7) [0xc1237]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::image::read::image::ReadImage<F,L>::from_buffered+0x17b) [0xc502b]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::image::read::image::ReadImage<F,L>::from_unbuffered+0x75) [0xc5bd5]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(exr::image::read::image::ReadImage<F,L>::from_file+0xd6) [0xc65b6]
 > /home/ben/exrs/target/debug/deps/fuzz-41c5820066a9324e(fuzz::damaged::{{closure}}+0x1bf) [0xd991f]

I do not have a special OS or hardware, and the project isn't in a special folder. I'm running this on a totally normal Linux desktop. It looks like the tests do pass on my x86_64 Macbook, which is odd but wouldn't be the first time I've seen a substantial cross-platform issue between MacOS and Linux. There's nothing fishy going on with the hardware for sure, some basic debugging shows that enormous seek offset is indeed being passed to skip_to but only on Linux.

Since this is some I/O code, my instinct is that there's some case where the number of bytes read by a Read impl isn't being handled correctly. I'll take another crack at this tomorrow by sprinkling the codebase with read_exact calls.

@johannesvollmer
Copy link
Owner

johannesvollmer commented Nov 24, 2021

Okay I think I have an idea what might be going on. Could it be that your Linux is big endian?

The big endian code paths are not tested at all (I don't have any big endian system). I thought I added a panic with an appropriate message, but it seems like the errors appear much earlier. The optimal solution would be to run the GitHub tests on little endian and big endian machines. However, I don't think it's possible yet.

Could you do me a favor and run the tests from the following IO dependency of exrs?
https://github.com/johannesvollmer/lebe/

@johannesvollmer
Copy link
Owner

johannesvollmer commented Nov 25, 2021

While it is strange that this behavior differs depending on the platform, the file being an error is actually fine. It's one of the damaged/invalid files. It should not panic, but we expect it to return an Error. If possible, the error should be an Invalid Error, but an IO Error is also fine. Maybe the test doesn't tolerate the fact that an invalid file sometimes returns IO Error instead of Invalid Error.

Now that I think about it, I'm not even sure that it's always possible to differentiate an invalid file from a failed IO Operation.

@saethlin
Copy link
Author

Could it be that your Linux is big endian?

Definitely not. There's nothing remarkable about the CPU.

$ lscpu
Architecture:            x86_64
  CPU op-mode(s):        32-bit, 64-bit
  Address sizes:         43 bits physical, 48 bits virtual
  Byte Order:            Little Endian
CPU(s):                  64
  On-line CPU(s) list:   0-63
Vendor ID:               AuthenticAMD
  Model name:            AMD Ryzen Threadripper 3970X 32-Core Processor

My best guess at the moment is that there's some read call that should be a read_exact call. Wouldn't be the first time I've seen a significant behavior difference between APFS and ext4. I'll do some experimentation in a few days when I have more time.

@johannesvollmer
Copy link
Owner

johannesvollmer commented Nov 26, 2021

Sounds great - I really wouldn't know what to try next otherwise 😅

@johannesvollmer
Copy link
Owner

removed an unused field in #145, compiles with the newest rust version. does this solve one of the two issues you observed?

@saethlin
Copy link
Author

saethlin commented Dec 2, 2021

Yes, this fixes the build error.

@johannesvollmer
Copy link
Owner

Excellent. Let me know if you find something out about the failed test file :)

@johannesvollmer johannesvollmer changed the title Build failure on beta due to improved dead_code lint Suspiciously large seek offset results in failed test Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants