All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
@ 2017-10-27 17:04 PrasannaKumar Muralidharan
  2017-10-30  8:10 ` Harald Freudenberger
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-10-27 17:04 UTC (permalink / raw)
  To: mpm, herbert, freude, linux-crypto; +Cc: PrasannaKumar Muralidharan

User is able to select a chosen rng by writing its name to rng_current
but there is no way to reset it without unbinding the rng. Let user
write "" to rng_current and delesect the chosen rng.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 drivers/char/hw_random/core.c | 51 +++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 9701ac7..be03024 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -292,26 +292,48 @@ static struct miscdevice rng_miscdev = {
 	.groups		= rng_dev_groups,
 };
 
+static int enable_best_rng(void)
+{
+	int ret = -ENODEV;
+
+	BUG_ON(!mutex_is_locked(&rng_mutex));
+
+	/* rng_list is sorted by quality, use the best (=first) one */
+	if (!list_empty(&rng_list)) {
+		struct hwrng *new_rng;
+
+		new_rng = list_entry(rng_list.next, struct hwrng, list);
+		ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
+		if (!ret)
+			cur_rng_set_by_user = 0;
+	}
+
+	return ret;
+}
+
 static ssize_t hwrng_attr_current_store(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t len)
 {
-	int err;
+	int err = -ENODEV;
 	struct hwrng *rng;
 
 	err = mutex_lock_interruptible(&rng_mutex);
 	if (err)
 		return -ERESTARTSYS;
-	err = -ENODEV;
-	list_for_each_entry(rng, &rng_list, list) {
-		if (sysfs_streq(rng->name, buf)) {
-			err = 0;
-			cur_rng_set_by_user = 1;
-			if (rng != current_rng)
+
+	if (sysfs_streq(buf, "")) {
+		err = enable_best_rng();
+	} else {
+		list_for_each_entry(rng, &rng_list, list) {
+			if (sysfs_streq(rng->name, buf)) {
+				cur_rng_set_by_user = 1;
 				err = set_current_rng(rng);
-			break;
+				break;
+			}
 		}
 	}
+
 	mutex_unlock(&rng_mutex);
 
 	return err ? : len;
@@ -493,17 +515,8 @@ void hwrng_unregister(struct hwrng *rng)
 	mutex_lock(&rng_mutex);
 
 	list_del(&rng->list);
-	if (current_rng == rng) {
-		drop_current_rng();
-		cur_rng_set_by_user = 0;
-		/* rng_list is sorted by quality, use the best (=first) one */
-		if (!list_empty(&rng_list)) {
-			struct hwrng *new_rng;
-
-			new_rng = list_entry(rng_list.next, struct hwrng, list);
-			set_current_rng(new_rng);
-		}
-	}
+	if (current_rng == rng)
+		enable_best_rng();
 
 	if (list_empty(&rng_list)) {
 		mutex_unlock(&rng_mutex);
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
  2017-10-27 17:04 [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current PrasannaKumar Muralidharan
@ 2017-10-30  8:10 ` Harald Freudenberger
  2017-10-30 12:59   ` PrasannaKumar Muralidharan
  2017-10-30 13:28 ` Harald Freudenberger
  2017-11-03 14:28 ` Herbert Xu
  2 siblings, 1 reply; 6+ messages in thread
From: Harald Freudenberger @ 2017-10-30  8:10 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan, mpm, herbert, linux-crypto

On 10/27/2017 07:04 PM, PrasannaKumar Muralidharan wrote:
> User is able to select a chosen rng by writing its name to rng_current
> but there is no way to reset it without unbinding the rng. Let user
> write "" to rng_current and delesect the chosen rng.
>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  drivers/char/hw_random/core.c | 51 +++++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 9701ac7..be03024 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -292,26 +292,48 @@ static struct miscdevice rng_miscdev = {
>  	.groups		= rng_dev_groups,
>  };
>
> +static int enable_best_rng(void)
> +{
> +	int ret = -ENODEV;
> +
> +	BUG_ON(!mutex_is_locked(&rng_mutex));
> +
> +	/* rng_list is sorted by quality, use the best (=first) one */
> +	if (!list_empty(&rng_list)) {
> +		struct hwrng *new_rng;
> +
> +		new_rng = list_entry(rng_list.next, struct hwrng, list);
> +		ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
> +		if (!ret)
> +			cur_rng_set_by_user = 0;
> +	}
> +
> +	return ret;
> +}
> +
>  static ssize_t hwrng_attr_current_store(struct device *dev,
>  					struct device_attribute *attr,
>  					const char *buf, size_t len)
>  {
> -	int err;
> +	int err = -ENODEV;
>  	struct hwrng *rng;
>
>  	err = mutex_lock_interruptible(&rng_mutex);
>  	if (err)
>  		return -ERESTARTSYS;
> -	err = -ENODEV;
> -	list_for_each_entry(rng, &rng_list, list) {
> -		if (sysfs_streq(rng->name, buf)) {
> -			err = 0;
> -			cur_rng_set_by_user = 1;
> -			if (rng != current_rng)
> +
> +	if (sysfs_streq(buf, "")) {
> +		err = enable_best_rng();
> +	} else {
> +		list_for_each_entry(rng, &rng_list, list) {
> +			if (sysfs_streq(rng->name, buf)) {
> +				cur_rng_set_by_user = 1;
>  				err = set_current_rng(rng);
> -			break;
> +				break;
> +			}
>  		}
>  	}
> +
>  	mutex_unlock(&rng_mutex);
>
>  	return err ? : len;
> @@ -493,17 +515,8 @@ void hwrng_unregister(struct hwrng *rng)
>  	mutex_lock(&rng_mutex);
>
>  	list_del(&rng->list);
> -	if (current_rng == rng) {
> -		drop_current_rng();
> -		cur_rng_set_by_user = 0;
> -		/* rng_list is sorted by quality, use the best (=first) one */
> -		if (!list_empty(&rng_list)) {
> -			struct hwrng *new_rng;
> -
> -			new_rng = list_entry(rng_list.next, struct hwrng, list);
> -			set_current_rng(new_rng);
> -		}
> -	}
> +	if (current_rng == rng)
> +		enable_best_rng();
>
>  	if (list_empty(&rng_list)) {
>  		mutex_unlock(&rng_mutex);
That's a really good idea. I also thought about something like that.
regards
Harald Freudenberger

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
  2017-10-30  8:10 ` Harald Freudenberger
