Commit 142a27f0a731 added support for a "best" RNG, and in doing so
introduced a hang from rmmod/modprobe -r when the last RNG on the list
was unloaded.
When the hwrng list is depleted, return the global variables to their
original state and decrement all references to the object.
Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
Signed-off-by: Gary R Hook <[email protected]>
---
Changes since v1: fix misspelled word in subject
drivers/char/hw_random/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 657b8770b6b9..91bb98c42a1c 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -306,6 +306,10 @@ static int enable_best_rng(void)
ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
if (!ret)
cur_rng_set_by_user = 0;
+ } else {
+ drop_current_rng();
+ cur_rng_set_by_user = 0;
+ ret = 0;
}
return ret;
Hi Gary,
Some minor comments below.
On 16 December 2017 at 01:25, Gary R Hook <[email protected]> wrote:
>
> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
> introduced a hang from rmmod/modprobe -r when the last RNG on the list
> was unloaded.
Nice catch. Thanks for fixing this.
> When the hwrng list is depleted, return the global variables to their
> original state and decrement all references to the object.
>
> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
Please cc the commit author (in this case its me) so that this patch
gets noticed easily.
> Signed-off-by: Gary R Hook <[email protected]>
> ---
>
> Changes since v1: fix misspelled word in subject
>
> drivers/char/hw_random/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 657b8770b6b9..91bb98c42a1c 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
> ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
> if (!ret)
> cur_rng_set_by_user = 0;
> + } else {
> + drop_current_rng();
When the hwrng list is empty just set current_rng = NULL instead of
calling drop_current_rng().
> + cur_rng_set_by_user = 0;
> + ret = 0;
> }
>
> return ret;
>
Regards,
PrasannaKumar
On 17 December 2017 at 14:53, PrasannaKumar Muralidharan
<[email protected]> wrote:
> Hi Gary,
>
> Some minor comments below.
>
> On 16 December 2017 at 01:25, Gary R Hook <[email protected]> wrote:
>>
>> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
>> introduced a hang from rmmod/modprobe -r when the last RNG on the list
>> was unloaded.
>
> Nice catch. Thanks for fixing this.
>
>> When the hwrng list is depleted, return the global variables to their
>> original state and decrement all references to the object.
>>
>> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
>
> Please cc the commit author (in this case its me) so that this patch
> gets noticed easily.
>
>> Signed-off-by: Gary R Hook <[email protected]>
>> ---
>>
>> Changes since v1: fix misspelled word in subject
>>
>> drivers/char/hw_random/core.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>> index 657b8770b6b9..91bb98c42a1c 100644
>> --- a/drivers/char/hw_random/core.c
>> +++ b/drivers/char/hw_random/core.c
>> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
>> ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>> if (!ret)
>> cur_rng_set_by_user = 0;
>> + } else {
>> + drop_current_rng();
>
> When the hwrng list is empty just set current_rng = NULL instead of
> calling drop_current_rng().
>
>> + cur_rng_set_by_user = 0;
>> + ret = 0;
>> }
>>
>> return ret;
>>
>
> Regards,
> PrasannaKumar
I am fine with the code as is.
Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
Regards,
PrasannaKumar
On 12/17/2017 03:49 AM, PrasannaKumar Muralidharan wrote:
> On 17 December 2017 at 14:53, PrasannaKumar Muralidharan
> <[email protected]> wrote:
>> Hi Gary,
>>
>> Some minor comments below.
>>
>> On 16 December 2017 at 01:25, Gary R Hook <[email protected]> wrote:
>>>
>>> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
>>> introduced a hang from rmmod/modprobe -r when the last RNG on the list
>>> was unloaded.
>>
>> Nice catch. Thanks for fixing this.
>>
>>> When the hwrng list is depleted, return the global variables to their
>>> original state and decrement all references to the object.
>>>
>>> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
>>
>> Please cc the commit author (in this case its me) so that this patch
>> gets noticed easily.
D'oh! I did not do so on this version. My apologies.
>>
>>> Signed-off-by: Gary R Hook <[email protected]>
>>> ---
>>>
>>> Changes since v1: fix misspelled word in subject
>>>
>>> drivers/char/hw_random/core.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>>> index 657b8770b6b9..91bb98c42a1c 100644
>>> --- a/drivers/char/hw_random/core.c
>>> +++ b/drivers/char/hw_random/core.c
>>> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
>>> ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>>> if (!ret)
>>> cur_rng_set_by_user = 0;
>>> + } else {
>>> + drop_current_rng();
>>
>> When the hwrng list is empty just set current_rng = NULL instead of
>> calling drop_current_rng().
>>
>>> + cur_rng_set_by_user = 0;
>>> + ret = 0;
>>> }
>>>
>>> return ret;
>>>
>>
>> Regards,
>> PrasannaKumar
>
> I am fine with the code as is.
>
> Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
>
> Regards,
> PrasannaKumar
>
On Fri, Dec 15, 2017 at 01:55:59PM -0600, Gary R Hook wrote:
> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
> introduced a hang from rmmod/modprobe -r when the last RNG on the list
> was unloaded.
>
> When the hwrng list is depleted, return the global variables to their
> original state and decrement all references to the object.
>
> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
> Signed-off-by: Gary R Hook <[email protected]>
Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 12/15/2017 01:55 PM, Gary R Hook wrote:
> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
> introduced a hang from rmmod/modprobe -r when the last RNG on the list
> was unloaded.
>
> When the hwrng list is depleted, return the global variables to their
> original state and decrement all references to the object.
*ping*
It may not have been obvious from the title but this fixes a bug which
will impact the use of any HW RNG that is the only RNG registered. The
breakage of rmmod/modprobe -r that this fix obviates suggests that it
needs to make the 4.15 kernel, please. Just want to ensure this gets the
proper attention. Thank you.
>
> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
> Signed-off-by: Gary R Hook <[email protected]>
> ---
>
> Changes since v1: fix misspelled word in subject
>
> drivers/char/hw_random/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 657b8770b6b9..91bb98c42a1c 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
> ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
> if (!ret)
> cur_rng_set_by_user = 0;
> + } else {
> + drop_current_rng();
> + cur_rng_set_by_user = 0;
> + ret = 0;
> }
>
> return ret;
>
On Fri, Jan 05, 2018 at 11:28:23AM -0600, Gary R Hook wrote:
>
> It may not have been obvious from the title but this fixes a bug
> which will impact the use of any HW RNG that is the only RNG
> registered. The breakage of rmmod/modprobe -r that this fix obviates
> suggests that it needs to make the 4.15 kernel, please. Just want to
> ensure this gets the proper attention. Thank you.
I don't think breaking rmmod is sufficiently serious to warrant
immediate inclusion at this point. We can always send it via
stable after the release.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 01/07/2018 11:05 PM, Herbert Xu wrote:
> On Fri, Jan 05, 2018 at 11:28:23AM -0600, Gary R Hook wrote:
>>
>> It may not have been obvious from the title but this fixes a bug
>> which will impact the use of any HW RNG that is the only RNG
>> registered. The breakage of rmmod/modprobe -r that this fix obviates
>> suggests that it needs to make the 4.15 kernel, please. Just want to
>> ensure this gets the proper attention. Thank you.
>
> I don't think breaking rmmod is sufficiently serious to warrant
> immediate inclusion at this point. We can always send it via
> stable after the release.
>
> Thanks,
>
Oh. I just figured that problems introduced in a particular kernel level
should be resolved (if possible) in that same level.
Your call, of course; thank you for the clarification.