Example does not find panicking input since nightly-2021-12-18

Open
#90 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
cpp, rust

Research direction

Start with example/src/main.rs and ci/script.sh, then reproduce with rustup nightly-2021-12-17 and nightly-2021-12-18. Read Rust PR 91838 and compare the generated behavior and assembly described in the issue. Done means identifying and fixing the compiler or LLVM-related behavior so the first example reliably finds the panicking input.

Written by the indexing model from the issue text.

Description

While taking a look at #89, I noticed that even with the proposed workaround, the ci script fails because the first example doesn't find the panicking input. I guess this is an issue caused by llvm, but I am filing it here because I am not sure.

You can use my fork to reproduce the issue. This was working with an old version of rustc, so I was able to bisect the error. 2021-12-17 is good, 2021-12-18 is bad. This is the failing example:

// example/src/main.rs
fuzz_target!(|data: &[u8]| {
    if data == b"banana!" {
        panic!("success!");
    }
});
Bad output
rustup override set nightly-2021-12-18
./ci/script.sh
# Fails to find panicking input
INFO: A corpus is not provided, starting from an empty corpus
#2	INITED ft: 15 corp: 1/1b exec/s: 0 rss: 33Mb
#418	NEW    ft: 24 corp: 2/8b lim: 8 exec/s: 0 rss: 34Mb L: 7/7 MS: 1 InsertRepeatedBytes-
#100000	DONE   ft: 24 corp: 2/8b lim: 994 exec/s: 0 rss: 42Mb
Done 100000 runs in 0 second(s)
Good output
rustup override set nightly-2021-12-17
./ci/script.sh
# Everything works as expected
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#2	INITED ft: 15 corp: 1/1b exec/s: 0 rss: 30Mb
#469	NEW    ft: 18 corp: 2/8b lim: 8 exec/s: 0 rss: 30Mb L: 7/7 MS: 2 InsertByte-InsertRepeatedBytes-
thread '<unnamed>' panicked at 'success!', example/src/main.rs:7:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
==5118== ERROR: libFuzzer: deadly signal
Cause

I bisected the cause to this PR: https://github.com/rust-lang/rust/pull/91838 which optimizes comparison of slice and array. But note that this is just a library change, so the actual bug was probably already present before. The PR changes the PartialEq implementation from this one, which forwards the implementation to slice-slice comparison:

    #[inline]
    fn eq(&self, other: &[B]) -> bool {
        self[..] == other[..]
    }

to this one, which uses array-array comparison:

    #[inline]
    fn eq(&self, other: &[B]) -> bool {
        let b: Result<&[B; N], _> = other.try_into();
        match b {
            Ok(b) => *self == *b,
            Err(_) => false,
        }
    }

So a quick workaround would be to force slice-slice comparison, like this:

if data == &b"banana!"[..] {
    panic!("success!");
}

And that does fix the issue. Another workaround is to use a longer string, which somehow prevents the optimization from happening:

if data == b"a very long banana!" {
    panic!("success!");
}

And this also fixes the issue. But obviously it would be better to fix this in the compiler, otherwise there will be many bugs that could have been found by fuzzing but they were not.

I also tried changing the compilation flags, but I did not find any combination that fixed the issue. Also I tried other versions such as the latest nightly 2022-02-28 and the issue is not fixed yet.

Assembly

I tried comparing the assembly code of the good version and the bad version, and I think the main difference is that in the bad version the banana! string is stored inside a register as the constant 0x21616e616e6162, while in the old version it must be stored somewhere else because I did not find that constant.

Also I see that most of the calls to cmp and test are preceded by a call to __sanitizer_cov_trace_const_cmp1, which I assume is the coverage code, while the cmp with 0x21616e616e6162 from the new version does not have such a call, as you can see in this snippet:

movzbl 0x7fff8000(%rax),%r13d
xor    %edi,%edi
mov    %r13d,%esi
call   0xf6950 <__sanitizer_cov_trace_const_cmp1>
test   %r13d,%r13d
jne    0xe0c80 <rust_fuzzer_test_input+3136>
addb   $0x1,0x845f2(%rip)        # 0x164f8a
addb   $0x1,0x845ef(%rip)        # 0x164f8e
movzwl 0x4(%r14),%eax
movzbl 0x6(%r14),%ecx
shl    $0x10,%ecx
or     %eax,%ecx
shl    $0x20,%rcx
mov    (%r14),%eax
or     %rcx,%rax
movabs $0x21616e616e6162,%rcx
cmp    %rcx,%rax
je     0xe0d52 <rust_fuzzer_test_input+3346>
addb   $0x1,0x845bd(%rip)        # 0x164f8f
jmp    0xe0a93 <rust_fuzzer_test_input+2643>
addb   $0x1,0x845a3(%rip)        # 0x164f81

Here is the full disassembly of the rust_fuzzer_test_input function, so you can compare the differences: fuzz_main_dissasembly.zip

Conclusion

I hope this report was useful, and if someone knows what is going on then let me know. If I have time I will try to check if the new slice comparison code has ever compiled correctly in older versions of rust, and also I may try to reproduce the bug in C++ and open an issue directly in the llvm repo.

Dominant language
C++
Stars
267
Forks
51
PR merge metrics
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rust-fuzz/libfuzzer

All issues in rust-fuzz/libfuzzer

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.