Collin Funk wrote:
> 256 bytes seems like enough for the foreseeable future.
I agree: that's more than 3x the 72 bytes that it currently needs.
> I think it is okay to leave the abort () calls for EVP_DigestFinal_ex
> and EVP_DigestUpdate which seem like they should never fail outside of
> programmer error. For example, using a NULL pointer argument or calling
> them without calling EVP_DigestInit_ex. I rather abort () there than
> have a program print uninitialized memory or something like that.
Agreed. These are the situations for which abort() is adequate.
> What do you think of the attached patch?
Looks essentially right, except that the casts
(EVP_MD_CTX *) &ctx->evp_ctx_buffer
open a possibility of alignment problems (ISO C 23 § 6.3.2.3.(7)).
You can't work around it by using a
union { uint8_t evp_ctx_buffer[256]; EVP_MD_CTX evp_ctx; }
since the latter is an incomplete type. Therefore IMO the best solution
is to use max_align_t from <stddef.h>:
max_align_t evp_ctx_buffer[256 / sizeof (max_align_t)];
Also, why write
(EVP_MD_CTX *) &ctx->evp_ctx_buffer
instead of
(EVP_MD_CTX *) ctx->evp_ctx_buffer
since it's an array anyway?
Finally, if sizeof (EVP_MD_CTX) some day gets increased > 256, we should be
able to detect it. I think it can be done through a separate unit test file,
that overrides the functions
malloc
calloc
realloc
(with dlsym RTLD_NEXT), in such a way that these overrides abort if the
size argument is > 256. This test should only be activated on __ELF__
systems, because on the other ones (macOS, Windows, etc.) you probably
can't rely on dlsym RTLD_NEXT.
Bruno