@ 2017-10-30 12:59   ` PrasannaKumar Muralidharan
  0 siblings, 0 replies; 6+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-10-30 12:59 UTC (permalink / raw)
  To: Harald Freudenberger
  Cc: Matt Mackall, Herbert Xu,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE

Hi Harald,

On 30 October 2017 at 13:40, Harald Freudenberger
<freude@linux.vnet.ibm.com> wrote:
> That's a really good idea. I also thought about something like that.

Do you mind reviewing the code?

Thanks,
PrasannaKumar

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
  2017-10-27 17:04 [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current PrasannaKumar Muralidharan
  2017-10-30  8:10 ` Harald Freudenberger
@ 2017-10-30 13:28 ` Harald Freudenberger
  2017-10-30 13:42   ` PrasannaKumar Muralidharan
  2017-11-03 14:28 ` Herbert Xu
  2 siblings, 1 reply; 6+ messages in thread
From: Harald Freudenberger @ 2017-10-30 13:28 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan, mpm, herbert, linux-crypto

On 10/27/2017 07:04 PM, PrasannaKumar Muralidharan wrote:
> User is able to select a chosen rng by writing its name to rng_current
> but there is no way to reset it without unbinding the rng. Let user
> write "" to rng_current and delesect the chosen rng.
>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  drivers/char/hw_random/core.c | 51 +++++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 9701ac7..be03024 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -292,26 +292,48 @@ static struct miscdevice rng_miscdev = {
>  	.groups		= rng_dev_groups,
>  };
>
> +static int enable_best_rng(void)
> +{
> +	int ret = -ENODEV;
> +
> +	BUG_ON(!mutex_is_locked(&rng_mutex));
> +
> +	/* rng_list is sorted by quality, use the best (=first) one */
> +	if (!list_empty(&rng_list)) {
> +		struct hwrng *new_rng;
> +
> +		new_rng = list_entry(rng_list.next, struct hwrng, list);
> +		ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
> +		if (!ret)
> +			cur_rng_set_by_user = 0;
> +	}
> +
> +	return ret;
> +}
> +
>  static ssize_t hwrng_attr_current_store(struct device *dev,
>  					struct device_attribute *attr,
>  					const char *buf, size_t len)
>  {
> -	int err;
> +	int err = -ENODEV;
Two lines later err is assigned the return value of mutex_lock_interruptible().
So the -ENODEV value assignment does nothing.
>  	struct hwrng *rng;
>
>  	err = mutex_lock_interruptible(&rng_mutex);
>  	if (err)
>  		return -ERESTARTSYS;
> -	err = -ENODEV;
> -	list_for_each_entry(rng, &rng_list, list) {
> -		if (sysfs_streq(rng->name, buf)) {
> -			err = 0;
> -			cur_rng_set_by_user = 1;
> -			if (rng != current_rng)
> +
> +	if (sysfs_streq(buf, "")) {
> +		err = enable_best_rng();
> +	} else {
> +		list_for_each_entry(rng, &rng_list, list) {
> +			if (sysfs_streq(rng->name, buf)) {
> +				cur_rng_set_by_user = 1;
>  				err = set_current_rng(rng);
> -			break;
> +				break;
> +			}
>  		}
>  	}
> +
>  	mutex_unlock(&rng_mutex);
>
>  	return err ? : len;
> @@ -493,17 +515,8 @@ void hwrng_unregister(struct hwrng *rng)
>  	mutex_lock(&rng_mutex);
>
>  	list_del(&rng->list);
> -	if (current_rng == rng) {
> -		drop_current_rng();
> -		cur_rng_set_by_user = 0;
> -		/* rng_list is sorted by quality, use the best (=first) one */
> -		if (!list_empty(&rng_list)) {
> -			struct hwrng *new_rng;
> -
> -			new_rng = list_entry(rng_list.next, struct hwrng, list);
> -			set_current_rng(new_rng);
> -		}
> -	}
> +	if (current_rng == rng)
> +		enable_best_rng();
>
>  	if (list_empty(&rng_list)) {
>  		mutex_unlock(&rng_mutex);
looks like for me.
reviewed-by: Harald Freudenberger <freude@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
  2017-10-30 13:28 ` Harald Freudenberger
