Skip to contentSkip to conversation

hwrng: core - reject unknown RNG names

PATCH In review Thorsten Blum 3 participants 4 messages Sep 24 – Oct 2

Subsystems (2)
Crypto API Hardware random number generator core
lore
Share & tools
lore

Thorsten Blum

author
#0 lore raw

PATCH 1/1 hwrng: core - reject unknown RNG names

In rng_current_store(), err is 0 after successfully locking rng_mutex, and if the requested RNG name cannot be found, the function reports success even though the current RNG was not changed.

Set err = -ENODEV before searching the RNG list to reject unknown names.

Fixes 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
Cc [email protected]
Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/char/hw_random/core.c | 1 +
 1 file changed, 1 insertion(+)
drivers/char/hw_random/core.c +1 -0
index 57eeefe90da9..1f9e31cdc7f0 100644
@@ -376,6 +376,7 @@ static ssize_t rng_current_store(struct device *dev,
 		cur_rng_set_by_user = 1;
 		drop_current_rng();
 	} else {
+		err = -ENODEV;
 		list_for_each_entry(rng, &rng_list, list) {
 			if (sysfs_streq(rng->name, buf)) {
 				err = set_current_rng(rng);
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Thorsten Blum

In rng_current_store(), err is 0 after successfully locking rng_mutex, and if the requested RNG name cannot be found, the function reports success even though the current RNG was not changed.

Set err = -ENODEV before searching the RNG list to reject unknown names.

Yeah, from the

Quoted 1 line
Fixes 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")

it looks like it was the intention, but the mutex_lock_interruptible() rewrites an error code. That's a good example on why I often tell people to decouple assignment from the definition and the very same issue might happen and would be hard to debug later on. Surprisingly this happened in the same patch...

Reviewed-by Andy Shevchenko <[email protected]>
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#2 lore raw
Quoted 9 lines · Andy Shevchenko
Quoted 5 lines · Thorsten Blum

In rng_current_store(), err is 0 after successfully locking rng_mutex, and if the requested RNG name cannot be found, the function reports success even though the current RNG was not changed.

Set err = -ENODEV before searching the RNG list to reject unknown names.

Yeah, from the

Quoted 8 lines
Quoted 1 line
Fixes 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")

it looks like it was the intention, but the mutex_lock_interruptible() rewrites an error code. That's a good example on why I often tell people to decouple assignment from the definition and the very same issue might happen and would be hard to debug later on. Surprisingly this happened in the same patch...

Reviewed-by Andy Shevchenko <[email protected]>

Also

Fixes cb028f1662a9 ("hwrng: core - remove redundant initialization of variable err")

as that one removed the indeed redundant assignment, but forget to fix the issue.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#3 lore raw
In reply to Thorsten Blum
Quoted 4 lines · Thorsten Blum

In rng_current_store(), err is 0 after successfully locking rng_mutex, and if the requested RNG name cannot be found, the function reports success even though the current RNG was not changed.

Quoted 8 lines

Set err = -ENODEV before searching the RNG list to reject unknown names.

Fixes 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
Cc [email protected]
Signed-off-by Thorsten Blum <[email protected]>
---
 drivers/char/hw_random/core.c | 1 +
 1 file changed, 1 insertion(+)

Patch applied. 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