hwrng: core - Fix wrong quality calculation at hw rng registration
Subsystems (2)
PATCH 1/1 hwrng: core - Fix wrong quality calculation at hw rng registration
When there are rng sources registering at the hwrng core via hwrng_register() a struct hwrng is delivered. There is a quality field in there which is used to decide which of the registered hw rng sources will be used by the hwrng core.
With commit 16bdbae39428 ("hwrng: core - treat default_quality as a maximum and default to 1024") there came in a new default of 1024 in case this field is empty and all the known hw rng sources at that time had been reworked to not fill this field and thus use the default of 1024.
The code choosing the 'better' hw rng source during registration of a new hw rng source has never been adapted to this and thus used 0 if the hw rng implementation does not fill the quality field. So when two rng sources register, one with 0 (meaning 1024) and the other one with 999, the 999 hw rng will be chosen.
This patch simple takes into account that a quality field value of 0 is to be treated as 1024 and then the decision about which hw rng to use works as expected.
Tested on s390 with two hardware rng sources: crypto cards and trng true random generator device driver.
--- drivers/char/hw_random/core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/char/hw_random/core.c +8 -1
@@ -525,6 +525,7 @@ static int hwrng_fillfn(void *unused)int hwrng_register (struct hwrng *rng) { +unsigned short rng_quality, cur_quality;int err = -EINVAL ;struct hwrng *tmp;
@@ -545,8 +546,14 @@ int hwrng_register(struct hwrng *rng)complete (&rng->cleanup_done);init_completion (&rng->dying); +/* Quality field not set in struct hwrng means 1024 */ + rng_quality = rng->quality ? rng->quality :1024 ; + cur_quality = current_rng ? + (current_rng->quality ? current_rng->quality :1024 ) : +0 ; +if (!current_rng || - (!cur_rng_set_by_user && rng->quality > current_rng->quality)) { + (!cur_rng_set_by_user && rng_quality > cur_quality)) {/* * Set new rng as current as the new rng source * provides better entropy quality and was not
Signature · 1 line
2.34.1
Herbert Xu
maintainerdrivers/char/hw_random/core.c int hwrng_register(struct hwrng *rng) · line 553
Quoted 10 lines · Harald Freudenberger
drivers/char/hw_random/core.c +5
-0@@ -545,8 +546,14 @@ int hwrng_register(struct hwrng *rng)complete (&rng->cleanup_done);init_completion (&rng->dying); +/* Quality field not set in struct hwrng means 1024 */ + rng_quality = rng->quality ? rng->quality :1024 ; + cur_quality = current_rng ? + (current_rng->quality ? current_rng->quality :1024 ) : +0 ;
The number 1024 is meant to be user configurable. We sould move the rng->quality initialisation from hwrng_init to hwrng_register.
Thanks,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 37 lines · Harald Freudenberger
When there are rng sources registering at the hwrng core via
hwrng_register()a struct hwrng is delivered. There is a quality field in there which is used to decide which of the registered hw rng sources will be used by the hwrng core.With commit
16bdbae39428("hwrng: core - treatdefault_qualityas a maximum and default to 1024") there came in a new default of 1024 in case this field is empty and all the known hw rng sources at that time had been reworked to not fill this field and thus use the default of 1024.The code choosing the 'better' hw rng source during registration of a new hw rng source has never been adapted to this and thus used 0 if the hw rng implementation does not fill the quality field. So when two rng sources register, one with 0 (meaning 1024) and the other one with 999, the 999 hw rng will be chosen.
This patch simple takes into account that a quality field value of 0 is to be treated as 1024 and then the decision about which hw rng to use works as expected.
Tested on s390 with two hardware rng sources: crypto cards and trng true random generator device driver.
Signed-off-by Harald Freudenberger <[email protected]>Reported-by Christian Rund <[email protected]>Fixes 16bdbae39428 ("hwrng: core - treat default_quality as a maximum and default to 1024")--- drivers/char/hw_random/core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/char/hw_random/core.c +0
-0
drivers/char/hw_random/core.c static int hwrng_fillfn(void *unused) · line 528
Quoted 4 lines
drivers/char/hw_random/core.c +1
-0@@ -525,6 +525,7 @@ static int hwrng_fillfn(void *unused)int hwrng_register (struct hwrng *rng) { +unsigned short rng_quality, cur_quality;
In my opinion, we no not need these variables.
drivers/char/hw_random/core.c int hwrng_register(struct hwrng *rng) · line 550
Quoted 9 lines
drivers/char/hw_random/core.c +2
-0int err = -EINVAL ;struct hwrng *tmp;@@ -545,8 +546,14 @@ int hwrng_register(struct hwrng *rng)complete (&rng->cleanup_done);init_completion (&rng->dying); +/* Quality field not set in struct hwrng means 1024 */ + rng_quality = rng->quality ? rng->quality :1024 ;
Please use the shortcut "(a) ?: (b)" for "(a) ? (a) : (b)", also remove non-necessary parenthesis.
rng_quality = rng->quality ?: 1024;
Because this variable is only used once, you can also change it directly in the if statement below.
drivers/char/hw_random/core.c int hwrng_register(struct hwrng *rng) · line 554
Quoted 4 lines
drivers/char/hw_random/core.c +4
-0+ cur_quality = current_rng ? + (current_rng->quality ? current_rng->quality :1024 ) : +0 ; +
This one is not necessary. The quality field of current_rng is has been updated already by the hwrng_init() function.
drivers/char/hw_random/core.c int hwrng_register(struct hwrng *rng) · line 556
Quoted 3 lines
drivers/char/hw_random/core.c +1
-1if (!current_rng || - (!cur_rng_set_by_user && rng->quality > current_rng->quality)) { + (!cur_rng_set_by_user && rng_quality > cur_quality)) {
Unfortunately, the quality field of rng is read here, before the quality field is updated by hwrng_init(). Maybe we can use the following:
if (!current_rng ||
(!cur_rng_set_by_user && (rng->quality ?: 1024) > current_rng->quality)) {Quoted 3 lines
drivers/char/hw_random/core.c +0
-0/* * Set new rng as current as the new rng source * provides better entropy quality and was not
Signature · 5 lines
Mit freundlichen Grüßen / Kind regards Holger Dengler -- IBM Systems, Linux on IBM Z Development [email protected]