@ 2017-10-30 13:42   ` PrasannaKumar Muralidharan
  0 siblings, 0 replies; 6+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-10-30 13:42 UTC (permalink / raw)
  To: Harald Freudenberger
  Cc: Matt Mackall, Herbert Xu,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE

Hi Harald,

On 30 October 2017 at 18:58, Harald Freudenberger
<freude@linux.vnet.ibm.com> wrote:
> On 10/27/2017 07:04 PM, PrasannaKumar Muralidharan wrote:
>> User is able to select a chosen rng by writing its name to rng_current
>> but there is no way to reset it without unbinding the rng. Let user
>> write "" to rng_current and delesect the chosen rng.
>>
>> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
>> ---
>>  drivers/char/hw_random/core.c | 51 +++++++++++++++++++++++++++----------------
>>  1 file changed, 32 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>> index 9701ac7..be03024 100644
>> --- a/drivers/char/hw_random/core.c
>> +++ b/drivers/char/hw_random/core.c
>> @@ -292,26 +292,48 @@ static struct miscdevice rng_miscdev = {
>>       .groups         = rng_dev_groups,
>>  };
>>
>> +static int enable_best_rng(void)
>> +{
>> +     int ret = -ENODEV;
>> +
>> +     BUG_ON(!mutex_is_locked(&rng_mutex));
>> +
>> +     /* rng_list is sorted by quality, use the best (=first) one */
>> +     if (!list_empty(&rng_list)) {
>> +             struct hwrng *new_rng;
>> +
>> +             new_rng = list_entry(rng_list.next, struct hwrng, list);
>> +             ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>> +             if (!ret)
>> +                     cur_rng_set_by_user = 0;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>>  static ssize_t hwrng_attr_current_store(struct device *dev,
>>                                       struct device_attribute *attr,
>>                                       const char *buf, size_t len)
>>  {
>> -     int err;
>> +     int err = -ENODEV;
> Two lines later err is assigned the return value of mutex_lock_interruptible().
> So the -ENODEV value assignment does nothing.

Indeed true. Will change this when I make a v2.

I am even thinking if it makes sense to return error when user writes
"" to rng_current and there is no rng device registered. I think
returning 0 should be fine.

>>       struct hwrng *rng;
>>
>>       err = mutex_lock_interruptible(&rng_mutex);
>>       if (err)
>>               return -ERESTARTSYS;
>> -     err = -ENODEV;
>> -     list_for_each_entry(rng, &rng_list, list) {
>> -             if (sysfs_streq(rng->name, buf)) {
>> -                     err = 0;
>> -                     cur_rng_set_by_user = 1;
>> -                     if (rng != current_rng)
>> +
>> +     if (sysfs_streq(buf, "")) {
>> +             err = enable_best_rng();
>> +     } else {
>> +             list_for_each_entry(rng, &rng_list, list) {
>> +                     if (sysfs_streq(rng->name, buf)) {
>> +                             cur_rng_set_by_user = 1;
>>                               err = set_current_rng(rng);
>> -                     break;
>> +                             break;
>> +                     }
>>               }
>>       }
>> +
>>       mutex_unlock(&rng_mutex);
>>
>>       return err ? : len;
>> @@ -493,17 +515,8 @@ void hwrng_unregister(struct hwrng *rng)
>>       mutex_lock(&rng_mutex);
>>
>>       list_del(&rng->list);
>> -     if (current_rng == rng) {
>> -             drop_current_rng();
>> -             cur_rng_set_by_user = 0;
>> -             /* rng_list is sorted by quality, use the best (=first) one */
>> -             if (!list_empty(&rng_list)) {
>> -                     struct hwrng *new_rng;
>> -
>> -                     new_rng = list_entry(rng_list.next, struct hwrng, list);
>> -                     set_current_rng(new_rng);
>> -             }
>> -     }
>> +     if (current_rng == rng)
>> +             enable_best_rng();
>>
>>       if (list_empty(&rng_list)) {
>>               mutex_unlock(&rng_mutex);
> looks like for me.
> reviewed-by: Harald Freudenberger <freude@linux.vnet.ibm.com>
>

Thank you.

Thanks,
PrasannaKumar

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
  2017-10-27 17:04 [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current PrasannaKumar Muralidharan
  2017-10-30  8:10 ` Harald Freudenberger
  2017-10-30 13:28 ` Harald Freudenberger
@ 2017-11-03 14:28 ` Herbert Xu
  2 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2017-11-03 14:28 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan; +Cc: mpm, freude, linux-crypto

On Fri, Oct 27, 2017 at 10:34:04PM +0530, PrasannaKumar Muralidharan wrote:
> User is able to select a chosen rng by writing its name to rng_current
> but there is no way to reset it without unbinding the rng. Let user
> write "" to rng_current and delesect the chosen rng.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>

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

end of thread, other threads:[~2017-11-03 14:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27 17:04 [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current PrasannaKumar Muralidharan
2017-10-30  8:10 ` Harald Freudenberger
2017-10-30 12:59   ` PrasannaKumar Muralidharan
2017-10-30 13:28 ` Harald Freudenberger
2017-10-30 13:42   ` PrasannaKumar Muralidharan
2017-11-03 14:28 ` 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.