* [PATCH] hw_random: Always drop the RNG in hwrng_unregister()
@ 2018-06-14 18:08 ` Michael Büsch
0 siblings, 0 replies; 6+ messages in thread
From: Michael Büsch @ 2018-06-14 18:08 UTC (permalink / raw)
To: Matt Mackall, Herbert Xu
Cc: Wirz, linux-crypto, b43-dev, linux-wireless,
PrasannaKumar Muralidharan, Harald Freudenberger
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
enable_best_rng() is used in hwrng_unregister() to switch away from the
currently active RNG, if that is the one currently being removed.
However enable_best_rng() might fail, if the next RNG's init routine
fails. In that case enable_best_rng() will return an error code and
the currently active RNG will remain active.
After unregistering this might lead to crashes due to use-after-free.
Fix this by dropping the currently active RNG, if enable_best_rng()
failed. This will result in no RNG to be active, if the next-best
one failed to initialize.
This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21
Reported-by: Wirz <spam@lukas-wirz.de>
Tested-by: Wirz <spam@lukas-wirz.de>
Signed-off-by: Michael Büsch <m@bues.ch>
Cc: stable@vger.kernel.org
---
See this discussion for a crash in b43's hwrng caused by this problem:
https://www.spinics.net/lists/linux-wireless/msg173089.html
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 91bb98c42a1c..aaf9e5afaad4 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -516,11 +516,18 @@ EXPORT_SYMBOL_GPL(hwrng_register);
void hwrng_unregister(struct hwrng *rng)
{
+ int err;
+
mutex_lock(&rng_mutex);
list_del(&rng->list);
- if (current_rng == rng)
- enable_best_rng();
+ if (current_rng == rng) {
+ err = enable_best_rng();
+ if (err) {
+ drop_current_rng();
+ cur_rng_set_by_user = 0;
+ }
+ }
if (list_empty(&rng_list)) {
mutex_unlock(&rng_mutex);
--
Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] hw_random: Always drop the RNG in hwrng_unregister()
@ 2018-06-14 18:08 ` Michael Büsch
0 siblings, 0 replies; 6+ messages in thread
From: Michael Büsch @ 2018-06-14 18:08 UTC (permalink / raw)
To: Matt Mackall, Herbert Xu
Cc: Wirz, linux-crypto-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-wireless,
PrasannaKumar Muralidharan, Harald Freudenberger
[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]
enable_best_rng() is used in hwrng_unregister() to switch away from the
currently active RNG, if that is the one currently being removed.
However enable_best_rng() might fail, if the next RNG's init routine
fails. In that case enable_best_rng() will return an error code and
the currently active RNG will remain active.
After unregistering this might lead to crashes due to use-after-free.
Fix this by dropping the currently active RNG, if enable_best_rng()
failed. This will result in no RNG to be active, if the next-best
one failed to initialize.
This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21
Reported-by: Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org>
Tested-by: Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org>
Signed-off-by: Michael Büsch <m@bues.ch>
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
See this discussion for a crash in b43's hwrng caused by this problem:
https://www.spinics.net/lists/linux-wireless/msg173089.html
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 91bb98c42a1c..aaf9e5afaad4 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -516,11 +516,18 @@ EXPORT_SYMBOL_GPL(hwrng_register);
void hwrng_unregister(struct hwrng *rng)
{
+ int err;
+
mutex_lock(&rng_mutex);
list_del(&rng->list);
- if (current_rng == rng)
- enable_best_rng();
+ if (current_rng == rng) {
+ err = enable_best_rng();
+ if (err) {
+ drop_current_rng();
+ cur_rng_set_by_user = 0;
+ }
+ }
if (list_empty(&rng_list)) {
mutex_unlock(&rng_mutex);
--
Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] hw_random: Always drop the RNG in hwrng_unregister()
2018-06-14 18:08 ` Michael Büsch
@ 2018-06-15 8:08 ` Harald Freudenberger
-1 siblings, 0 replies; 6+ messages in thread
From: Harald Freudenberger @ 2018-06-15 8:08 UTC (permalink / raw)
To: Michael Büsch, Matt Mackall, Herbert Xu
Cc: Wirz, linux-crypto, b43-dev, linux-wireless,
PrasannaKumar Muralidharan, Harald Freudenberger
On 14.06.2018 20:08, Michael Büsch wrote:
> enable_best_rng() is used in hwrng_unregister() to switch away from the
> currently active RNG, if that is the one currently being removed.
> However enable_best_rng() might fail, if the next RNG's init routine
> fails. In that case enable_best_rng() will return an error code and
> the currently active RNG will remain active.
> After unregistering this might lead to crashes due to use-after-free.
>
> Fix this by dropping the currently active RNG, if enable_best_rng()
> failed. This will result in no RNG to be active, if the next-best
> one failed to initialize.
>
> This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21
>
>
> Reported-by: Wirz <spam@lukas-wirz.de>
> Tested-by: Wirz <spam@lukas-wirz.de>
> Signed-off-by: Michael Büsch <m@bues.ch>
> Cc: stable@vger.kernel.org
>
> ---
>
> See this discussion for a crash in b43's hwrng caused by this problem:
> https://www.spinics.net/lists/linux-wireless/msg173089.html
>
>
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 91bb98c42a1c..aaf9e5afaad4 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -516,11 +516,18 @@ EXPORT_SYMBOL_GPL(hwrng_register);
>
> void hwrng_unregister(struct hwrng *rng)
> {
> + int err;
> +
> mutex_lock(&rng_mutex);
>
> list_del(&rng->list);
> - if (current_rng == rng)
> - enable_best_rng();
> + if (current_rng == rng) {
> + err = enable_best_rng();
> + if (err) {
> + drop_current_rng();
> + cur_rng_set_by_user = 0;
> + }
> + }
>
> if (list_empty(&rng_list)) {
> mutex_unlock(&rng_mutex);
>
>
>
Yes, if the hw_init() of the newly chosen rng fails, enable_best_rng() comes
back with an error and did not drop the current rng. The patch handles
this case. I did not test it, but the code looks fine.
reviewed-by Harald Freudenberger <freude@linux.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw_random: Always drop the RNG in hwrng_unregister()
@ 2018-06-15 8:08 ` Harald Freudenberger
0 siblings, 0 replies; 6+ messages in thread
From: Harald Freudenberger @ 2018-06-15 8:08 UTC (permalink / raw)
To: Michael Büsch, Matt Mackall, Herbert Xu
Cc: Wirz, linux-crypto-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-wireless,
PrasannaKumar Muralidharan, Harald Freudenberger
On 14.06.2018 20:08, Michael Büsch wrote:
> enable_best_rng() is used in hwrng_unregister() to switch away from the
> currently active RNG, if that is the one currently being removed.
> However enable_best_rng() might fail, if the next RNG's init routine
> fails. In that case enable_best_rng() will return an error code and
> the currently active RNG will remain active.
> After unregistering this might lead to crashes due to use-after-free.
>
> Fix this by dropping the currently active RNG, if enable_best_rng()
> failed. This will result in no RNG to be active, if the next-best
> one failed to initialize.
>
> This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21
>
>
> Reported-by: Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org>
> Tested-by: Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org>
> Signed-off-by: Michael Büsch <m@bues.ch>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>
> ---
>
> See this discussion for a crash in b43's hwrng caused by this problem:
> https://www.spinics.net/lists/linux-wireless/msg173089.html
>
>
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 91bb98c42a1c..aaf9e5afaad4 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -516,11 +516,18 @@ EXPORT_SYMBOL_GPL(hwrng_register);
>
> void hwrng_unregister(struct hwrng *rng)
> {
> + int err;
> +
> mutex_lock(&rng_mutex);
>
> list_del(&rng->list);
> - if (current_rng == rng)
> - enable_best_rng();
> + if (current_rng == rng) {
> + err = enable_best_rng();
> + if (err) {
> + drop_current_rng();
> + cur_rng_set_by_user = 0;
> + }
> + }
>
> if (list_empty(&rng_list)) {
> mutex_unlock(&rng_mutex);
>
>
>
Yes, if the hw_init() of the newly chosen rng fails, enable_best_rng() comes
back with an error and did not drop the current rng. The patch handles
this case. I did not test it, but the code looks fine.
reviewed-by Harald Freudenberger <freude-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw_random: Always drop the RNG in hwrng_unregister()
2018-06-14 18:08 ` Michael Büsch
@ 2018-06-15 15:15 ` Herbert Xu
-1 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2018-06-15 15:15 UTC (permalink / raw)
To: Michael Büsch
Cc: mpm, spam, linux-crypto, b43-dev, linux-wireless,
prasannatsmkumar, freude
Michael Büsch <m@bues.ch> wrote:
> [-- text/plain, encoding quoted-printable, charset: UTF-8, 57 lines --]
>
> enable_best_rng() is used in hwrng_unregister() to switch away from the
> currently active RNG, if that is the one currently being removed.
> However enable_best_rng() might fail, if the next RNG's init routine
> fails. In that case enable_best_rng() will return an error code and
> the currently active RNG will remain active.
> After unregistering this might lead to crashes due to use-after-free.
>
> Fix this by dropping the currently active RNG, if enable_best_rng()
> failed. This will result in no RNG to be active, if the next-best
> one failed to initialize.
>
> This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21
>
>
> Reported-by: Wirz <spam@lukas-wirz.de>
> Tested-by: Wirz <spam@lukas-wirz.de>
> Signed-off-by: Michael Büsch <m@bues.ch>
> Cc: stable@vger.kernel.org
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw_random: Always drop the RNG in hwrng_unregister()
@ 2018-06-15 15:15 ` Herbert Xu
0 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2018-06-15 15:15 UTC (permalink / raw)
To: Michael Büsch
Cc: mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, spam-rxbgZ4vWfLhdz0/ABlLGQA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
prasannatsmkumar-Re5JQEeQqe8AvxtiuMwx3w,
freude-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
Michael Büsch <m@bues.ch> wrote:
> [-- text/plain, encoding quoted-printable, charset: UTF-8, 57 lines --]
>
> enable_best_rng() is used in hwrng_unregister() to switch away from the
> currently active RNG, if that is the one currently being removed.
> However enable_best_rng() might fail, if the next RNG's init routine
> fails. In that case enable_best_rng() will return an error code and
> the currently active RNG will remain active.
> After unregistering this might lead to crashes due to use-after-free.
>
> Fix this by dropping the currently active RNG, if enable_best_rng()
> failed. This will result in no RNG to be active, if the next-best
> one failed to initialize.
>
> This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21
>
>
> Reported-by: Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org>
> Tested-by: Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org>
> Signed-off-by: Michael Büsch <m@bues.ch>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Patch applied. Thanks.
--
Email: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-15 15:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-14 18:08 [PATCH] hw_random: Always drop the RNG in hwrng_unregister() Michael Büsch
2018-06-14 18:08 ` Michael Büsch
2018-06-15 8:08 ` Harald Freudenberger
2018-06-15 8:08 ` Harald Freudenberger
2018-06-15 15:15 ` Herbert Xu
2018-06-15 15:15 ` Herbert Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.