Bruno Haible <[email protected]> writes:
> abort() means "here is a situation that the programmer has not foreseen";
> it is not the adequate action upon an out-of-memory situation.
> (We have had this distinction between abort() and xalloc_die() for
> over 25 years. It really is solid and good.)
>
> If you say "I rather keep the module LGPL", you must avoid the abort().
>
> Technically I see at least two ways to do so:
> * Change the API so that the responsibility for calling xalloc_die()
> is with the caller. Or
> * Allocate the ctx->evp_ctx not via malloc, but on the stack. You can
> use a conservative estimate. For instance, if EVP_MD_CTX_create ()
> would allocate 217 bytes via malloc(), you can allocate 512 bytes
> on the stack and be sufficiently certain that this be enough for
> all future evolutions of OpenSSL / EVP.
> (Other libraries use the same trick as well. For instance, FreeBSD
> has an mbstate_t that is 128 bytes large — fixed but sufficient for
> all practical purposes. Or GNU libiconv, which has a type
> 'iconv_allocation_t' that is opaque but large enough for stack
> allocations.)
I think the second option is better. Using this test program:
$ cat main.c
#include <openssl/evp.h>
int
main (void)
{
EVP_MD_CTX_create ();
return 0;
}
$ gcc main.c -lcrypto
$ valgrind ./a.out
[...]
==410900== HEAP SUMMARY:
==410900== in use at exit: 72 bytes in 1 blocks
==410900== total heap usage: 1 allocs, 0 frees, 72 bytes allocated
[...]
256 bytes seems like enough for the foreseeable future. What do you
think of the attached patch?
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.
Collin
>From fce2d3861561f3ca3e1c6237b25bbab65df29533 Mon Sep 17 00:00:00 2001
Message-ID: <fce2d3861561f3ca3e1c6237b25bbab65df29533.1756865932.git.collin.fu...@gmail.com>
From: Collin Funk <[email protected]>
Date: Tue, 2 Sep 2025 19:16:17 -0700
Subject: [PATCH] crypto/sha3-buffer: Don't abort on OOM.
* lib/sha3.h (sha3_ctx): Use a stack allocated buffer used to cast to
store an EVP_MD_CTX.
* lib/sha3.c (DEFINE_SHA3_INIT_CTX): Initialize the structure to zero.
Use the stack allocated buffer instead of calling EVP_MD_CTX_create.
(sha3_read_ctx): Just call sha3_finish_ctx similar to the other crypto
modules.
(sha3_finish_ctx): Remove call to EVP_MD_CTX_free. Call
EVP_DigestFinal_ex.
---
ChangeLog | 10 ++++++++++
lib/sha3.c | 25 +++++++++++++------------
lib/sha3.h | 6 +++---
3 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 0e489bd482..8cd13b61ed 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
2025-09-02 Collin Funk <[email protected]>
+ crypto/sha3-buffer: Don't abort on OOM.
+ * lib/sha3.h (sha3_ctx): Use a stack allocated buffer used to cast to
+ store an EVP_MD_CTX.
+ * lib/sha3.c (DEFINE_SHA3_INIT_CTX): Initialize the structure to zero.
+ Use the stack allocated buffer instead of calling EVP_MD_CTX_create.
+ (sha3_read_ctx): Just call sha3_finish_ctx similar to the other crypto
+ modules.
+ (sha3_finish_ctx): Remove call to EVP_MD_CTX_free. Call
+ EVP_DigestFinal_ex.
+
crypto/sha3-buffer: Add support for OpenSSL.
* lib/sha3.c (DEFINE_SHA3_INIT_CTX, sha3_read_ctx, sha3_finish_ctx)
(DEFINE_SHA3_BUFFER, sha3_process_bytes, sha3_process_block)
diff --git a/lib/sha3.c b/lib/sha3.c
index 5da85db5f2..aecfe25f3e 100644
--- a/lib/sha3.c
+++ b/lib/sha3.c
@@ -323,11 +323,11 @@ sha3_process_block (const void *buffer, size_t len, struct sha3_ctx *ctx)
void \
sha3_##SIZE##_init_ctx (struct sha3_ctx *ctx) \
{ \
- int rc; \
- ctx->evp_ctx = EVP_MD_CTX_create (); \
- if (ctx->evp_ctx == NULL) \
- abort (); \
- rc = EVP_DigestInit_ex (ctx->evp_ctx, EVP_sha3_##SIZE (), NULL); \
+ /* EVP_DigestInit_ex expects all bytes to be zero. */ \
+ memset (ctx, 0, sizeof *ctx); \
+ EVP_MD_CTX *evp_ctx = (EVP_MD_CTX *) &ctx->evp_ctx_buffer; \
+ int rc = EVP_DigestInit_ex (evp_ctx, EVP_sha3_##SIZE (), NULL); \
+ /* This should never fail. */ \
if (rc == 0) \
abort (); \
}
@@ -341,17 +341,17 @@ void *
sha3_read_ctx (const struct sha3_ctx *ctx, void *resbuf)
{
/* Assume any unprocessed bytes in ctx are not to be ignored. */
- int result = EVP_DigestFinal_ex (ctx->evp_ctx, resbuf, NULL);
- if (result == 0)
- abort ();
- return resbuf;
+ return sha3_finish_ctx ((struct sha3_ctx *) ctx, resbuf);
}
void *
sha3_finish_ctx (struct sha3_ctx *ctx, void *resbuf)
{
- void *result = sha3_read_ctx (ctx, resbuf);
- EVP_MD_CTX_free (ctx->evp_ctx);
+ EVP_MD_CTX *evp_ctx = (EVP_MD_CTX *) &ctx->evp_ctx_buffer;
+ /* This should never fail. */
+ int result = EVP_DigestFinal_ex (evp_ctx, resbuf, NULL);
+ if (result == 0)
+ abort ();
return resbuf;
}
@@ -373,7 +373,8 @@ DEFINE_SHA3_BUFFER (512)
void
sha3_process_bytes (const void *buffer, size_t len, struct sha3_ctx *ctx)
{
- int result = EVP_DigestUpdate (ctx->evp_ctx, buffer, len);
+ EVP_MD_CTX *evp_ctx = (EVP_MD_CTX *) &ctx->evp_ctx_buffer;
+ int result = EVP_DigestUpdate (evp_ctx, buffer, len);
if (result == 0)
abort ();
}
diff --git a/lib/sha3.h b/lib/sha3.h
index 7de1216fda..045ff3c49c 100644
--- a/lib/sha3.h
+++ b/lib/sha3.h
@@ -50,9 +50,9 @@ enum { SHA3_512_BLOCK_SIZE = 576 / 8 };
struct sha3_ctx
{
# if HAVE_OPENSSL_SHA3
- /* This is an incomplete type, so we can only place a pointer in the
- struct. */
- EVP_MD_CTX *evp_ctx;
+ /* EVP_MD_CTX is an incomplete type. EVP_MD_CTX_create allocates 72 bytes of
+ memory as of 2025-09-02. */
+ uint8_t evp_ctx_buffer[256];
# else
u64 state[25];
uint8_t buffer[144]; /* Up to BLOCKLEN in use. */
--
2.51.0