2006-05-07 11:29:53

by Michael Büsch

[permalink] [raw]
Subject: [patch 2/6] New Generic HW RNG

Add a new generic H/W RNG core.

Signed-off-by: Michael Buesch <[email protected]>
Index: hwrng/drivers/char/Kconfig
===================================================================
--- hwrng.orig/drivers/char/Kconfig 2006-05-07 01:35:45.000000000 +0200
+++ hwrng/drivers/char/Kconfig 2006-05-07 01:36:15.000000000 +0200
@@ -670,6 +670,8 @@

If you're not sure, say N.

+source "drivers/char/hw_random/Kconfig"
+
config NVRAM
tristate "/dev/nvram support"
depends on ATARI || X86 || ARM || GENERIC_NVRAM
Index: hwrng/drivers/char/Makefile
===================================================================
--- hwrng.orig/drivers/char/Makefile 2006-05-07 01:35:45.000000000 +0200
+++ hwrng/drivers/char/Makefile 2006-05-07 01:36:15.000000000 +0200
@@ -75,6 +75,7 @@
obj-$(CONFIG_TOSHIBA) += toshiba.o
obj-$(CONFIG_I8K) += i8k.o
obj-$(CONFIG_DS1620) += ds1620.o
+obj-$(CONFIG_HW_RANDOM) += hw_random/
obj-$(CONFIG_FTAPE) += ftape/
obj-$(CONFIG_COBALT_LCD) += lcd.o
obj-$(CONFIG_PPDEV) += ppdev.o
Index: hwrng/drivers/char/hw_random/Kconfig
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ hwrng/drivers/char/hw_random/Kconfig 2006-05-07 01:40:45.000000000 +0200
@@ -0,0 +1,11 @@
+#
+# Hardware Random Number Generator (RNG) configuration
+#
+
+config HW_RANDOM
+ bool "Hardware Random Number Generator Core support"
+ default y
+ ---help---
+ Hardware Random Number Generator Core infrastructure.
+
+ If unsure, say Y.
Index: hwrng/drivers/char/hw_random/Makefile
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ hwrng/drivers/char/hw_random/Makefile 2006-05-07 01:41:11.000000000 +0200
@@ -0,0 +1,5 @@
+#
+# Makefile for HW Random Number Generator (RNG) device drivers.
+#
+
+obj-$(CONFIG_HW_RANDOM) += core.o
Index: hwrng/drivers/char/hw_random/core.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ hwrng/drivers/char/hw_random/core.c 2006-05-07 01:04:42.000000000 +0200
@@ -0,0 +1,324 @@
+/*
+ Added support for the AMD Geode LX RNG
+ (c) Copyright 2004-2005 Advanced Micro Devices, Inc.
+
+ derived from
+
+ Hardware driver for the Intel/AMD/VIA Random Number Generators (RNG)
+ (c) Copyright 2003 Red Hat Inc <[email protected]>
+
+ derived from
+
+ Hardware driver for the AMD 768 Random Number Generator (RNG)
+ (c) Copyright 2001 Red Hat Inc <[email protected]>
+
+ derived from
+
+ Hardware driver for Intel i810 Random Number Generator (RNG)
+ Copyright 2000,2001 Jeff Garzik <[email protected]>
+ Copyright 2000,2001 Philipp Rumpf <[email protected]>
+
+ Added generic RNG API
+ Copyright 2006 Michael Buesch <[email protected]>
+ Copyright 2005 (c) MontaVista Software, Inc.
+
+ Please read Documentation/hw_random.txt for details on use.
+
+ ----------------------------------------------------------
+ This software may be used and distributed according to the terms
+ of the GNU General Public License, incorporated herein by reference.
+
+ */
+
+
+#include <linux/device.h>
+#include <linux/hw_random.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/miscdevice.h>
+#include <linux/delay.h>
+#include <asm/uaccess.h>
+
+
+#define RNG_MODULE_NAME "hw_random"
+#define PFX RNG_MODULE_NAME ": "
+#define RNG_MISCDEV_MINOR 183 /* official */
+
+
+static struct hwrng *current_rng;
+static LIST_HEAD(rng_list);
+static DEFINE_MUTEX(rng_mutex);
+
+
+static int rng_dev_open(struct inode *inode, struct file *filp)
+{
+ /* enforce read-only access to this chrdev */
+ if ((filp->f_mode & FMODE_READ) == 0)
+ return -EINVAL;
+ if (filp->f_mode & FMODE_WRITE)
+ return -EINVAL;
+ return 0;
+}
+
+static ssize_t rng_dev_read(struct file *filp, char __user *buf,
+ size_t size, loff_t *offp)
+{
+ unsigned int have_data;
+ u32 data = 0;
+ ssize_t ret = 0;
+ int i, err;
+
+ while (size) {
+ err = mutex_lock_interruptible(&rng_mutex);
+ if (err)
+ return err;
+ if (!current_rng) {
+ mutex_unlock(&rng_mutex);
+ return -ENODEV;
+ }
+ have_data = 0;
+ if (current_rng->data_present == NULL ||
+ current_rng->data_present(current_rng))
+ have_data = current_rng->data_read(current_rng, &data);
+ mutex_unlock(&rng_mutex);
+
+ while (have_data && size) {
+ if (put_user((u8)data, buf++)) {
+ ret = ret ? : -EFAULT;
+ break;
+ }
+ size--;
+ ret++;
+ have_data--;
+ data>>=8;
+ }
+
+ if (filp->f_flags & O_NONBLOCK)
+ return ret ? : -EAGAIN;
+
+ if (need_resched()) {
+ schedule_timeout_interruptible(1);
+ } else {
+ err = mutex_lock_interruptible(&rng_mutex);
+ if (err)
+ return err;
+ if (!current_rng) {
+ mutex_unlock(&rng_mutex);
+ return -ENODEV;
+ }
+ for (i = 0; i < 20; i++) {
+ if (current_rng->data_present == NULL ||
+ current_rng->data_present(current_rng))
+ break;
+ udelay(10);
+ }
+ mutex_unlock(&rng_mutex);
+ }
+
+ if (signal_pending(current))
+ return ret ? : -ERESTARTSYS;
+ }
+ return ret;
+}
+
+
+static struct file_operations rng_chrdev_ops = {
+ .owner = THIS_MODULE,
+ .open = rng_dev_open,
+ .read = rng_dev_read,
+};
+
+static struct miscdevice rng_miscdev = {
+ .minor = RNG_MISCDEV_MINOR,
+ .name = RNG_MODULE_NAME,
+ .fops = &rng_chrdev_ops,
+};
+
+
+static ssize_t hwrng_attr_current_store(struct class_device *class,
+ const char *buf, size_t len)
+{
+ int err;
+ struct hwrng *rng;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ err = mutex_lock_interruptible(&rng_mutex);
+ if (err)
+ return err;
+ err = -ENODEV;
+ list_for_each_entry(rng, &rng_list, list) {
+ if (strncmp(rng->name, buf, len) == 0) {
+ if (rng->init) {
+ err = rng->init(rng);
+ if (err)
+ break;
+ }
+ if (current_rng && current_rng->cleanup)
+ current_rng->cleanup(current_rng);
+ current_rng = rng;
+ err = 0;
+ break;
+ }
+ }
+ mutex_unlock(&rng_mutex);
+
+ return err ? : len;
+}
+
+static ssize_t hwrng_attr_current_show(struct class_device *class,
+ char *buf)
+{
+ int err;
+ ssize_t ret;
+ const char *name = "none";
+
+ err = mutex_lock_interruptible(&rng_mutex);
+ if (err)
+ return err;
+ if (current_rng)
+ name = current_rng->name;
+ ret = snprintf(buf, PAGE_SIZE, "%s\n", name);
+ mutex_unlock(&rng_mutex);
+
+ return ret;
+}
+
+static ssize_t hwrng_attr_available_show(struct class_device *class,
+ char *buf)
+{
+ int err;
+ ssize_t ret = 0;
+ struct hwrng *rng;
+
+ err = mutex_lock_interruptible(&rng_mutex);
+ if (err)
+ return err;
+ buf[0] = '\0';
+ list_for_each_entry(rng, &rng_list, list) {
+ strncat(buf, rng->name, PAGE_SIZE - ret - 1);
+ ret += strlen(rng->name);
+ strncat(buf, " ", PAGE_SIZE - ret - 1);
+ ret++;
+ }
+ strncat(buf, "\n", PAGE_SIZE - ret - 1);
+ ret++;
+ mutex_unlock(&rng_mutex);
+
+ return ret;
+}
+
+static CLASS_DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR,
+ hwrng_attr_current_show,
+ hwrng_attr_current_store);
+static CLASS_DEVICE_ATTR(rng_available, S_IRUGO,
+ hwrng_attr_available_show,
+ NULL);
+
+
+static void unregister_miscdev(void)
+{
+ class_device_remove_file(rng_miscdev.class,
+ &class_device_attr_rng_available);
+ class_device_remove_file(rng_miscdev.class,
+ &class_device_attr_rng_current);
+ misc_deregister(&rng_miscdev);
+}
+
+static int register_miscdev(void)
+{
+ int err;
+
+ err = misc_register(&rng_miscdev);
+ if (err)
+ goto out;
+ err = class_device_create_file(rng_miscdev.class,
+ &class_device_attr_rng_current);
+ if (err)
+ goto err_misc_dereg;
+ err = class_device_create_file(rng_miscdev.class,
+ &class_device_attr_rng_available);
+ if (err)
+ goto err_remove_current;
+out:
+ return err;
+
+err_remove_current:
+ class_device_remove_file(rng_miscdev.class,
+ &class_device_attr_rng_current);
+err_misc_dereg:
+ misc_deregister(&rng_miscdev);
+ goto out;
+}
+
+int hwrng_register(struct hwrng *rng)
+{
+ int must_register_misc;
+ int err;
+ struct hwrng *old_current;
+
+ if (rng->name == NULL)
+ return -EINVAL;
+ if (rng->data_read == NULL)
+ return -EINVAL;
+
+ mutex_lock(&rng_mutex);
+
+ must_register_misc = (current_rng == NULL);
+ old_current = current_rng;
+ if (!current_rng) {
+ if (rng->init) {
+ err = rng->init(rng);
+ if (err) {
+ mutex_unlock(&rng_mutex);
+ return err;
+ }
+ }
+ current_rng = rng;
+ }
+ INIT_LIST_HEAD(&rng->list);
+ list_add_tail(&rng->list, &rng_list);
+ err = 0;
+ if (must_register_misc) {
+ err = register_miscdev();
+ if (err) {
+ if (rng->cleanup)
+ rng->cleanup(rng);
+ list_del(&rng->list);
+ current_rng = old_current;
+ }
+ }
+
+ mutex_unlock(&rng_mutex);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(hwrng_register);
+
+void hwrng_unregister(struct hwrng *rng)
+{
+ mutex_lock(&rng_mutex);
+
+ list_del(&rng->list);
+ if (current_rng == rng) {
+ if (rng->cleanup)
+ rng->cleanup(rng);
+ if (list_empty(&rng_list))
+ current_rng = NULL;
+ else
+ current_rng = list_entry(rng_list.prev, struct hwrng, list);
+ }
+ if (list_empty(&rng_list))
+ unregister_miscdev();
+
+ mutex_unlock(&rng_mutex);
+}
+EXPORT_SYMBOL_GPL(hwrng_unregister);
+
+
+MODULE_AUTHOR("The Linux Kernel team");
+MODULE_DESCRIPTION("H/W Random Number Generator (RNG) driver");
+MODULE_LICENSE("GPL");

--


2006-05-07 13:03:32

by Sergey Vlasov

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Sun, 07 May 2006 13:35:15 +0200 Michael Buesch wrote:

> Add a new generic H/W RNG core.
>
> Signed-off-by: Michael Buesch <[email protected]>
[skip]
> Index: hwrng/drivers/char/hw_random/core.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ hwrng/drivers/char/hw_random/core.c 2006-05-07 01:04:42.000000000 +0200
> @@ -0,0 +1,324 @@
> +/*
> + Added support for the AMD Geode LX RNG
> + (c) Copyright 2004-2005 Advanced Micro Devices, Inc.
> +
> + derived from
> +
> + Hardware driver for the Intel/AMD/VIA Random Number Generators (RNG)
> + (c) Copyright 2003 Red Hat Inc <[email protected]>
> +
> + derived from
> +
> + Hardware driver for the AMD 768 Random Number Generator (RNG)
> + (c) Copyright 2001 Red Hat Inc <[email protected]>
> +
> + derived from
> +
> + Hardware driver for Intel i810 Random Number Generator (RNG)
> + Copyright 2000,2001 Jeff Garzik <[email protected]>
> + Copyright 2000,2001 Philipp Rumpf <[email protected]>
> +
> + Added generic RNG API
> + Copyright 2006 Michael Buesch <[email protected]>
> + Copyright 2005 (c) MontaVista Software, Inc.
> +
> + Please read Documentation/hw_random.txt for details on use.
> +
> + ----------------------------------------------------------
> + This software may be used and distributed according to the terms
> + of the GNU General Public License, incorporated herein by reference.
> +
> + */
> +
> +
> +#include <linux/device.h>
> +#include <linux/hw_random.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/miscdevice.h>
> +#include <linux/delay.h>
> +#include <asm/uaccess.h>
> +
> +
> +#define RNG_MODULE_NAME "hw_random"
> +#define PFX RNG_MODULE_NAME ": "
> +#define RNG_MISCDEV_MINOR 183 /* official */
> +
> +
> +static struct hwrng *current_rng;
> +static LIST_HEAD(rng_list);
> +static DEFINE_MUTEX(rng_mutex);
> +
> +
> +static int rng_dev_open(struct inode *inode, struct file *filp)
> +{
> + /* enforce read-only access to this chrdev */
> + if ((filp->f_mode & FMODE_READ) == 0)
> + return -EINVAL;
> + if (filp->f_mode & FMODE_WRITE)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> + size_t size, loff_t *offp)
> +{
> + unsigned int have_data;
> + u32 data = 0;
> + ssize_t ret = 0;
> + int i, err;
> +
> + while (size) {
> + err = mutex_lock_interruptible(&rng_mutex);
> + if (err)
> + return err;

This does not handle the case of partial read correctly - the code
should be

return ret ? : -ERESTARTSYS;

> + if (!current_rng) {
> + mutex_unlock(&rng_mutex);
> + return -ENODEV;

The same problem here (although finding the RNG suddenly missing after
we heve just read something from it is pretty unlikely).

> + }
> + have_data = 0;
> + if (current_rng->data_present == NULL ||
> + current_rng->data_present(current_rng))
> + have_data = current_rng->data_read(current_rng, &data);
> + mutex_unlock(&rng_mutex);
> +
> + while (have_data && size) {
> + if (put_user((u8)data, buf++)) {
> + ret = ret ? : -EFAULT;
> + break;
> + }
> + size--;
> + ret++;
> + have_data--;
> + data>>=8;
> + }
> +
> + if (filp->f_flags & O_NONBLOCK)
> + return ret ? : -EAGAIN;
> +
> + if (need_resched()) {
> + schedule_timeout_interruptible(1);
> + } else {
> + err = mutex_lock_interruptible(&rng_mutex);
> + if (err)
> + return err;

And here...

> + if (!current_rng) {
> + mutex_unlock(&rng_mutex);
> + return -ENODEV;

And here too.

> + }
> + for (i = 0; i < 20; i++) {
> + if (current_rng->data_present == NULL ||
> + current_rng->data_present(current_rng))
> + break;
> + udelay(10);
> + }
> + mutex_unlock(&rng_mutex);
> + }
> +
> + if (signal_pending(current))
> + return ret ? : -ERESTARTSYS;
> + }
> + return ret;
> +}
> +
> +
> +static struct file_operations rng_chrdev_ops = {
> + .owner = THIS_MODULE,
> + .open = rng_dev_open,
> + .read = rng_dev_read,
> +};
> +
> +static struct miscdevice rng_miscdev = {
> + .minor = RNG_MISCDEV_MINOR,
> + .name = RNG_MODULE_NAME,
> + .fops = &rng_chrdev_ops,
> +};
> +
> +
> +static ssize_t hwrng_attr_current_store(struct class_device *class,
> + const char *buf, size_t len)
> +{
> + int err;
> + struct hwrng *rng;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + err = mutex_lock_interruptible(&rng_mutex);
> + if (err)
> + return err;
> + err = -ENODEV;
> + list_for_each_entry(rng, &rng_list, list) {
> + if (strncmp(rng->name, buf, len) == 0) {

This will match if the passed string is just a prefix of rng->name.
Apparently sysfs guarantees that the buffer passed to ->store will be
NUL-terminated, so this should be just a strcmp().

> + if (rng->init) {
> + err = rng->init(rng);
> + if (err)
> + break;
> + }
> + if (current_rng && current_rng->cleanup)
> + current_rng->cleanup(current_rng);

What if rng == current_rng here (someone has written the same RNG name
to the "store" attribute)? The lowlevel RNG driver should not have to
handle nested init/cleanup calls.

[skip]


Attachments:
(No filename) (5.11 kB)
(No filename) (190.00 B)
Download all attachments

2006-05-07 13:09:32

by Michael Büsch

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Sunday 07 May 2006 15:03, you wrote:
> This does not handle the case of partial read correctly - the code
> should be
>
> return ret ? : -ERESTARTSYS;
>
> > + if (!current_rng) {
> > + mutex_unlock(&rng_mutex);
> > + return -ENODEV;
>
> The same problem here (although finding the RNG suddenly missing after
> we heve just read something from it is pretty unlikely).
>
> > + }
> > + have_data = 0;
> > + if (current_rng->data_present == NULL ||
> > + current_rng->data_present(current_rng))
> > + have_data = current_rng->data_read(current_rng, &data);
> > + mutex_unlock(&rng_mutex);
> > +
> > + while (have_data && size) {
> > + if (put_user((u8)data, buf++)) {
> > + ret = ret ? : -EFAULT;
> > + break;
> > + }
> > + size--;
> > + ret++;
> > + have_data--;
> > + data>>=8;
> > + }
> > +
> > + if (filp->f_flags & O_NONBLOCK)
> > + return ret ? : -EAGAIN;
> > +
> > + if (need_resched()) {
> > + schedule_timeout_interruptible(1);
> > + } else {
> > + err = mutex_lock_interruptible(&rng_mutex);
> > + if (err)
> > + return err;
>
> And here...
>
> > + if (!current_rng) {
> > + mutex_unlock(&rng_mutex);
> > + return -ENODEV;
>
> And here too.

Whoops, will fix these.

> > + list_for_each_entry(rng, &rng_list, list) {
> > + if (strncmp(rng->name, buf, len) == 0) {
>
> This will match if the passed string is just a prefix of rng->name.
> Apparently sysfs guarantees that the buffer passed to ->store will be
> NUL-terminated, so this should be just a strcmp().

I am not sure if it is guaranteed NUL terminated. Greg?
But I will look into this.

> > + if (rng->init) {
> > + err = rng->init(rng);
> > + if (err)
> > + break;
> > + }
> > + if (current_rng && current_rng->cleanup)
> > + current_rng->cleanup(current_rng);
>
> What if rng == current_rng here (someone has written the same RNG name
> to the "store" attribute)? The lowlevel RNG driver should not have to
> handle nested init/cleanup calls.

I see. Will fix this.


I will also fix the bcm43xx patch. It registers always with the same "name".
That will blow up, if there is more than one bcm43xx device in the system.

--
Greetings Michael.

2006-05-07 13:21:03

by Michael Büsch

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Sunday 07 May 2006 15:16, you wrote:
> On Sunday 07 May 2006 15:03, you wrote:
> > This does not handle the case of partial read correctly - the code
> > should be
> >
> > return ret ? : -ERESTARTSYS;

Or, hm. Shouldn't we
return ret ? : err;

err is -EINTR

--
Greetings Michael.

2006-05-07 13:45:35

by Sergey Vlasov

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Sun, May 07, 2006 at 03:27:33PM +0200, Michael Buesch wrote:
> On Sunday 07 May 2006 15:16, you wrote:
> > On Sunday 07 May 2006 15:03, you wrote:
> > > This does not handle the case of partial read correctly - the code
> > > should be
> > >
> > > return ret ? : -ERESTARTSYS;
>
> Or, hm. Shouldn't we
> return ret ? : err;
>
> err is -EINTR

-ERESTARTSYS is the proper return code for this case - the signal
handling code will either convert it to EINTR for userspace, or
restart the system call after handling the signal, depending on the
state of the SA_RESTART flag set by sigaction().


Attachments:
(No filename) (599.00 B)
(No filename) (191.00 B)
Download all attachments

2006-05-07 13:48:57

by Michael Büsch

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Sunday 07 May 2006 15:45, you wrote:
> On Sun, May 07, 2006 at 03:27:33PM +0200, Michael Buesch wrote:
> > On Sunday 07 May 2006 15:16, you wrote:
> > > On Sunday 07 May 2006 15:03, you wrote:
> > > > This does not handle the case of partial read correctly - the code
> > > > should be
> > > >
> > > > return ret ? : -ERESTARTSYS;
> >
> > Or, hm. Shouldn't we
> > return ret ? : err;
> >
> > err is -EINTR
>
> -ERESTARTSYS is the proper return code for this case - the signal
> handling code will either convert it to EINTR for userspace, or
> restart the system call after handling the signal, depending on the
> state of the SA_RESTART flag set by sigaction().

Ok, I fixed all your stuff. Thanks.
I will run some more tests now and re-submit, if noone else finds a bug. ;)

--
Greetings Michael.

2006-05-10 21:03:19

by Greg KH

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Sun, May 07, 2006 at 03:16:08PM +0200, Michael Buesch wrote:
> On Sunday 07 May 2006 15:03, you wrote:
> > > + list_for_each_entry(rng, &rng_list, list) {
> > > + if (strncmp(rng->name, buf, len) == 0) {
> >
> > This will match if the passed string is just a prefix of rng->name.
> > Apparently sysfs guarantees that the buffer passed to ->store will be
> > NUL-terminated, so this should be just a strcmp().
>
> I am not sure if it is guaranteed NUL terminated. Greg?
> But I will look into this.

Yes it will be.

thanks,

greg k-h

2006-05-10 21:10:54

by Michael Büsch

[permalink] [raw]
Subject: Re: [patch 2/6] New Generic HW RNG

On Wednesday 10 May 2006 22:57, you wrote:
> On Sun, May 07, 2006 at 03:16:08PM +0200, Michael Buesch wrote:
> > On Sunday 07 May 2006 15:03, you wrote:
> > > > + list_for_each_entry(rng, &rng_list, list) {
> > > > + if (strncmp(rng->name, buf, len) == 0) {
> > >
> > > This will match if the passed string is just a prefix of rng->name.
> > > Apparently sysfs guarantees that the buffer passed to ->store will be
> > > NUL-terminated, so this should be just a strcmp().
> >
> > I am not sure if it is guaranteed NUL terminated. Greg?
> > But I will look into this.
>
> Yes it will be.

Ok, very nice.
I will resend the patches as soon as I have some time.
I also splitted up the x86 drivers. Looks very good and clean now. ;)

--
Greetings Michael.