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

Armory format: Pre-check for cracks within parallel section #5424

Merged
merged 1 commit into from
Jan 13, 2024

Conversation

solardiz
Copy link
Member

This is a controversial change for a slow format like this, but OTOH I don't want my decision not to make this format salt-only (where we'd have to check for cracks in crypt_all) to have any obvious performance cost, whereas the number of comparisons per crypt_all can be significant when there are many threads and SIMD vectors are wide (e.g., 192*8 in my recent test means 1536 comparisons, which when done sequentially out of crypt_all can have a measurable cost).

@solardiz
Copy link
Member Author

Weird, this segfaulted on ci/circleci: arm, whereas the previous revision passed tests there. I wonder if I introduced or exposed a bug here, or if it's something intermittent - e.g., maybe we simply don't always have the same amount of RAM available there?

@solardiz
Copy link
Member Author

I wonder if I introduced or exposed a bug here

Indeed, I did. Thesalt argument to crypt_all is allowed to be NULL, which I didn't handle. I could also reproduce the crash on Solaris/sparc32. Now I fixed that. However, I'm puzzled why this didn't show up on x86(-64) - is our self-test/benchmark on x86(-64) different, not ever setting salt = NULL there?

@solardiz solardiz merged commit c460f82 into openwall:bleeding-jumbo Jan 13, 2024
31 of 32 checks passed
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.

1 participant