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

Reply via email to