Download raw body.
[PATCH] Fix memory leaks related to ML-KEM
On Tue, Dec 17, 2024 at 03:40:26PM +0900, Kenjiro Nakayama wrote:
> This patch resolves memory leaks and ensures proper resource cleanup:
Thanks
> - Call CBB_cleanup() to avoid leaks after initializing CBB.
These two are quite nasty. There are allocations in a void function.
So I added an XXX for error checking before the CBB_init_fixed().
Also, the _external_entropy() has become _external_seed() in more recent
versions of the upstream code. Did BoringSSL's switch to C++ also
involve some paradigm shift towards allocations can't fail?
> - Remove unnecessary "break" before MlkemKeygenFileTest in tests.
This seems to be a leftover from attempts at debugging an earlier
version. Also, a CBS holding a pointer to free is really not great.
Would be nice to fix this.
> - Add missing fclose() calls to release file resources in tests.
I took these, although this comes right before exit. It is weird to free
one thing but not clean up the other.
>
> Signed-off-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
> ---
> src/lib/libcrypto/mlkem/mlkem1024.c | 1 +
> src/lib/libcrypto/mlkem/mlkem768.c | 1 +
> src/regress/lib/libcrypto/mlkem/mlkem1024_keygen_tests.c | 2 +-
> src/regress/lib/libcrypto/mlkem/mlkem1024_nist_keygen_tests.c | 1 +
> src/regress/lib/libcrypto/mlkem/mlkem768_keygen_tests.c | 2 +-
> src/regress/lib/libcrypto/mlkem/mlkem768_nist_keygen_tests.c | 1 +
> 6 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git src/lib/libcrypto/mlkem/mlkem1024.c src/lib/libcrypto/mlkem/mlkem1024.c
> index e0a71f335..813d84f88 100644
> --- src/lib/libcrypto/mlkem/mlkem1024.c
> +++ src/lib/libcrypto/mlkem/mlkem1024.c
> @@ -878,6 +878,7 @@ MLKEM1024_generate_key_external_entropy(
> if (!mlkem_marshal_public_key(&cbb, &priv->pub)) {
> abort();
> }
> + CBB_cleanup(&cbb);
>
> hash_h(priv->pub.public_key_hash, out_encoded_public_key,
> MLKEM1024_PUBLIC_KEY_BYTES);
> diff --git src/lib/libcrypto/mlkem/mlkem768.c src/lib/libcrypto/mlkem/mlkem768.c
> index fed4704f8..7ab48fab6 100644
> --- src/lib/libcrypto/mlkem/mlkem768.c
> +++ src/lib/libcrypto/mlkem/mlkem768.c
> @@ -877,6 +877,7 @@ MLKEM768_generate_key_external_entropy(
> if (!mlkem_marshal_public_key(&cbb, &priv->pub)) {
> abort();
> }
> + CBB_cleanup(&cbb);
>
> hash_h(priv->pub.public_key_hash, out_encoded_public_key,
> MLKEM768_PUBLIC_KEY_BYTES);
> diff --git src/regress/lib/libcrypto/mlkem/mlkem1024_keygen_tests.c src/regress/lib/libcrypto/mlkem/mlkem1024_keygen_tests.c
> index d27d5aacf..e1a00c5b8 100644
> --- src/regress/lib/libcrypto/mlkem/mlkem1024_keygen_tests.c
> +++ src/regress/lib/libcrypto/mlkem/mlkem1024_keygen_tests.c
> @@ -112,7 +112,6 @@ main(int argc, char **argv)
> break;
> grab_data(&private_key, buf, strlen("private_key: "));
> state = S_START;
> - break;
>
> MlkemKeygenFileTest(&seed, &public_key, &private_key);
> free((void *)CBS_data(&seed));
> @@ -126,5 +125,6 @@ main(int argc, char **argv)
> }
>
> free(buf);
> + fclose(fp);
> exit(failure);
> }
> diff --git src/regress/lib/libcrypto/mlkem/mlkem1024_nist_keygen_tests.c src/regress/lib/libcrypto/mlkem/mlkem1024_nist_keygen_tests.c
> index b9600dc6b..effa1f734 100644
> --- src/regress/lib/libcrypto/mlkem/mlkem1024_nist_keygen_tests.c
> +++ src/regress/lib/libcrypto/mlkem/mlkem1024_nist_keygen_tests.c
> @@ -133,5 +133,6 @@ main(int argc, char **argv)
> }
>
> free(buf);
> + fclose(fp);
> exit(failure);
> }
> diff --git src/regress/lib/libcrypto/mlkem/mlkem768_keygen_tests.c src/regress/lib/libcrypto/mlkem/mlkem768_keygen_tests.c
> index b54b8c53c..f00d009fa 100644
> --- src/regress/lib/libcrypto/mlkem/mlkem768_keygen_tests.c
> +++ src/regress/lib/libcrypto/mlkem/mlkem768_keygen_tests.c
> @@ -112,7 +112,6 @@ main(int argc, char **argv)
> break;
> grab_data(&private_key, buf, strlen("private_key: "));
> state = S_START;
> - break;
>
> MlkemKeygenFileTest(&seed, &public_key, &private_key);
> free((void *)CBS_data(&seed));
> @@ -126,5 +125,6 @@ main(int argc, char **argv)
> }
>
> free(buf);
> + fclose(fp);
> exit(failure);
> }
> diff --git src/regress/lib/libcrypto/mlkem/mlkem768_nist_keygen_tests.c src/regress/lib/libcrypto/mlkem/mlkem768_nist_keygen_tests.c
> index ee71aa262..8b87afefc 100644
> --- src/regress/lib/libcrypto/mlkem/mlkem768_nist_keygen_tests.c
> +++ src/regress/lib/libcrypto/mlkem/mlkem768_nist_keygen_tests.c
> @@ -133,5 +133,6 @@ main(int argc, char **argv)
> }
>
> free(buf);
> + fclose(fp);
> exit(failure);
> }
> --
> 2.39.5 (Apple Git-154)
>
[PATCH] Fix memory leaks related to ML-KEM