OpenCL SHA-512 "free" formats: Fix double race conditions #5840
Conversation
|
Looks like these specific formats have just one Edit: OTOH, clearing the flag just once per all |
Yes and that's also the reason for race conditions. Looking at it today I can't understand why it was made this way, but it was very early in our steps towards mastering GPU formats.
How would cmp_all know there aren't more matching binaries? |
It wouldn't, but it doesn't need to since it's called again for each loaded hash. That's when bitmap+hash table are not in use (for the current salt if applicable) - when they are, |
|
Oh, I see what you mean now. In my mind the cmp kernel was called from within crypt_all. The result should obviously be cleared prior to calling the cmp kernel, so in cmp_all. I'll fix that. |
Only gid 0 cleared the "result" buffer, then a global barrier. The barrier only synchronizes within a single workgroup so this is not safe. The only safe and sensible way of clearing it is on host side. Then multiple threads (potentially) wrote to the same "result" location without atomic operations. This is a data race and result is undefined in OpenCL. We have to use atomic operations.
cef404b to
adf48d1
Compare
|
BTW, why do you also add |
It's required for atomic functions. IIRC there's not always a warning if you forget, and it may even (intermittently) work just fine although perhaps unsafe so could possibly be a bug really hard to spot. |
Found when reviewing all uses of barriers in our kernels.
I recall we've had problems with intermittent failures from these two formats. The global barrier was added by me in good faith at some point in time. It did help a little but still not safe. Before that, there wasn't even the barrier...
I believe all other kernels' use of barriers are just fine.