thr3ads.net - Virtualization - [PATCH v2 3/6] hw_random: use reference counts on each struct hwrng. [Oct 2014]

If this information is useful, please help other people find it:
Share via:

Amos Kong

2014-Sep-18 17:08 UTC

[PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

On Thu, Sep 18, 2014 at 08:37:44PM +0800, Amos Kong
wrote:> From: Rusty Russell <rusty at rustcorp.com.au>
> 
> current_rng holds one reference, and we bump it every time we want
> to do a read from it.
> 
> This means we only hold the rng_mutex to grab or drop a reference,
> so accessing /sys/devices/virtual/misc/hw_random/rng_current doesn't
> block on read of /dev/hwrng.
> 
> Using a kref is overkill (we're always under the rng_mutex), but
> a standard pattern.
> 
> This also solves the problem that the hwrng_fillfn thread was
> accessing current_rng without a lock, which could change (eg. to NULL)
> underneath it.
> 
> V2: reduce reference count in signal_pending path
> 
> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
> Signed-off-by: Amos Kong <akong at redhat.com>
Reply myself.
> ---
>  drivers/char/hw_random/core.c | 136
+++++++++++++++++++++++++++++-------------
>  include/linux/hw_random.h     |   2 +
>  2 files changed, 95 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index a0905c8..ee9e504 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -42,6 +42,7 @@
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/random.h>
> +#include <linux/err.h>
>  #include <asm/uaccess.h>
>  
>  
> @@ -91,6 +92,59 @@ static void add_early_randomness(struct hwrng *rng)
>  		add_device_randomness(bytes, bytes_read);
>  }
>  
> +static inline void cleanup_rng(struct kref *kref)
> +{
> +	struct hwrng *rng = container_of(kref, struct hwrng, ref);
> +
> +	if (rng->cleanup)
> +		rng->cleanup(rng);
> +}
> +
> +static void set_current_rng(struct hwrng *rng)
> +{
> +	BUG_ON(!mutex_is_locked(&rng_mutex));
> +	kref_get(&rng->ref);
> +	current_rng = rng;
> +}
> +
> +static void drop_current_rng(void)
> +{
> +	BUG_ON(!mutex_is_locked(&rng_mutex));
> +	if (!current_rng)
> +		return;
> +
> +	kref_put(&current_rng->ref, cleanup_rng);
> +	current_rng = NULL;
> +}
> +
> +/* Returns ERR_PTR(), NULL or refcounted hwrng */
> +static struct hwrng *get_current_rng(void)
> +{
> +	struct hwrng *rng;
> +
> +	if (mutex_lock_interruptible(&rng_mutex))
> +		return ERR_PTR(-ERESTARTSYS);
> +
> +	rng = current_rng;
> +	if (rng)
> +		kref_get(&rng->ref);
> +
> +	mutex_unlock(&rng_mutex);
> +	return rng;
> +}
> +
> +static void put_rng(struct hwrng *rng)
> +{
> +	/*
> +	 * Hold rng_mutex here so we serialize in case they set_current_rng
> +	 * on rng again immediately.
> +	 */
> +	mutex_lock(&rng_mutex);
> +	if (rng)
> +		kref_put(&rng->ref, cleanup_rng);
> +	mutex_unlock(&rng_mutex);
> +}
> +
>  static inline int hwrng_init(struct hwrng *rng)
>  {
>  	if (rng->init) {
> @@ -113,12 +167,6 @@ static inline int hwrng_init(struct hwrng *rng)
>  	return 0;
>  }
>  
> -static inline void hwrng_cleanup(struct hwrng *rng)
> -{
> -	if (rng && rng->cleanup)
> -		rng->cleanup(rng);
> -}
> -
>  static int rng_dev_open(struct inode *inode, struct file *filp)
>  {
>  	/* enforce read-only access to this chrdev */
> @@ -154,21 +202,22 @@ static ssize_t rng_dev_read(struct file *filp, char
__user *buf,
>  	ssize_t ret = 0;
>  	int err = 0;
>  	int bytes_read, len;
> +	struct hwrng *rng;
>  
>  	while (size) {
> -		if (mutex_lock_interruptible(&rng_mutex)) {
> -			err = -ERESTARTSYS;
> +		rng = get_current_rng();
> +		if (IS_ERR(rng)) {
> +			err = PTR_ERR(rng);
>  			goto out;
>  		}
> -
> -		if (!current_rng) {
> +		if (!rng) {
>  			err = -ENODEV;
> -			goto out_unlock;
> +			goto out;
>  		}
>  
>  		mutex_lock(&reading_mutex);
>  		if (!data_avail) {
> -			bytes_read = rng_get_data(current_rng, rng_buffer,
> +			bytes_read = rng_get_data(rng, rng_buffer,
>  				rng_buffer_size(),
>  				!(filp->f_flags & O_NONBLOCK));
>  			if (bytes_read < 0) {
> @@ -200,7 +249,6 @@ static ssize_t rng_dev_read(struct file *filp, char
__user *buf,
>  			ret += len;
>  		}
>  
> -		mutex_unlock(&rng_mutex);
>  		mutex_unlock(&reading_mutex);
>  
>  		if (need_resched())
> @@ -208,17 +256,19 @@ static ssize_t rng_dev_read(struct file *filp, char
__user *buf,
>  
>  		if (signal_pending(current)) {
>  			err = -ERESTARTSYS;
> +			put_rng(rng);
>  			goto out;
>  		}
> +
> +		put_rng(rng);
>  	}
>  out:
>  	return ret ? : err;
> -out_unlock:
> -	mutex_unlock(&rng_mutex);
> -	goto out;
> +
>  out_unlock_reading:
>  	mutex_unlock(&reading_mutex);
> -	goto out_unlock;
> +	put_rng(rng);
> +	goto out;
>  }
>  
>  
> @@ -257,8 +307,8 @@ static ssize_t hwrng_attr_current_store(struct device
*dev,
>  			err = hwrng_init(rng);
>  			if (err)
>  				break;
> -			hwrng_cleanup(current_rng);
> -			current_rng = rng;
> +			drop_current_rng();
> +			set_current_rng(rng);
We got a warning in boot stage when above set_current_rng() is executed,
it can be fixed by init rng->ref in hwrng_init().


@@ -166,6 +169,8 @@ static inline int hwrng_init(struct hwrng *rng)
        if (current_quality > 0 && !hwrng_fill)
                start_khwrngd();
 
+       kref_init(&rng->ref);
+
        return 0;
 }


[    2.754303] ------------[ cut here ]------------
[    2.756018] WARNING: at include/linux/kref.h:47 kref_get.part.2+0x1e/0x27()
[    2.758150] Modules linked in: virtio_rng(+) parport_pc(+) parport mperf xfs
libcrc32c sd_mod sr_mod cdrom crc_t10dif crct10dif_common ata_generic pata_acpi
virtio_net cirrus syscopyarea sysfillrect sysimgblt drm_kms_helper ttm ata_piix
drm libata virtio_pci virtio_ring virtio i2c_core floppy dm_mirror
dm_region_hash dm_log dm_mod
[    2.765686] CPU: 0 PID: 470 Comm: systemd-udevd Not tainted
3.10.0-161.el7.rusty.x86_64 #1
[    2.767806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014
[    2.770449]  0000000000000000 0000000075730141 ffff88007bb13b68
ffffffff815f0673
[    2.772570]  ffff88007bb13ba0 ffffffff8106bc41 ffffffff819a0e50
ffff880036cd1000
[    2.774970]  ffff8800370a2cc0 0000000000000000 ffffffffa025c0e0
ffff88007bb13bb0
[    2.777267] Call Trace:
[    2.778843]  [<ffffffff815f0673>] dump_stack+0x19/0x1b
[    2.780824]  [<ffffffff8106bc41>] warn_slowpath_common+0x61/0x80
[    2.782726]  [<ffffffff8106bd6a>] warn_slowpath_null+0x1a/0x20
[    2.784566]  [<ffffffff815f106f>] kref_get.part.2+0x1e/0x27
[    2.786382]  [<ffffffff813b2d1a>] set_current_rng+0x3a/0x50
[    2.788184]  [<ffffffff813b32f8>] hwrng_register+0x148/0x290
[    2.790175]  [<ffffffffa005f7c0>] ? vp_reset+0x90/0x90 [virtio_pci]
[    2.792456]  [<ffffffffa025a149>] virtrng_scan+0x19/0x30 [virtio_rng]
[    2.794424]  [<ffffffffa0022208>] virtio_dev_probe+0x118/0x150 [virtio]
[    2.796391]  [<ffffffff813cac57>] driver_probe_device+0x87/0x390
[    2.798579]  [<ffffffff813cb033>] __driver_attach+0x93/0xa0
[    2.800576]  [<ffffffff813cafa0>] ? __device_attach+0x40/0x40
[    2.802500]  [<ffffffff813c89e3>] bus_for_each_dev+0x73/0xc0
[    2.804387]  [<ffffffff813ca6ae>] driver_attach+0x1e/0x20
[    2.806284]  [<ffffffff813ca200>] bus_add_driver+0x200/0x2d0
[    2.808511]  [<ffffffffa0034000>] ? 0xffffffffa0033fff
[    2.810313]  [<ffffffff813cb6b4>] driver_register+0x64/0xf0
[    2.812265]  [<ffffffffa0022540>] register_virtio_driver+0x20/0x30
[virtio]
[    2.814246]  [<ffffffffa0034010>] virtio_rng_driver_init+0x10/0x1000
[virtio_rng]
[    2.816253]  [<ffffffff810020b8>] do_one_initcall+0xb8/0x230
[    2.818053]  [<ffffffff810da4fa>] load_module+0x131a/0x1b20
[    2.819835]  [<ffffffff812ede80>] ? ddebug_proc_write+0xf0/0xf0
[    2.821635]  [<ffffffff810d6a93>] ?
copy_module_from_fd.isra.43+0x53/0x150
[    2.823723]  [<ffffffff810daeb6>] SyS_finit_module+0xa6/0xd0
[    2.825892]  [<ffffffff81600669>] system_call_fastpath+0x16/0x1b
[    2.827768] ---[ end trace bf8396ed0ec968f2 ]---

>  			err = 0;
>  			break;
>  		}
> @@ -272,17 +322,15 @@ static ssize_t hwrng_attr_current_show(struct device
*dev,
>  				       struct device_attribute *attr,
>  				       char *buf)
>  {
> -	int err;
>  	ssize_t ret;
> -	const char *name = "none";
> +	struct hwrng *rng;
>  
> -	err = mutex_lock_interruptible(&rng_mutex);
> -	if (err)
> -		return -ERESTARTSYS;
> -	if (current_rng)
> -		name = current_rng->name;
> -	ret = snprintf(buf, PAGE_SIZE, "%s\n", name);
> -	mutex_unlock(&rng_mutex);
> +	rng = get_current_rng();
> +	if (IS_ERR(rng))
> +		return PTR_ERR(rng);
> +
> +	ret = snprintf(buf, PAGE_SIZE, "%s\n", rng ? rng->name :
"none");
> +	put_rng(rng);
>  
>  	return ret;
>  }
> @@ -357,12 +405,16 @@ static int hwrng_fillfn(void *unused)
>  	long rc;
>  
>  	while (!kthread_should_stop()) {
> -		if (!current_rng)
> +		struct hwrng *rng;
> +
> +		rng = get_current_rng();
> +		if (IS_ERR(rng) || !rng)
>  			break;
>  		mutex_lock(&reading_mutex);
> -		rc = rng_get_data(current_rng, rng_fillbuf,
> +		rc = rng_get_data(rng, rng_fillbuf,
>  				  rng_buffer_size(), 1);
>  		mutex_unlock(&reading_mutex);
> +		put_rng(rng);
>  		if (rc <= 0) {
>  			pr_warn("hwrng: no data available\n");
>  			msleep_interruptible(10000);
> @@ -423,14 +475,13 @@ int hwrng_register(struct hwrng *rng)
>  		err = hwrng_init(rng);
>  		if (err)
>  			goto out_unlock;
> -		current_rng = rng;
> +		set_current_rng(rng);
>  	}
>  	err = 0;
>  	if (!old_rng) {
>  		err = register_miscdev();
>  		if (err) {
> -			hwrng_cleanup(rng);
> -			current_rng = NULL;
> +			drop_current_rng();
>  			goto out_unlock;
>  		}
>  	}
> @@ -457,22 +508,21 @@ 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) {
> -		hwrng_cleanup(rng);
> -		if (list_empty(&rng_list)) {
> -			current_rng = NULL;
> -		} else {
> -			current_rng = list_entry(rng_list.prev, struct hwrng, list);
> -			err = hwrng_init(current_rng);
> -			if (err)
> -				current_rng = NULL;
> +		drop_current_rng();
> +		if (!list_empty(&rng_list)) {
> +			struct hwrng *tail;
> +
> +			tail = list_entry(rng_list.prev, struct hwrng, list);
> +
> +			if (hwrng_init(tail) == 0)
> +				set_current_rng(tail);
>  		}
>  	}
> +
>  	if (list_empty(&rng_list)) {
>  		mutex_unlock(&rng_mutex);
>  		unregister_miscdev();
> diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
> index 914bb08..c212e71 100644
> --- a/include/linux/hw_random.h
> +++ b/include/linux/hw_random.h
> @@ -14,6 +14,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/list.h>
> +#include <linux/kref.h>
>  
>  /**
>   * struct hwrng - Hardware Random Number Generator driver
> @@ -44,6 +45,7 @@ struct hwrng {
>  
>  	/* internal. */
>  	struct list_head list;
> +	struct kref ref;
>  };
>  
>  /** Register a new Hardware Random Number Generator driver. */
> -- 
> 1.9.3
> 
> _______________________________________________
> Virtualization mailing list
> Virtualization at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
-- 
			Amos.

Rusty Russell

2014-Oct-20 00:12 UTC

head link

[PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

Amos Kong <akong at redhat.com> writes:> We got a warning in boot stage when above set_current_rng() is executed,
> it can be fixed by init rng->ref in hwrng_init().
>
>
> @@ -166,6 +169,8 @@ static inline int hwrng_init(struct hwrng *rng)
>         if (current_quality > 0 && !hwrng_fill)
>                 start_khwrngd();
>  
> +       kref_init(&rng->ref);
> +
>         return 0;
>  }
OK, I folded this fix on.

Thanks,
Rusty.

hw_random: use reference counts on each struct hwrng.

current_rng holds one reference, and we bump it every time we want
to do a read from it.

This means we only hold the rng_mutex to grab or drop a reference,
so accessing /sys/devices/virtual/misc/hw_random/rng_current doesn't
block on read of /dev/hwrng.

Using a kref is overkill (we're always under the rng_mutex), but
a standard pattern.

This also solves the problem that the hwrng_fillfn thread was
accessing current_rng without a lock, which could change (eg. to NULL)
underneath it.

v3: initialize kref (thanks Amos Kong)
v2: fix missing put_rng() on exit path (thanks Amos Kong)
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index a0905c818e13..0ecac38da954 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -42,6 +42,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/random.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
 
@@ -91,6 +92,59 @@ static void add_early_randomness(struct hwrng *rng)
 		add_device_randomness(bytes, bytes_read);
 }
 
+static inline void cleanup_rng(struct kref *kref)
+{
+	struct hwrng *rng = container_of(kref, struct hwrng, ref);
+
+	if (rng->cleanup)
+		rng->cleanup(rng);
+}
+
+static void set_current_rng(struct hwrng *rng)
+{
+	BUG_ON(!mutex_is_locked(&rng_mutex));
+	kref_get(&rng->ref);
+	current_rng = rng;
+}
+
+static void drop_current_rng(void)
+{
+	BUG_ON(!mutex_is_locked(&rng_mutex));
+	if (!current_rng)
+		return;
+
+	kref_put(&current_rng->ref, cleanup_rng);
+	current_rng = NULL;
+}
+
+/* Returns ERR_PTR(), NULL or refcounted hwrng */
+static struct hwrng *get_current_rng(void)
+{
+	struct hwrng *rng;
+
+	if (mutex_lock_interruptible(&rng_mutex))
+		return ERR_PTR(-ERESTARTSYS);
+
+	rng = current_rng;
+	if (rng)
+		kref_get(&rng->ref);
+
+	mutex_unlock(&rng_mutex);
+	return rng;
+}
+
+static void put_rng(struct hwrng *rng)
+{
+	/*
+	 * Hold rng_mutex here so we serialize in case they set_current_rng
+	 * on rng again immediately.
+	 */
+	mutex_lock(&rng_mutex);
+	if (rng)
+		kref_put(&rng->ref, cleanup_rng);
+	mutex_unlock(&rng_mutex);
+}
+
 static inline int hwrng_init(struct hwrng *rng)
 {
 	if (rng->init) {
@@ -110,13 +164,9 @@ static inline int hwrng_init(struct hwrng *rng)
 	if (current_quality > 0 && !hwrng_fill)
 		start_khwrngd();
 
-	return 0;
-}
+	kref_init(&rng->ref);
 
-static inline void hwrng_cleanup(struct hwrng *rng)
-{
-	if (rng && rng->cleanup)
-		rng->cleanup(rng);
+	return 0;
 }
 
 static int rng_dev_open(struct inode *inode, struct file *filp)
@@ -154,21 +204,22 @@ static ssize_t rng_dev_read(struct file *filp, char __user
*buf,
 	ssize_t ret = 0;
 	int err = 0;
 	int bytes_read, len;
+	struct hwrng *rng;
 
 	while (size) {
-		if (mutex_lock_interruptible(&rng_mutex)) {
-			err = -ERESTARTSYS;
+		rng = get_current_rng();
+		if (IS_ERR(rng)) {
+			err = PTR_ERR(rng);
 			goto out;
 		}
-
-		if (!current_rng) {
+		if (!rng) {
 			err = -ENODEV;
-			goto out_unlock;
+			goto out;
 		}
 
 		mutex_lock(&reading_mutex);
 		if (!data_avail) {
-			bytes_read = rng_get_data(current_rng, rng_buffer,
+			bytes_read = rng_get_data(rng, rng_buffer,
 				rng_buffer_size(),
 				!(filp->f_flags & O_NONBLOCK));
 			if (bytes_read < 0) {
@@ -200,8 +251,8 @@ static ssize_t rng_dev_read(struct file *filp, char __user
*buf,
 			ret += len;
 		}
 
-		mutex_unlock(&rng_mutex);
 		mutex_unlock(&reading_mutex);
+		put_rng(rng);
 
 		if (need_resched())
 			schedule_timeout_interruptible(1);
@@ -213,12 +264,11 @@ static ssize_t rng_dev_read(struct file *filp, char __user
*buf,
 	}
 out:
 	return ret ? : err;
-out_unlock:
-	mutex_unlock(&rng_mutex);
-	goto out;
+
 out_unlock_reading:
 	mutex_unlock(&reading_mutex);
-	goto out_unlock;
+	put_rng(rng);
+	goto out;
 }
 
 
@@ -257,8 +307,8 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
 			err = hwrng_init(rng);
 			if (err)
 				break;
-			hwrng_cleanup(current_rng);
-			current_rng = rng;
+			drop_current_rng();
+			set_current_rng(rng);
 			err = 0;
 			break;
 		}
@@ -272,17 +322,15 @@ static ssize_t hwrng_attr_current_show(struct device *dev,
 				       struct device_attribute *attr,
 				       char *buf)
 {
-	int err;
 	ssize_t ret;
-	const char *name = "none";
+	struct hwrng *rng;
 
-	err = mutex_lock_interruptible(&rng_mutex);
-	if (err)
-		return -ERESTARTSYS;
-	if (current_rng)
-		name = current_rng->name;
-	ret = snprintf(buf, PAGE_SIZE, "%s\n", name);
-	mutex_unlock(&rng_mutex);
+	rng = get_current_rng();
+	if (IS_ERR(rng))
+		return PTR_ERR(rng);
+
+	ret = snprintf(buf, PAGE_SIZE, "%s\n", rng ? rng->name :
"none");
+	put_rng(rng);
 
 	return ret;
 }
@@ -357,12 +405,16 @@ static int hwrng_fillfn(void *unused)
 	long rc;
 
 	while (!kthread_should_stop()) {
-		if (!current_rng)
+		struct hwrng *rng;
+
+		rng = get_current_rng();
+		if (IS_ERR(rng) || !rng)
 			break;
 		mutex_lock(&reading_mutex);
-		rc = rng_get_data(current_rng, rng_fillbuf,
+		rc = rng_get_data(rng, rng_fillbuf,
 				  rng_buffer_size(), 1);
 		mutex_unlock(&reading_mutex);
+		put_rng(rng);
 		if (rc <= 0) {
 			pr_warn("hwrng: no data available\n");
 			msleep_interruptible(10000);
@@ -423,14 +475,13 @@ int hwrng_register(struct hwrng *rng)
 		err = hwrng_init(rng);
 		if (err)
 			goto out_unlock;
-		current_rng = rng;
+		set_current_rng(rng);
 	}
 	err = 0;
 	if (!old_rng) {
 		err = register_miscdev();
 		if (err) {
-			hwrng_cleanup(rng);
-			current_rng = NULL;
+			drop_current_rng();
 			goto out_unlock;
 		}
 	}
@@ -457,22 +508,21 @@ 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) {
-		hwrng_cleanup(rng);
-		if (list_empty(&rng_list)) {
-			current_rng = NULL;
-		} else {
-			current_rng = list_entry(rng_list.prev, struct hwrng, list);
-			err = hwrng_init(current_rng);
-			if (err)
-				current_rng = NULL;
+		drop_current_rng();
+		if (!list_empty(&rng_list)) {
+			struct hwrng *tail;
+
+			tail = list_entry(rng_list.prev, struct hwrng, list);
+
+			if (hwrng_init(tail) == 0)
+				set_current_rng(tail);
 		}
 	}
+
 	if (list_empty(&rng_list)) {
 		mutex_unlock(&rng_mutex);
 		unregister_miscdev();
diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index 914bb08cd738..c212e71ea886 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -14,6 +14,7 @@
 
 #include <linux/types.h>
 #include <linux/list.h>
+#include <linux/kref.h>
 
 /**
  * struct hwrng - Hardware Random Number Generator driver
@@ -44,6 +45,7 @@ struct hwrng {
 
 	/* internal. */
 	struct list_head list;
+	struct kref ref;
 };
 
 /** Register a new Hardware Random Number Generator driver. */

Amos Kong

2014-Oct-20 03:58 UTC

head link

[PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

On Mon, Oct 20, 2014 at 10:42:11AM +1030, Rusty Russell
wrote:> Amos Kong <akong at redhat.com> writes:
> > We got a warning in boot stage when above set_current_rng() is
executed,
> > it can be fixed by init rng->ref in hwrng_init().
> >
> >
> > @@ -166,6 +169,8 @@ static inline int hwrng_init(struct hwrng *rng)
> >         if (current_quality > 0 && !hwrng_fill)
> >                 start_khwrngd();
> >  
> > +       kref_init(&rng->ref);
> > +
> >         return 0;
> >  }
> 
> OK, I folded this fix on.
Thanks.

Reviewed-by: Amos Kong <akong at redhat.com>
 > Thanks,
> Rusty.
> 
> hw_random: use reference counts on each struct hwrng.
> 
> current_rng holds one reference, and we bump it every time we want
> to do a read from it.
> 
> This means we only hold the rng_mutex to grab or drop a reference,
> so accessing /sys/devices/virtual/misc/hw_random/rng_current doesn't
> block on read of /dev/hwrng.
> 
> Using a kref is overkill (we're always under the rng_mutex), but
> a standard pattern.
> 
> This also solves the problem that the hwrng_fillfn thread was
> accessing current_rng without a lock, which could change (eg. to NULL)
> underneath it.
> 
> v3: initialize kref (thanks Amos Kong)
> v2: fix missing put_rng() on exit path (thanks Amos Kong)
> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index a0905c818e13..0ecac38da954 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c....

Seemingly Similar Threads

Search for more reasonably related threads

Virtualization - Oct 2014 - [PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

[PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

[PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

[PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.

Seemingly Similar Threads