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

Fuzzer and CI enhancements, format bug fixes #5415

Merged
merged 11 commits into from
Jan 5, 2024

Conversation

solardiz
Copy link
Member

@solardiz solardiz commented Jan 5, 2024

This adds support for --fuzz=LIMIT, where LIMIT is a decimal number specifying the maximum number of mangled ciphertexts to try per format. Since we had already supported an argument to that option to specify a "dictionary", I had to add a check for isdec and support both features (one at a time). This is a hack, but this whole feature is (and it is not compiled in by default), so it's "fine".

This also adds a number of GitHub Actions, which build with ASan and varying other options. One of them even runs --fuzz=500, which takes under a minute and fits 2 GB RAM. All of them use gcc 13. To reduce load on the CI infrastructure and have all of these jobs complete sooner, OMP_NUM_THREADS is now capped to 2, which is hopefully enough to have a good chance of catching multithreading-related bugs. Previously, the auto-detected number of threads on GitHub Actions would be 4, and tests were taking significantly longer (I guess those are logical CPUs in 2 cores).

Finally, the --disable-simd and --without-openssl ASan workers (under my personal GitHub account) identified some bugs in the formats, which are now fixed.

@solardiz
Copy link
Member Author

solardiz commented Jan 5, 2024

I added a 32-bit x86 target, which uses gcc -m32. It configures as:

config.status: linking x86-64.h to arch.h
config.status: executing default commands
configure: creating ./fmt_externs.h
configure: creating ./fmt_registers.h

Configured for building John the Ripper jumbo:

Target CPU ......................................... x86_64 SSE2, 32-bit LE
AES-NI support ..................................... no
Target OS .......................................... linux-gnu
Cross compiling .................................... no
Legacy arch header ................................. x86-64.h

so is quite weird. Somehow it ends up using the x86-64.h legacy arch header. Yet all tests pass. I wonder how that is even possible, since x86-64.h should have told our code that many things are 64-bit, which in this build they are not?

@solardiz solardiz merged commit 64819a2 into openwall:bleeding-jumbo Jan 5, 2024
29 of 32 checks passed
@claudioandre-br
Copy link
Member

Somehow it ends up using the x86-64.h legacy arch header.

It uses "values" from autoconfig.h (there is an #include), so it ends up working (in most cases, probably).
I personally would ignore all 32-bit, spare hardware and keep testing the 32-bit stuff using the current CircleCI Wine-32 build job.

Off the top of my head the team has 33 hours a month (per organization). Active projects must take care of usage.


BTW: it seems we can retire the CircleCI ASAN job (and use it for something else).

@solardiz
Copy link
Member Author

solardiz commented Jan 5, 2024

This adds support for --fuzz=LIMIT, where LIMIT is a decimal number specifying the maximum number of mangled ciphertexts to try per format.

FWIW, here's what I got with -fuzz=100000 -form=cpu (edit: in my local build with ASan):

All 740 formats passed fuzzing test!

real    200m44.170s
user    196m14.415s
sys     4m25.875s

This consumed a lot of memory (at least 70 GB, probably more), mostly for KeePass and LUKS.

@solardiz
Copy link
Member Author

solardiz commented Jan 7, 2024

BTW: it seems we can retire the CircleCI ASAN job (and use it for something else).

I think the CircleCI ASan job is with AVX-512, right? Those I added on GitHub Actions are currently at most AVX2. So they're different, and I'd rather keep both.

@claudioandre-br
Copy link
Member

BTW: it seems we can retire the CircleCI ASAN job (and use it for something else).

I think the CircleCI ASan job is with AVX-512, right? Those I added on GitHub Actions are currently at most AVX2. So they're different, and I'd rather keep both.

Removal reversed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants