Argon2 format: Custom malloc/free using callbacks

[magnumripper]

We never free memory except in format's done(), instead we reuse the allocations per thread, and realloc if needed.

We also got rid of a small malloc/free per call for a temporary buffer, by moving away from argon2_hash() in favor of argon2_ctx().

Closes #5558

[magnumripper]

Not sure why they supply a size to deallocate() but I used it for an extra check, bc why not.

[solardiz]

Maybe can even insist on == size in that check?

[magnumripper]

No, it might be smaller after we reused the existing alloc for a smaller request.

[magnumripper]

Well that crashed and burned. I'll be trying to understand what ASan wants to explain to me.

Edit: OK that was a fencepost error for non-OMP. Thread was hard coded as 1 instead of 0.

[magnumripper]

@solardiz if this PR is correct, I can go ahead and switch to alloc_region_t() instead - either as a separate commit or as a fixup here.

[solardiz]

Looks good to me. I've made a couple of suggestions for a fixup of the existing commit.

Please do also switch to using the "regions" functions, but in a separate commit. Thank you!

[solardiz]

Maybe can even insist on == size in that check?

[solardiz]

Maybe use struct initialized with named fields here, which is a C feature that we already use in the tree anyway (and for a related purpose). Like this:

argon2_context context = {
        .out = (uint8_t*)crypted[i],
and so on
};

omitting the fields where we initialize to NULL or 0. That way, we'll be sure to initialize the entire struct even if more fields are added later.

Our existing usage is e.g.:

yescrypt_params_t params = {.N = cur_salt->N, .r = cur_salt->r, .p = cur_salt->p};

[solardiz]

The switch to regions looks good to me, except that our size field became redundant with what's inside the region struct - maybe switch to reusing the latter?

[solardiz]

e.g. yescrypt does:

                if (local->aligned_size < need) {
                        if (free_region(local))
                                return -1;
                        if (!alloc_region(local, need))
                                return -1;
                }

without needing to maintain a separate size field.

[solardiz]

and in the Armory format I did:

        lut_item *lut = memory->aligned;
        size_t bytes_reqd = (size_t)n * sizeof(*lut);
        if (!lut || memory->aligned_size < bytes_reqd) {
                free_region_t(memory);
                if (!(lut = alloc_region_t(memory, bytes_reqd)))
                        return -1;
        }

[magnumripper]

The switch to regions looks good to me, except that our size field became redundant with what's inside the region struct - maybe switch to reusing the latter?

Ah, yes. The fact we can hand out an oversized existing alloc had me confused but right now they are indeed always the same value.

I can throw in a commit resetting MKPC and OMP_SCALE to 1 as well, while at it.

[solardiz]

I can throw in a commit resetting MKPC and OMP_SCALE to 1 as well, while at it.

That's OK with me.

[solardiz]

We were ignoring possible error returns from Argon2 code before this PR and made it even worse with this PR. Memory allocation errors go undetected, resulting in "high speeds". I'm going to fix that.