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

o5logon-opencl: Support Oracle 12 #5749

Open
wants to merge 2 commits into
base: bleeding-jumbo
Choose a base branch
from

Conversation

magnumripper
Copy link
Member

@magnumripper magnumripper commented Apr 4, 2025

Also implements internal mask, for a 3-4x boost (due to "packed" key buffer it was pretty good already).

Closes #5648

The "Mute stupid warnings" commit (#5456) will be dropped before merging (I could make a separate PR after adding a conf option for it). It strips the idiot warnings while retaining any relevant warnings (mutes the warning entirely if there were no relevant ones).

As of now, there's a bug in here. I have stared at the code for days, can't find it. Things behave weird - segfaults and such, on any device (including super's AMD that brings down the entire host). Debugging sometimes show super weird stuff that I can't draw any conclusions from.

Switching to the alternative (bitsliced) AES code sometimes move the problem around, or nearly hides it.

Running at LWS=1 sometimes helps a little or a lot. Everything points to a buffer overflow, except I can't find any, ASan can't either, and under ASan there's no nvidia device to be seen. A backtrace nearly always points to opaque OpenCL runtime stuff only.

The o5logon stuff should ideally have been two commits, one for separating shared code and one for internal mask. But it's all too tangled to be fixed in retrospect.

@magnumripper magnumripper marked this pull request as draft April 4, 2025 01:26
@magnumripper magnumripper force-pushed the o5logon-opencl-oracle12 branch from 88325f1 to bb37248 Compare April 4, 2025 01:33
@magnumripper magnumripper added the RFC / discussion Help or comments wanted label Apr 4, 2025
@magnumripper magnumripper force-pushed the o5logon-opencl-oracle12 branch 3 times, most recently from 5e8cc4d to bceeae0 Compare April 4, 2025 05:58
@magnumripper
Copy link
Member Author

magnumripper commented Apr 4, 2025

The "Mute stupid warnings" commit (#5456) will be dropped before merging (I could make a separate PR after adding a conf option for it). It strips the idiot warnings while retaining any relevant warnings (mutes the warning entirely if there were no relevant ones).

I added conf options. IMO it can now be kept in this PR.

# Mute buggy nvidia warnings about kernel overriding noinline
# attribute. Even with this set, they will show at "debug verbosity"
# as in --verbose:6.
MuteBogusNvidiaWarnings = Y

Also added these in same commit:

# Show runtime build warnings regardless of verbosity.
AlwaysShowBuildWarnings = N

# Add ptxas info (-cl-nv-verbose) for nvidia to build log
NvidiaShowPtxas = Y

Perhaps the ptax info should default to N - as developers we can enable it in john-local.conf.

@magnumripper magnumripper force-pushed the o5logon-opencl-oracle12 branch 4 times, most recently from 0e12f95 to add5c3c Compare April 4, 2025 08:38
This suppresses messages like "(): Warning: Function zip_final is a
kernel, so overriding noinline attribute. The function may be inlined
when called." unless verbosity is bumped.  A configuration is added,
"MuteBogusNvidiaWarnings = N", for not muting.

We take care to only mute the exact message (bar the function name).
Closes openwall#5456.

Two other configuration options added (but default to old behavior):

NvidiaShowPtxas = Y

AlwaysShowBuildWarnings = N
@magnumripper magnumripper force-pushed the o5logon-opencl-oracle12 branch from add5c3c to 3a5c55a Compare April 4, 2025 08:49
@magnumripper
Copy link
Member Author

Pro-tip: For POCL, you get much better crash info after adding -g to conf's GlobalBuildOpts.

@magnumripper magnumripper force-pushed the o5logon-opencl-oracle12 branch from 3a5c55a to ab4b0c1 Compare April 4, 2025 11:40
@magnumripper
Copy link
Member Author

As of now, there's a bug in here

I think I squashed it. Need to test more, and also check if other formats with that keybuffer packing method have the same problem, never surfaced.

Also implements internal mask, for a 3-4x boost.

Closes openwall#5648
@magnumripper magnumripper force-pushed the o5logon-opencl-oracle12 branch from ab4b0c1 to eeb7ba4 Compare April 4, 2025 12:11
@magnumripper magnumripper removed the RFC / discussion Help or comments wanted label Apr 4, 2025
@magnumripper magnumripper marked this pull request as ready for review April 4, 2025 12:22
@magnumripper magnumripper requested a review from solardiz April 4, 2025 12:22
@magnumripper
Copy link
Member Author

Need to test more, and also check if other formats with that keybuffer packing method have the same problem, never surfaced.

It seems fine now. Other formats may or may not have been affected (it should often be triggered in self tests, which apparently didn't happen) but I added safety checks in #5750 anyway.

@magnumripper
Copy link
Member Author

Pro-tip: For POCL, you get much better crash info after adding -g to conf's GlobalBuildOpts.

This breaks build on some other platforms though, such as nvidia, so need to be used selectively.

@claudioandre-br
Copy link
Member

claudioandre-br commented Apr 4, 2025

Pro-tip: For POCL, you get much better crash info after adding -g to conf's GlobalBuildOpts.

This breaks build on some other platforms though, such as nvidia, so need to be used selectively.

So I think we can add it automatically inside get_build_opts() if the verbosity is > XX (only if pocl and ...).

diff --git a/src/opencl_common.c b/src/opencl_common.c
index 87dea222b..004a64f7a 100644
--- a/src/opencl_common.c
+++ b/src/opencl_common.c
@@ -1136,8 +1136,11 @@ static char *get_build_opts(int sequential_id, const char *opts)
                        global_opts = OPENCLBUILDOPTIONS;
 
        snprintf(build_opts, LINE_BUFFER_SIZE,
-                "-I opencl %s %s%s%s%s%s%d %s%d %s -D_OPENCL_COMPILER %s",
+                "-I opencl %s %s%s%s%s%s%s%d %s%d %s -D_OPENCL_COMPILER %s",
                global_opts,
+                       options.verbosity >= VERB_LEGACY &&
+                       get_platform_vendor_id(get_platform_id(sequential_id)) ==
+                        PLATFORM_POCL ? "-g " : "",
                get_device_version(sequential_id) >= 200 ? "-cl-std=CL1.2 " : "",
 #ifdef __APPLE__
                "-D__OS_X__ ",

0001-OpenCL-add-g-for-POCL-to-get-better-crach-info.PATCH

@magnumripper
Copy link
Member Author

So I think we can add it automatically inside get_build_opts() if the verbosity is > XX (only if pocl and ...).

True, like for -v:6 (debug) and above. Theoretically we could also auto-detect support for it, autoconf style 🤣

@magnumripper
Copy link
Member Author

magnumripper commented Apr 4, 2025

It now fails self-test on Super's Vega, but at least it doesn't bring the entire machine down now 👀
Switched to the alternative AES, no difference. Running with -lws=1 -gws=1 (for clues) did not help.

The format worked fine before this PR. would have been nicer the other way around. Had 250927K c/s before, now (running with -skip) 330752K.

Tried several other little tricks to no avail. Giving up on that.

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.

Update o5logon-opencl to support Oracle 12 (the CPU format does already)
2 participants