-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
SunMD5 format crashes on ARM #5296
Comments
Probably something unaligned. |
Variables were declared as char and cast to int, for no apparent reason. Hopefully closes openwall#5296 but I couldn't reproduce as my gear apparently happened to end up aligned.
Variables were declared as char and cast to int, for no apparent reason. Hopefully closes openwall#5296 but I couldn't reproduce as my gear apparently happened to end up aligned.
Variables were declared as char and cast to int, for no apparent reason. Hopefully closes openwall#5296 but I couldn't reproduce as my gear apparently happened to end up aligned.
Variables were declared as char and cast to int, for no apparent reason. Hopefully closes openwall#5296 but I couldn't reproduce as my gear apparently happened to end up aligned.
Variables were declared as char and cast to int, for no apparent reason. Hopefully closes openwall#5296 but I couldn't reproduce as my gear apparently happened to end up aligned.
I managed to reproduce now. The trick is to use eg. |
I don't quite understand this backtrace.
Not a single line of it refers to john, does it? |
OK, that lldb crash happens with any format - must be some totally unrelated problem with lldb itself (it and the gcc are the ones included in MacOS though). |
Without lldb working, my only clue is this:
I think I'll try installing a real gcc and gdb and see what happens. Edit: Apparently I already had gcc-12 and that's what built john. But Homebrew doesn't supply gdb for arm64. I'll try building with native clang "gcc" and see if anything changes. |
Unfortunately a build with native clang "gcc" never segfaults, so I can't reproduce. I even tried |
Variables were declared as char and cast to int, for no apparent reason. These changes were made while researching openwall#5296 but didn't help at all. The aliasing violations are real though, so committing anyway.
Variables were declared as char and cast to int, for no apparent reason. These changes were made while researching openwall#5296 but didn't help at all. The aliasing violations are real though, so committing anyway. Also tweak OpenMP pragma; Move a private variable inside the block.
Lots of comments in #5299 but here's a better place. I no longer think this is about alignment at all. Putting that debug print with correct brackets results in a never segfault nor fail (and never shows misalignment). And to recap, when compiled with GNU gcc it never crashes and we've also ruled out OpenSSL as Oh, and also my fixes for the aliasing violations doesn't make a difference - so it's not the compiler acting up on UB. |
The mentions of |
If we can detect the issue with ASan, then that's what we should be using to debug it - not later crashes in a non-ASan build. Can you show that report from ASan you're referring to? |
Note to self: I couldn't use Edit: It's the combo |
I can't reproduce any problem with ASan enabled. I tried the #5299 branch as well as current bleeding-jumbo, |
What did you refer to, @claudioandre-br? Can you post that ASan report? Thanks! |
Not many build logs left, I didn't find anything. When I have news I'll post. |
Variables were declared as char and cast to int, for no apparent reason. These changes were made while researching #5296 but didn't help at all. The aliasing violations are real though, so committing anyway. Also tweak OpenMP pragma; Move a private variable inside the block.
If you could get a core file after such crash it would probably help |
Ubuntu 22 LTS (easy to reproduce using a cloud ARM instance and a simple gcc version: 11.4.0
|
I guess for best debugging, this should go along with a corresponding |
Thinking of possible reasons why this issue would only show up on Arm, we could want to try setting |
If you enable ASAN, no errors are seen: A regular Should we add something?
|
The non-simd build works fine. If you remove the stack protection (to get another hint). You get a bus error.
And gdb is useless.
|
Maybe it is possible to reproduce using X86_64. (gdb) bt
#0 SIMDmd5body (_data=<optimized out>, out=0x555559309c00, reload_state=reload_state@entry=0x0, SSEi_flags=SSEi_flags@entry=0) at simd-intrinsics.c:408
#1 0x000055555571fd73 in crypt_all._omp_fn.0 () at sunmd5_fmt_plug.c:789
#2 0x00007ffff7969a16 in GOMP_parallel (fn=fn@entry=0x55555571e8c0 <crypt_all._omp_fn.0>, data=data@entry=0x7fffffffd9e0, num_threads=8, num_threads@entry=0, flags=flags@entry=0)
at ../../../src/libgomp/parallel.c:178
#3 0x000055555571dfe8 in crypt_all (pcount=<optimized out>, salt=<optimized out>) at sunmd5_fmt_plug.c:527
#4 0x000055555573a0fb in is_key_right (format=format@entry=0x5555559005a0 <fmt_sunmd5>, index=index@entry=19, binary=binary@entry=0x555556b66f24,
ciphertext=ciphertext@entry=0x5555558690f8 "$md5$rounds=904$Vc3VgyFx44iS8.Yu$Scf90iLWN6O6mT9TA06NK/", plaintext=plaintext@entry=0x55555587315f "test",
is_test_fmt_case=is_test_fmt_case@entry=0, dbsalt=0x5555592fd670) at formats.c:592
#5 0x000055555573d2b0 in is_key_right (is_test_fmt_case=0, dbsalt=0x5555592fd670, plaintext=0x55555587315f "test",
ciphertext=0x5555558690f8 "$md5$rounds=904$Vc3VgyFx44iS8.Yu$Scf90iLWN6O6mT9TA06NK/", binary=<optimized out>, index=19, format=0x5555559005a0 <fmt_sunmd5>) at formats.c:588
#6 fmt_self_test_body (full_lvl=-1, db=0x555558fdfcc0, salt_copy=<optimized out>, binary_copy=<optimized out>, format=0x5555559005a0 <fmt_sunmd5>) at formats.c:1295
#7 fmt_self_test (format=format@entry=0x5555559005a0 <fmt_sunmd5>, db=db@entry=0x555558fdfcc0) at formats.c:2045
#8 0x000055555572c87d in benchmark_all () at bench.c:876
#9 0x000055555574585a in john_run () at john.c:1672
#10 main (argc=<optimized out>, argv=<optimized out>) at john.c:2092 diff --git a/src/x86-64.h b/src/x86-64.h
index f77c44ac13..39eec60dca 100644
--- a/src/x86-64.h
+++ b/src/x86-64.h
@@ -214,7 +214,7 @@
#define SIMD_COEF_32 16
#define SIMD_COEF_64 8
#elif __AVX2__
-#define SIMD_COEF_32 8
+#define SIMD_COEF_32 4
#define SIMD_COEF_64 4
#elif __SSE2__
#define SIMD_COEF_32 4
@@ -258,7 +258,7 @@
#elif defined(__GNUC__) && (GCC_VERSION < 40600 || defined(__XOP__)) // 4.6.0
#define SIMD_PARA_MD5 2
#else
-#define SIMD_PARA_MD5 3
+#define SIMD_PARA_MD5 2
#endif
#endif
If that is correct, the format should not use SIMD at all, it is buggy. |
Thank you @claudioandre-br for these additional tests.
I find these comments confusing, as it's not clear what they're comparing against. Surely we did see the crash in CircleCI with ASan enabled, so why wouldn't we in some other(?) test with ASan (and in what way it differs)? Perhaps you do know what compiler flags were used in CircleCI? Perhaps you also know where you ran those other tests (so can mention that)?
That's wrong, things are not supposed to work with incorrect settings like that. To correctly obtain |
I'm not sure if I've done this test previously with ASAN enabled. I removed or edited some previous comments as "everything was" 100% remote and only sure a regular build fails on mac M2 [1]. Now, using SSH (so, I can rebuild the way I want, re-test and so son), I was unable to crash when enabling ASAN. Even forcing CPPFLAGS=sanitize=address (-O2 is used in these cases). So, I didn't receive anything from ASAN. [1]
|
SunMD5 format crashes on ARM (macOS and Linux) See #5296. Signed-off-by: Claudio André <claudioandre.br@gmail.com>
I can't reproduce this problem on an
|
Thank you @kholia. It's great that you're still around! I don't recall all detail of this issue (was the issue disappearing with ASan for us?), but if you have time you could also try an |
The last time I saw it failing was on Apr 7 at https://launchpadlibrarian.net/723587866/buildlog_snap_ubuntu_jammy_arm64_john-the-ripper_BUILDING.txt.gz Random. A second run managed to succeed.
|
This is macOS on ARM:
It's more about documenting, but maybe someone can reproduce the issue and find a misuse of 'some' API by the format.
Note: it is a
test-full
run.Everything else looks ok:
If fails (to me) when:
The text was updated successfully, but these errors were encountered: