From: Markus Theil theil.markus@gmail.com
[ Upstream commit 735b72568c73875269a6b73ab9543a70f6ac8a9f ]
The intermediary value was included in the wrong hash state. While there, adapt to user-space by setting the timestamp to 0 if stuck and inserting the values nevertheless.
Acked-by: Stephan Mueller smueller@chronox.de Signed-off-by: Markus Theil theil.markus@gmail.com Signed-off-by: Herbert Xu herbert@gondor.apana.org.au Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Backport Status: **YES**
This commit should be backported to stable kernel trees. Here's the detailed analysis:
### Bug Analysis
1. **Critical Bug Fix**: The commit fixes a bug where the intermediary hash value was being updated to the wrong hash state descriptor. Looking at line 147 in the original code (before the fix): ```c ret = crypto_shash_update(desc, intermediary, sizeof(intermediary)); ``` This was updating the temporary `desc` instead of `hash_state_desc`. The fix correctly changes this to: ```c ret = crypto_shash_update(hash_state_desc, intermediary, sizeof(intermediary)); ```
2. **Security Impact**: This is in the jitterentropy random number generator, which is a critical security component used for entropy collection in the kernel's crypto subsystem. Using the wrong hash descriptor means the intermediary values weren't being properly mixed into the entropy pool, potentially reducing the quality of randomness.
3. **SP800-90B Compliance**: The second part of the fix addresses SP800-90B compliance (NIST standard for entropy sources). The original code would skip inserting the timestamp when stuck: ```c if (!stuck) { ret = crypto_shash_update(hash_state_desc, (u8 *)&time, sizeof(__u64)); } ``` The fix changes this to always insert a value (0 when stuck) to maintain consistent input data size as required by SP800-90B section 3.1.5: ```c if (stuck) { time = 0; } ret = crypto_shash_update(hash_state_desc, (u8 *)&time, sizeof(__u64)); ```
### Backport Criteria Met
1. **Fixes a real bug**: Yes - incorrect hash state usage affecting entropy pool quality 2. **Small and contained**: Yes - only 5 lines changed in a single function 3. **No architectural changes**: Yes - simple logic fix, no API changes 4. **Critical subsystem**: Yes - affects kernel crypto/RNG subsystem 5. **Low regression risk**: Yes - straightforward fix with clear correctness 6. **Security relevance**: Yes - fixes entropy generation which is security-critical 7. **Standards compliance**: Yes - fixes SP800-90B compliance issue
The bug could potentially lead to weaker random number generation, which is a serious security concern. The fix is minimal, clearly correct, and has been acked by the subsystem maintainer (Stephan Mueller). This makes it an excellent candidate for stable backporting.
crypto/jitterentropy-kcapi.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c index c24d4ff2b4a8..1266eb790708 100644 --- a/crypto/jitterentropy-kcapi.c +++ b/crypto/jitterentropy-kcapi.c @@ -144,7 +144,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl, * Inject the data from the previous loop into the pool. This data is * not considered to contain any entropy, but it stirs the pool a bit. */ - ret = crypto_shash_update(desc, intermediary, sizeof(intermediary)); + ret = crypto_shash_update(hash_state_desc, intermediary, sizeof(intermediary)); if (ret) goto err;
@@ -157,11 +157,12 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl, * conditioning operation to have an identical amount of input data * according to section 3.1.5. */ - if (!stuck) { - ret = crypto_shash_update(hash_state_desc, (u8 *)&time, - sizeof(__u64)); + if (stuck) { + time = 0; }
+ ret = crypto_shash_update(hash_state_desc, (u8 *)&time, sizeof(__u64)); + err: shash_desc_zero(desc); memzero_explicit(intermediary, sizeof(intermediary));