The generate function in struct rng_alg expects that the destination
buffer is completely filled if the function returns 0. qcom_rng_read()
can run into a situation where the buffer is partially filled with
randomness and the remaining part of the buffer is zeroed since
qcom_rng_generate() doesn't check the return value. This issue can
be reproduced by running the following from libkcapi:
kcapi-rng -b 9000000 > OUTFILE
The generated OUTFILE will have three huge sections that contain all
zeros, and this is caused by the code where the test
'val & PRNG_STATUS_DATA_AVAIL' fails.
Let's fix this issue by ensuring that qcom_rng_read() always returns
with a full buffer if the function returns success. Let's also have
qcom_rng_generate() return the correct value.
Here's some statistics from the ent project
(https://www.fourmilab.ch/random/) that shows information about the
quality of the generated numbers:
$ ent -c qcom-random-before
Value Char Occurrences Fraction
0 606748 0.067416
1 33104 0.003678
2 33001 0.003667
...
253 � 32883 0.003654
254 � 33035 0.003671
255 � 33239 0.003693
Total: 9000000 1.000000
Entropy = 7.811590 bits per byte.
Optimum compression would reduce the size
of this 9000000 byte file by 2 percent.
Chi square distribution for 9000000 samples is 9329962.81, and
randomly would exceed this value less than 0.01 percent of the
times.
Arithmetic mean value of data bytes is 119.3731 (127.5 = random).
Monte Carlo value for Pi is 3.197293333 (error 1.77 percent).
Serial correlation coefficient is 0.159130 (totally uncorrelated =
0.0).
Without this patch, the results of the chi-square test is 0.01%, and
the numbers are certainly not random according to ent's project page.
The results improve with this patch:
$ ent -c qcom-random-after
Value Char Occurrences Fraction
0 35432 0.003937
1 35127 0.003903
2 35424 0.003936
...
253 � 35201 0.003911
254 � 34835 0.003871
255 � 35368 0.003930
Total: 9000000 1.000000
Entropy = 7.999979 bits per byte.
Optimum compression would reduce the size
of this 9000000 byte file by 0 percent.
Chi square distribution for 9000000 samples is 258.77, and randomly
would exceed this value 42.24 percent of the times.
Arithmetic mean value of data bytes is 127.5006 (127.5 = random).
Monte Carlo value for Pi is 3.141277333 (error 0.01 percent).
Serial correlation coefficient is 0.000468 (totally uncorrelated =
0.0).
This change was tested on a Nexus 5 phone (msm8974 SoC).
Signed-off-by: Brian Masney <[email protected]>
Fixes: ceec5f5b5988 ("crypto: qcom-rng - Add Qcom prng driver")
Cc: [email protected] # 4.19+
---
drivers/crypto/qcom-rng.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index 99ba8d51d102..11f30fd48c14 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -8,6 +8,7 @@
#include <linux/clk.h>
#include <linux/crypto.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -43,16 +44,19 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
{
unsigned int currsize = 0;
u32 val;
+ int ret;
/* read random data from hardware */
do {
- val = readl_relaxed(rng->base + PRNG_STATUS);
- if (!(val & PRNG_STATUS_DATA_AVAIL))
- break;
+ ret = readl_poll_timeout(rng->base + PRNG_STATUS, val,
+ val & PRNG_STATUS_DATA_AVAIL,
+ 200, 10000);
+ if (ret)
+ return ret;
val = readl_relaxed(rng->base + PRNG_DATA_OUT);
if (!val)
- break;
+ return -EINVAL;
if ((max - currsize) >= WORD_SZ) {
memcpy(data, &val, WORD_SZ);
@@ -61,11 +65,10 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
} else {
/* copy only remaining bytes */
memcpy(data, &val, max - currsize);
- break;
}
} while (currsize < max);
- return currsize;
+ return 0;
}
static int qcom_rng_generate(struct crypto_rng *tfm,
@@ -87,7 +90,7 @@ static int qcom_rng_generate(struct crypto_rng *tfm,
mutex_unlock(&rng->lock);
clk_disable_unprepare(rng->clk);
- return 0;
+ return ret;
}
static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
--
2.34.1
On Thu 10 Mar 15:24 PST 2022, Brian Masney wrote:
> The generate function in struct rng_alg expects that the destination
> buffer is completely filled if the function returns 0. qcom_rng_read()
> can run into a situation where the buffer is partially filled with
> randomness and the remaining part of the buffer is zeroed since
> qcom_rng_generate() doesn't check the return value. This issue can
> be reproduced by running the following from libkcapi:
>
> kcapi-rng -b 9000000 > OUTFILE
>
> The generated OUTFILE will have three huge sections that contain all
> zeros, and this is caused by the code where the test
> 'val & PRNG_STATUS_DATA_AVAIL' fails.
>
> Let's fix this issue by ensuring that qcom_rng_read() always returns
> with a full buffer if the function returns success. Let's also have
> qcom_rng_generate() return the correct value.
>
> Here's some statistics from the ent project
> (https://www.fourmilab.ch/random/) that shows information about the
> quality of the generated numbers:
>
> $ ent -c qcom-random-before
> Value Char Occurrences Fraction
> 0 606748 0.067416
> 1 33104 0.003678
> 2 33001 0.003667
> ...
> 253 ??? 32883 0.003654
> 254 ??? 33035 0.003671
> 255 ??? 33239 0.003693
>
> Total: 9000000 1.000000
>
> Entropy = 7.811590 bits per byte.
>
> Optimum compression would reduce the size
> of this 9000000 byte file by 2 percent.
>
> Chi square distribution for 9000000 samples is 9329962.81, and
> randomly would exceed this value less than 0.01 percent of the
> times.
>
> Arithmetic mean value of data bytes is 119.3731 (127.5 = random).
> Monte Carlo value for Pi is 3.197293333 (error 1.77 percent).
> Serial correlation coefficient is 0.159130 (totally uncorrelated =
> 0.0).
>
> Without this patch, the results of the chi-square test is 0.01%, and
> the numbers are certainly not random according to ent's project page.
> The results improve with this patch:
>
> $ ent -c qcom-random-after
> Value Char Occurrences Fraction
> 0 35432 0.003937
> 1 35127 0.003903
> 2 35424 0.003936
> ...
> 253 ??? 35201 0.003911
> 254 ??? 34835 0.003871
> 255 ??? 35368 0.003930
>
> Total: 9000000 1.000000
>
> Entropy = 7.999979 bits per byte.
>
> Optimum compression would reduce the size
> of this 9000000 byte file by 0 percent.
>
> Chi square distribution for 9000000 samples is 258.77, and randomly
> would exceed this value 42.24 percent of the times.
>
> Arithmetic mean value of data bytes is 127.5006 (127.5 = random).
> Monte Carlo value for Pi is 3.141277333 (error 0.01 percent).
> Serial correlation coefficient is 0.000468 (totally uncorrelated =
> 0.0).
>
> This change was tested on a Nexus 5 phone (msm8974 SoC).
>
> Signed-off-by: Brian Masney <[email protected]>
> Fixes: ceec5f5b5988 ("crypto: qcom-rng - Add Qcom prng driver")
> Cc: [email protected] # 4.19+
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> ---
> drivers/crypto/qcom-rng.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
> index 99ba8d51d102..11f30fd48c14 100644
> --- a/drivers/crypto/qcom-rng.c
> +++ b/drivers/crypto/qcom-rng.c
> @@ -8,6 +8,7 @@
> #include <linux/clk.h>
> #include <linux/crypto.h>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -43,16 +44,19 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
> {
> unsigned int currsize = 0;
> u32 val;
> + int ret;
>
> /* read random data from hardware */
> do {
> - val = readl_relaxed(rng->base + PRNG_STATUS);
> - if (!(val & PRNG_STATUS_DATA_AVAIL))
> - break;
> + ret = readl_poll_timeout(rng->base + PRNG_STATUS, val,
> + val & PRNG_STATUS_DATA_AVAIL,
> + 200, 10000);
> + if (ret)
> + return ret;
>
> val = readl_relaxed(rng->base + PRNG_DATA_OUT);
> if (!val)
> - break;
> + return -EINVAL;
>
> if ((max - currsize) >= WORD_SZ) {
> memcpy(data, &val, WORD_SZ);
> @@ -61,11 +65,10 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
> } else {
> /* copy only remaining bytes */
> memcpy(data, &val, max - currsize);
> - break;
> }
> } while (currsize < max);
>
> - return currsize;
> + return 0;
> }
>
> static int qcom_rng_generate(struct crypto_rng *tfm,
> @@ -87,7 +90,7 @@ static int qcom_rng_generate(struct crypto_rng *tfm,
> mutex_unlock(&rng->lock);
> clk_disable_unprepare(rng->clk);
>
> - return 0;
> + return ret;
> }
>
> static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
> --
> 2.34.1
>