From: Markus Theil <theil.markus@xxxxxxxxx>
[ 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@xxxxxxxxxx>
Signed-off-by: Markus Theil <theil.markus@xxxxxxxxx>
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
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));
--
2.39.5
