[PATCH 0/3] lib/crypto: mldsa: Address some minor nits

[PATCH 0/3] lib/crypto: mldsa: Address some minor nits

[Eric Biggers]

A few pedantic fixes for issues found by LLMs.

Eric Biggers (3):
  lib/crypto: mldsa: Check pk_len before doing pointer arithmetic
  lib/crypto: mldsa: Explicitly check that 'alg' is a valid value
  lib/crypto: mldsa: Add __must_check to mldsa_verify()

 include/crypto/mldsa.h         |  6 ++++--
 lib/crypto/mldsa.c             | 15 +++++++++++----
 lib/crypto/tests/mldsa_kunit.c |  9 +++++++++
 3 files changed, 24 insertions(+), 6 deletions(-)


base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
-- 
2.56.0

[PATCH 1/3] lib/crypto: mldsa: Check pk_len before doing pointer arithmetic

[Eric Biggers]

Though common in the kernel and benign in practice (not even triggering
UBSAN warnings), taking an address past the end of an array is undefined
behavior according to the C specification.

Therefore, compute &pk[RHO_LEN] only after pk_len has been validated,
since pk_len could be less than RHO_LEN if an incorrectly sized public
key is provided.

(Also fix a typo that isn't worth its own commit.)

Fixes: 64edccea594c ("lib/crypto: Add ML-DSA verification support")
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 lib/crypto/mldsa.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/crypto/mldsa.c b/lib/crypto/mldsa.c
index c96fddc4e7dcf..68a1b432315b1 100644
--- a/lib/crypto/mldsa.c
+++ b/lib/crypto/mldsa.c
@@ -306,7 +306,7 @@ static const u8 *decode_t1_elem(struct mldsa_ring_elem *out,
  * Reference: FIPS 204 Algorithm 27, sigDecode.
  *
  * This also validates that the coefficients of z are in range, corresponding
- * the infinity norm check at the end of Algorithm 8, ML-DSA.Verify_internal.
+ * to the infinity norm check at the end of Algorithm 8, ML-DSA.Verify_internal.
  *
  * Finally, this also converts z to NTT form.
  */
@@ -533,7 +533,7 @@ int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,
 	/* For now this just does pure ML-DSA with an empty context string. */
 	static const u8 msg_prefix[2] = { /* dom_sep= */ 0, /* ctx_len= */ 0 };
 	const u8 *ctilde; /* The signer's commitment hash */
-	const u8 *t1_encoded = &pk[RHO_LEN]; /* Next encoded element of t_1 */
+	const u8 *t1_encoded; /* Next encoded element of t_1 */
 	u8 *h; /* The signer's hint vector, length k * N */
 	size_t w1_enc_len;
 
@@ -587,6 +587,7 @@ int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,
 	 * The computation is the same for each of the k rows.  Just do each row
 	 * before moving on to the next, resulting in only one loop over k.
 	 */
+	t1_encoded = &pk[RHO_LEN];
 	for (int i = 0; i < k; i++) {
 		/*
 		 * tmp = NTT(A) * NTT(z) * 2^-32
-- 
2.56.0

[PATCH 2/3] lib/crypto: mldsa: Explicitly check that 'alg' is a valid value

[Eric Biggers]

C doesn't guarantee at the language level that an enum actually has one
of the declared values.

For robustness, make mldsa_verify() handle invalid values of 'alg'.

No effect on current callers, who always pass a valid value for 'alg'.
(Except for the KUnit test case that this commit adds.)

Fixes: 64edccea594c ("lib/crypto: Add ML-DSA verification support")
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/crypto/mldsa.h         |  1 +
 lib/crypto/mldsa.c             | 10 ++++++++--
 lib/crypto/tests/mldsa_kunit.c |  9 +++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/crypto/mldsa.h b/include/crypto/mldsa.h
index 3ef2676787c9e..e0754879e7452 100644
--- a/include/crypto/mldsa.h
+++ b/include/crypto/mldsa.h
@@ -48,6 +48,7 @@ enum mldsa_alg {
  * Return:
  * * 0 if the signature is valid
  * * -EBADMSG if the signature and/or public key is malformed
+ * * -EINVAL if @alg is not a valid value
  * * -EKEYREJECTED if the signature is invalid but otherwise well-formed
  * * -ENOMEM if out of memory so the validity of the signature is unknown
  */
diff --git a/lib/crypto/mldsa.c b/lib/crypto/mldsa.c
index 68a1b432315b1..80e0e63b418a7 100644
--- a/lib/crypto/mldsa.c
+++ b/lib/crypto/mldsa.c
@@ -528,8 +528,8 @@ static size_t encode_w1(u8 out[MAX_W1_ENCODED_LEN],
 int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,
 		 const u8 *msg, size_t msg_len, const u8 *pk, size_t pk_len)
 {
-	const struct mldsa_parameter_set *params = &mldsa_parameter_sets[alg];
-	const int k = params->k, l = params->l;
+	const struct mldsa_parameter_set *params;
+	int k, l;
 	/* For now this just does pure ML-DSA with an empty context string. */
 	static const u8 msg_prefix[2] = { /* dom_sep= */ 0, /* ctx_len= */ 0 };
 	const u8 *ctilde; /* The signer's commitment hash */
@@ -537,6 +537,12 @@ int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,
 	u8 *h; /* The signer's hint vector, length k * N */
 	size_t w1_enc_len;
 
+	if (alg != MLDSA44 && alg != MLDSA65 && alg != MLDSA87)
+		return -EINVAL;
+	params = &mldsa_parameter_sets[alg];
+	k = params->k;
+	l = params->l;
+
 	/* Validate the public key and signature lengths. */
 	if (pk_len != params->pk_len || sig_len != params->sig_len)
 		return -EBADMSG;
diff --git a/lib/crypto/tests/mldsa_kunit.c b/lib/crypto/tests/mldsa_kunit.c
index ec64e10c97f25..d0ee704a36944 100644
--- a/lib/crypto/tests/mldsa_kunit.c
+++ b/lib/crypto/tests/mldsa_kunit.c
@@ -372,6 +372,14 @@ static void test_mldsa_use_hint(struct kunit *test)
 	}
 }
 
+static void test_mldsa_invalid_alg(struct kunit *test)
+{
+	KUNIT_ASSERT_EQ(test, -EINVAL,
+			mldsa_verify(MLDSA44 - 1, NULL, 0, NULL, 0, NULL, 0));
+	KUNIT_ASSERT_EQ(test, -EINVAL,
+			mldsa_verify(MLDSA87 + 1, NULL, 0, NULL, 0, NULL, 0));
+}
+
 static void benchmark_mldsa(struct kunit *test,
 			    const struct mldsa_testvector *tv)
 {
@@ -414,6 +422,7 @@ static struct kunit_case mldsa_kunit_cases[] = {
 	KUNIT_CASE(test_mldsa65),
 	KUNIT_CASE(test_mldsa87),
 	KUNIT_CASE(test_mldsa_use_hint),
+	KUNIT_CASE(test_mldsa_invalid_alg),
 	KUNIT_CASE(benchmark_mldsa44),
 	KUNIT_CASE(benchmark_mldsa65),
 	KUNIT_CASE(benchmark_mldsa87),
-- 
2.56.0

[PATCH 3/3] lib/crypto: mldsa: Add __must_check to mldsa_verify()

[Eric Biggers]

Signature verification status always must be checked.  Add __must_check,
for consistency with e.g. the AES-GCM and ChaCha20Poly1305 functions.

No effect on current callers, who always check the return value.

Fixes: 64edccea594c ("lib/crypto: Add ML-DSA verification support")
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/crypto/mldsa.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/crypto/mldsa.h b/include/crypto/mldsa.h
index e0754879e7452..b156f166a1413 100644
--- a/include/crypto/mldsa.h
+++ b/include/crypto/mldsa.h
@@ -52,8 +52,9 @@ enum mldsa_alg {
  * * -EKEYREJECTED if the signature is invalid but otherwise well-formed
  * * -ENOMEM if out of memory so the validity of the signature is unknown
  */
-int mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,
-		 const u8 *msg, size_t msg_len, const u8 *pk, size_t pk_len);
+int __must_check mldsa_verify(enum mldsa_alg alg, const u8 *sig, size_t sig_len,
+			      const u8 *msg, size_t msg_len, const u8 *pk,
+			      size_t pk_len);
 
 #if IS_ENABLED(CONFIG_CRYPTO_LIB_MLDSA_KUNIT_TEST)
 /* Internal function, exposed only for unit testing */
-- 
2.56.0

Re: [PATCH 0/3] lib/crypto: mldsa: Address some minor nits

[Ard Biesheuvel]

On Thu, 29 Oct 2026, at 06:45, Eric Biggers wrote:
> A few pedantic fixes for issues found by LLMs.
>
> Eric Biggers (3):
>   lib/crypto: mldsa: Check pk_len before doing pointer arithmetic
>   lib/crypto: mldsa: Explicitly check that 'alg' is a valid value
>   lib/crypto: mldsa: Add __must_check to mldsa_verify()
>
>  include/crypto/mldsa.h         |  6 ++++--
>  lib/crypto/mldsa.c             | 15 +++++++++++----
>  lib/crypto/tests/mldsa_kunit.c |  9 +++++++++
>  3 files changed, 24 insertions(+), 6 deletions(-)
>
>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>