2016-12-05 10:51:00

by Corentin Labbe

[permalink] [raw]
Subject: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

From: LABBE Corentin <[email protected]>

The Security System have a PRNG.
This patch add support for it as an hwrng.

Signed-off-by: Corentin Labbe <[email protected]>
---
Changes since v1:
- Replaced all spin_lock_bh by simple spin_lock
- Removed handling of size not modulo 4 which will never happen
- Added add_random_ready_callback()

drivers/crypto/Kconfig | 8 +++
drivers/crypto/sunxi-ss/Makefile | 1 +
drivers/crypto/sunxi-ss/sun4i-ss-core.c | 14 +++++
drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c | 99 ++++++++++++++++++++++++++++++++
drivers/crypto/sunxi-ss/sun4i-ss.h | 9 +++
5 files changed, 131 insertions(+)
create mode 100644 drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..38f7aca 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -538,6 +538,14 @@ config CRYPTO_DEV_SUN4I_SS
To compile this driver as a module, choose M here: the module
will be called sun4i-ss.

+config CRYPTO_DEV_SUN4I_SS_PRNG
+ bool "Support for Allwinner Security System PRNG"
+ depends on CRYPTO_DEV_SUN4I_SS
+ select HW_RANDOM
+ help
+ This driver provides kernel-side support for the Pseudo-Random
+ Number Generator found in the Security System.
+
config CRYPTO_DEV_ROCKCHIP
tristate "Rockchip's Cryptographic Engine driver"
depends on OF && ARCH_ROCKCHIP
diff --git a/drivers/crypto/sunxi-ss/Makefile b/drivers/crypto/sunxi-ss/Makefile
index 8f4c7a2..ca049ee 100644
--- a/drivers/crypto/sunxi-ss/Makefile
+++ b/drivers/crypto/sunxi-ss/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sun4i-ss.o
sun4i-ss-y += sun4i-ss-core.o sun4i-ss-hash.o sun4i-ss-cipher.o
+sun4i-ss-$(CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG) += sun4i-ss-hwrng.o
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index 3ac6c6c..fa739de 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -359,6 +359,16 @@ static int sun4i_ss_probe(struct platform_device *pdev)
}
}
platform_set_drvdata(pdev, ss);
+
+#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG
+ /* Voluntarily made the PRNG optional */
+ err = sun4i_ss_hwrng_register(&ss->hwrng);
+ if (!err)
+ dev_info(ss->dev, "sun4i-ss PRNG loaded");
+ else
+ dev_err(ss->dev, "sun4i-ss PRNG failed");
+#endif
+
return 0;
error_alg:
i--;
@@ -386,6 +396,10 @@ static int sun4i_ss_remove(struct platform_device *pdev)
int i;
struct sun4i_ss_ctx *ss = platform_get_drvdata(pdev);

+#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG
+ sun4i_ss_hwrng_remove(&ss->hwrng);
+#endif
+
for (i = 0; i < ARRAY_SIZE(ss_algs); i++) {
switch (ss_algs[i].type) {
case CRYPTO_ALG_TYPE_ABLKCIPHER:
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c b/drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c
new file mode 100644
index 0000000..8319cae
--- /dev/null
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c
@@ -0,0 +1,99 @@
+#include "sun4i-ss.h"
+
+static void sun4i_ss_seed(struct random_ready_callback *rdy)
+{
+ struct sun4i_ss_ctx *ss;
+
+ ss = container_of(rdy, struct sun4i_ss_ctx, random_ready);
+ get_random_bytes(ss->seed, SS_SEED_LEN);
+ ss->random_ready.func = NULL;
+}
+
+static int sun4i_ss_hwrng_init(struct hwrng *hwrng)
+{
+ struct sun4i_ss_ctx *ss;
+ int ret;
+
+ ss = container_of(hwrng, struct sun4i_ss_ctx, hwrng);
+
+ ss->random_ready.owner = THIS_MODULE;
+ ss->random_ready.func = sun4i_ss_seed;
+
+ ret = add_random_ready_callback(&ss->random_ready);
+ switch (ret) {
+ case 0:
+ break;
+ case -EALREADY:
+ get_random_bytes(ss->seed, SS_SEED_LEN);
+ ss->random_ready.func = NULL;
+ ret = 0;
+ break;
+ default:
+ ss->random_ready.func = NULL;
+ }
+
+ return ret;
+}
+
+static int sun4i_ss_hwrng_read(struct hwrng *hwrng, void *buf,
+ size_t max, bool wait)
+{
+ int i;
+ u32 v;
+ u32 *data = buf;
+ const u32 mode = SS_OP_PRNG | SS_PRNG_CONTINUE | SS_ENABLED;
+ size_t len;
+ struct sun4i_ss_ctx *ss;
+
+ ss = container_of(hwrng, struct sun4i_ss_ctx, hwrng);
+ len = min_t(size_t, SS_DATA_LEN, max);
+
+ /* If the PRNG is not seeded */
+ if (ss->random_ready.func)
+ return -EAGAIN;
+
+ spin_lock(&ss->slock);
+
+ writel(mode, ss->base + SS_CTL);
+
+ /* write the seed */
+ for (i = 0; i < SS_SEED_LEN / 4; i++)
+ writel(ss->seed[i], ss->base + SS_KEY0 + i * 4);
+ writel(mode | SS_PRNG_START, ss->base + SS_CTL);
+
+ /* Read the random data */
+ readsl(ss->base + SS_TXFIFO, data, len / 4);
+
+ /* Update the seed */
+ for (i = 0; i < SS_SEED_LEN / 4; i++) {
+ v = readl(ss->base + SS_KEY0 + i * 4);
+ ss->seed[i] = v;
+ }
+
+ writel(0, ss->base + SS_CTL);
+ spin_unlock(&ss->slock);
+ return len;
+}
+
+int sun4i_ss_hwrng_register(struct hwrng *hwrng)
+{
+ hwrng->name = "sun4i Security System PRNG";
+ hwrng->init = sun4i_ss_hwrng_init;
+ hwrng->read = sun4i_ss_hwrng_read;
+ hwrng->quality = 1000;
+
+ /* Cannot use devm_hwrng_register() since we need to hwrng_unregister
+ * before stopping clocks/regulator
+ */
+ return hwrng_register(hwrng);
+}
+
+void sun4i_ss_hwrng_remove(struct hwrng *hwrng)
+{
+ struct sun4i_ss_ctx *ss;
+
+ ss = container_of(hwrng, struct sun4i_ss_ctx, hwrng);
+ if (ss->random_ready.func)
+ del_random_ready_callback(&ss->random_ready);
+ hwrng_unregister(hwrng);
+}
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss.h b/drivers/crypto/sunxi-ss/sun4i-ss.h
index f04c0f8..85772d7 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss.h
+++ b/drivers/crypto/sunxi-ss/sun4i-ss.h
@@ -23,6 +23,7 @@
#include <linux/scatterlist.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
+#include <linux/hw_random.h>
#include <crypto/md5.h>
#include <crypto/sha.h>
#include <crypto/hash.h>
@@ -125,6 +126,9 @@
#define SS_RXFIFO_EMP_INT_ENABLE (1 << 2)
#define SS_TXFIFO_AVA_INT_ENABLE (1 << 0)

+#define SS_SEED_LEN (192 / 8)
+#define SS_DATA_LEN (160 / 8)
+
struct sun4i_ss_ctx {
void __iomem *base;
int irq;
@@ -134,6 +138,9 @@ struct sun4i_ss_ctx {
struct device *dev;
struct resource *res;
spinlock_t slock; /* control the use of the device */
+ struct random_ready_callback random_ready;
+ struct hwrng hwrng;
+ u32 seed[SS_SEED_LEN / 4];
};

struct sun4i_ss_alg_template {
@@ -199,3 +206,5 @@ int sun4i_ss_des_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
unsigned int keylen);
int sun4i_ss_des3_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
unsigned int keylen);
+int sun4i_ss_hwrng_register(struct hwrng *hwrng);
+void sun4i_ss_hwrng_remove(struct hwrng *hwrng);
--
2.7.3


2016-12-05 12:37:23

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Mon, Dec 05, 2016 at 11:48:42AM +0100, Corentin Labbe wrote:
> From: LABBE Corentin <[email protected]>
>
> The Security System have a PRNG.
> This patch add support for it as an hwrng.
>
> Signed-off-by: Corentin Labbe <[email protected]>

Please don't add PRNGs to hwrng. If we have existing PRNGs in
there please let me know so that we can remove them.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-12-05 12:57:47

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Mon, Dec 05, 2016 at 08:37:05PM +0800, Herbert Xu wrote:
> On Mon, Dec 05, 2016 at 11:48:42AM +0100, Corentin Labbe wrote:
> > From: LABBE Corentin <[email protected]>
> >
> > The Security System have a PRNG.
> > This patch add support for it as an hwrng.
> >
> > Signed-off-by: Corentin Labbe <[email protected]>
>
> Please don't add PRNGs to hwrng. If we have existing PRNGs in
> there please let me know so that we can remove them.
>

So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?

I found hisi-rng, crypto4xx_ and exynos-rng which seems to be PRNG used as hwrng.

Regards

2016-12-07 12:09:32

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
>
> So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?

We do have the algif_rng interface.

> I found hisi-rng, crypto4xx_ and exynos-rng which seems to be PRNG used as hwrng.

Thanks for checking. Patches to remove these are welcome.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-12-07 12:52:49

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Wed, Dec 07, 2016 at 08:09:00PM +0800, Herbert Xu wrote:
> On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
> >
> > So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?
>
> We do have the algif_rng interface.
>

So I must expose it as a crypto_rng ?

Could you explain why PRNG must not be used as hw_random ?

Regards
Corentin Labbe

2016-12-08 09:06:39

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Wed, Dec 07, 2016 at 01:51:27PM +0100, Corentin Labbe wrote:
>
> So I must expose it as a crypto_rng ?

If it is to be exposed at all then algif_rng would be the best
place.

> Could you explain why PRNG must not be used as hw_random ?

The hwrng interface was always meant to be an interface for real
hardware random number generators. People rely on that so we
should not provide bogus entropy sources through this interface.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-12-08 09:24:10

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Thu, Dec 08, 2016 at 05:06:18PM +0800, Herbert Xu wrote:
> On Wed, Dec 07, 2016 at 01:51:27PM +0100, Corentin Labbe wrote:
> >
> > So I must expose it as a crypto_rng ?
>
> If it is to be exposed at all then algif_rng would be the best
> place.
>

I have badly said my question.
So I need to use the HW PRNG in a crypto_rng "provider" that could be thereafter used from user space via algif_rng. right ?

> > Could you explain why PRNG must not be used as hw_random ?
>
> The hwrng interface was always meant to be an interface for real
> hardware random number generators. People rely on that so we
> should not provide bogus entropy sources through this interface.
>

Why not adding a KCONFIG HW_RANDOM_ACCEPT_ALSO_PRNG with big warning ?
Or a HW_PRNG Kconfig which do the same than hwrandom with /dev/prng ?
With that it will be much easier to convert in-tree PRNG that you want to remove.

Regards
Corentin Labbe

Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

>> The hwrng interface was always meant to be an interface for real
>> hardware random number generators. People rely on that so we
>> should not provide bogus entropy sources through this interface.
>>
>
> Why not adding a KCONFIG HW_RANDOM_ACCEPT_ALSO_PRNG with big warning ?
> Or a HW_PRNG Kconfig which do the same than hwrandom with /dev/prng ?
> With that it will be much easier to convert in-tree PRNG that you want to remove.

I do have driver for a PRNG that I was planning to post in sometime.
Upon seeing this thread I think the code has to be changed.

I completely agree with Corentin, adding /dev/prng or /dev/hw_prng
will make it easier to move existing code. It can be made explicit
that using new device will provide only pseudo random number.

Thanks,
PrasannaKumar

2016-12-13 14:11:06

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Thu, Dec 08, 2016 at 05:06:18PM +0800, Herbert Xu wrote:
> On Wed, Dec 07, 2016 at 01:51:27PM +0100, Corentin Labbe wrote:
> >
> > So I must expose it as a crypto_rng ?
>
> If it is to be exposed at all then algif_rng would be the best
> place.
>
> > Could you explain why PRNG must not be used as hw_random ?
>
> The hwrng interface was always meant to be an interface for real
> hardware random number generators. People rely on that so we
> should not provide bogus entropy sources through this interface.
>
> Cheers,

I have found two solutions:

The simplier is to add an attribute isprng to hwrng like that:
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -150,7 +150,8 @@ static int hwrng_init(struct hwrng *rng)
reinit_completion(&rng->cleanup_done);

skip_init:
- add_early_randomness(rng);
+ if (!rng->isprng)
+ add_early_randomness(rng);

current_quality = rng->quality ? : default_quality;
if (current_quality > 1024)
@@ -158,7 +159,7 @@ static int hwrng_init(struct hwrng *rng)

if (current_quality == 0 && hwrng_fill)
kthread_stop(hwrng_fill);
- if (current_quality > 0 && !hwrng_fill)
+ if (current_quality > 0 && !hwrng_fill && !rng->isprng)
start_khwrngd();

return 0;
@@ -439,7 +440,7 @@ int hwrng_register(struct hwrng *rng)
}
list_add_tail(&rng->list, &rng_list);

- if (old_rng && !rng->init) {
+ if (old_rng && !rng->init && !rng->isprng) {
/*
* Use a new device's input to add some randomness to
* the system. If this rng device isn't going to be
diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index 34a0dc1..5a5b8dc 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -50,6 +50,7 @@ struct hwrng {
struct list_head list;
struct kref ref;
struct completion cleanup_done;
+ bool isprng;
};


With that, we still could register prng, but they dont provide any entropy.
An optional Kconfig/"module parameter" could still be added for people still wanting this old behavour.


The other solution is to "duplicate" /dev/hwrng to /dev/hwprng like that:
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -24,8 +24,11 @@
#include <linux/uaccess.h>

#define RNG_MODULE_NAME "hw_random"
+#define PRNG_MODULE_NAME "hw_prng"
+#define HWPRNG_MINOR 185 /* not official */

static struct hwrng *current_rng;
+static struct hwrng *current_prng;
static struct task_struct *hwrng_fill;
static LIST_HEAD(rng_list);
/* Protects rng_list and current_rng */
@@ -44,7 +47,7 @@ module_param(default_quality, ushort, 0644);
MODULE_PARM_DESC(default_quality,
"default entropy content of hwrng per mill");

-static void drop_current_rng(void);
+static void drop_current_rng(bool prng);
static int hwrng_init(struct hwrng *rng);
static void start_khwrngd(void);

@@ -56,6 +59,14 @@ static size_t rng_buffer_size(void)
return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
}

+static bool is_current_xrng(struct hwrng *rng)
+{
+ if (rng->isprng)
+ return (rng == current_prng);
+ else
+ return (rng == current_rng);
+}
+
static void add_early_randomness(struct hwrng *rng)
{
int bytes_read;
@@ -88,32 +99,46 @@ static int set_current_rng(struct hwrng *rng)
if (err)
return err;

- drop_current_rng();
- current_rng = rng;
+ drop_current_rng(rng->isprng);
+ if (rng->isprng)
+ current_prng = rng;
+ else
+ current_rng = rng;

return 0;
}

-static void drop_current_rng(void)
+static void drop_current_rng(bool prng)
{
BUG_ON(!mutex_is_locked(&rng_mutex));
- if (!current_rng)
- return;
-
- /* decrease last reference for triggering the cleanup */
- kref_put(&current_rng->ref, cleanup_rng);
- current_rng = NULL;
+ if (prng) {
+ if (!current_prng)
+ return;
+ /* decrease last reference for triggering the cleanup */
+ kref_put(&current_prng->ref, cleanup_rng);
+ current_prng = NULL;
+ } else {
+ if (!current_rng)
+ return;
+ /* decrease last reference for triggering the cleanup */
+ kref_put(&current_rng->ref, cleanup_rng);
+ current_rng = NULL;
+ }
}

/* Returns ERR_PTR(), NULL or refcounted hwrng */
-static struct hwrng *get_current_rng(void)
+static struct hwrng *get_current_rng(bool prng)
{
struct hwrng *rng;

if (mutex_lock_interruptible(&rng_mutex))
return ERR_PTR(-ERESTARTSYS);

- rng = current_rng;
+ if (prng)
+ rng = current_prng;
+ else
+ rng = current_rng;
+
if (rng)
kref_get(&rng->ref);

@@ -193,8 +218,8 @@ static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
return 0;
}

-static ssize_t rng_dev_read(struct file *filp, char __user *buf,
- size_t size, loff_t *offp)
+static ssize_t genrng_dev_read(struct file *filp, char __user *buf,
+ size_t size, loff_t *offp, bool prng)
{
ssize_t ret = 0;
int err = 0;
@@ -202,7 +227,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
struct hwrng *rng;

while (size) {
- rng = get_current_rng();
+ rng = get_current_rng(prng);
if (IS_ERR(rng)) {
err = PTR_ERR(rng);
goto out;
@@ -270,6 +295,18 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
goto out;
}

+static ssize_t rng_dev_read(struct file *filp, char __user *buf,
+ size_t size, loff_t *offp)
+{
+ return genrng_dev_read(filp, buf, size, offp, false);
+}
+
+static ssize_t prng_dev_read(struct file *filp, char __user *buf,
+ size_t size, loff_t *offp)
+{
+ return genrng_dev_read(filp, buf, size, offp, true);
+}
+
static const struct file_operations rng_chrdev_ops = {
.owner = THIS_MODULE,
.open = rng_dev_open,
@@ -278,6 +315,7 @@ static const struct file_operations rng_chrdev_ops = {
};

static const struct attribute_group *rng_dev_groups[];
+static const struct attribute_group *prng_dev_groups[];

static struct miscdevice rng_miscdev = {
.minor = HWRNG_MINOR,
@@ -287,9 +325,24 @@ static struct miscdevice rng_miscdev = {
.groups = rng_dev_groups,
};

-static ssize_t hwrng_attr_current_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
+static const struct file_operations prng_chrdev_ops = {
+ .owner = THIS_MODULE,
+ .open = rng_dev_open,
+ .read = prng_dev_read,
+ .llseek = noop_llseek,
+};
+
+static struct miscdevice prng_miscdev = {
+ .minor = HWPRNG_MINOR,
+ .name = PRNG_MODULE_NAME,
+ .nodename = "hwprng",
+ .fops = &prng_chrdev_ops,
+ .groups = prng_dev_groups,
+};
+
+static ssize_t genrng_attr_current_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len, bool prng)
{
int err;
struct hwrng *rng;
@@ -301,8 +354,13 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
list_for_each_entry(rng, &rng_list, list) {
if (sysfs_streq(rng->name, buf)) {
err = 0;
- if (rng != current_rng)
- err = set_current_rng(rng);
+ if (prng) {
+ if (rng != current_prng)
+ err = set_current_rng(rng);
+ } else {
+ if (rng != current_rng)
+ err = set_current_rng(rng);
+ }
break;
}
}
@@ -311,14 +369,28 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
return err ? : len;
}

-static ssize_t hwrng_attr_current_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+static ssize_t hwrng_attr_current_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return genrng_attr_current_store(dev, attr, buf, len, false);
+}
+
+static ssize_t hwprng_attr_current_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return genrng_attr_current_store(dev, attr, buf, len, true);
+}
+
+static ssize_t genrng_attr_current_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf, bool prng)
{
ssize_t ret;
struct hwrng *rng;

- rng = get_current_rng();
+ rng = get_current_rng(prng);
if (IS_ERR(rng))
return PTR_ERR(rng);

@@ -328,9 +400,24 @@ static ssize_t hwrng_attr_current_show(struct device *dev,
return ret;
}

-static ssize_t hwrng_attr_available_show(struct device *dev,
+static ssize_t hwrng_attr_current_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return genrng_attr_current_show(dev, attr, buf, false);
+}
+
+static ssize_t hwprng_attr_current_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return genrng_attr_current_show(dev, attr, buf, true);
+}
+
+
+static ssize_t hwgenrng_attr_available_show(struct device *dev,
struct device_attribute *attr,
- char *buf)
+ char *buf, bool prng)
{
int err;
struct hwrng *rng;
@@ -340,8 +427,10 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
return -ERESTARTSYS;
buf[0] = '\0';
list_for_each_entry(rng, &rng_list, list) {
- strlcat(buf, rng->name, PAGE_SIZE);
- strlcat(buf, " ", PAGE_SIZE);
+ if (rng->isprng == prng) {
+ strlcat(buf, rng->name, PAGE_SIZE);
+ strlcat(buf, " ", PAGE_SIZE);
+ }
}
strlcat(buf, "\n", PAGE_SIZE);
mutex_unlock(&rng_mutex);
@@ -349,6 +438,20 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
return strlen(buf);
}

+static ssize_t hwrng_attr_available_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return hwgenrng_attr_available_show(dev, attr, buf, false);
+}
+
+static ssize_t hwprng_attr_available_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return hwgenrng_attr_available_show(dev, attr, buf, true);
+}
+
static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR,
hwrng_attr_current_show,
hwrng_attr_current_store);
@@ -356,6 +459,13 @@ static DEVICE_ATTR(rng_available, S_IRUGO,
hwrng_attr_available_show,
NULL);

+static DEVICE_ATTR(prng_current, S_IRUGO | S_IWUSR,
+ hwprng_attr_current_show,
+ hwprng_attr_current_store);
+static DEVICE_ATTR(prng_available, S_IRUGO,
+ hwprng_attr_available_show,
+ NULL);
+
static struct attribute *rng_dev_attrs[] = {
&dev_attr_rng_current.attr,
&dev_attr_rng_available.attr,
@@ -364,14 +474,35 @@ static struct attribute *rng_dev_attrs[] = {

ATTRIBUTE_GROUPS(rng_dev);

+static struct attribute *prng_dev_attrs[] = {
+ &dev_attr_prng_current.attr,
+ &dev_attr_prng_available.attr,
+ NULL
+};
+
+ATTRIBUTE_GROUPS(prng_dev);
+
static void __exit unregister_miscdev(void)
{
misc_deregister(&rng_miscdev);
+ misc_deregister(&prng_miscdev);
}

static int __init register_miscdev(void)
{
- return misc_register(&rng_miscdev);
+ int err;
+
+ err = misc_register(&rng_miscdev);
+ if (err)
+ return err;
+ err = misc_register(&prng_miscdev);
+ if (err)
+ goto reg_error;
+ else
+ return 0;
+reg_error:
+ misc_deregister(&rng_miscdev);
+ return err;
}

static int hwrng_fillfn(void *unused)
@@ -381,7 +512,7 @@ static int hwrng_fillfn(void *unused)
while (!kthread_should_stop()) {
struct hwrng *rng;

- rng = get_current_rng();
+ rng = get_current_rng(false);
if (IS_ERR(rng) || !rng)
break;
mutex_lock(&reading_mutex);
@@ -462,8 +593,8 @@ void hwrng_unregister(struct hwrng *rng)
mutex_lock(&rng_mutex);

list_del(&rng->list);
- if (current_rng == rng) {
- drop_current_rng();
+ if (is_current_xrng(rng)) {
+ drop_current_rng(rng->isprng);
if (!list_empty(&rng_list)) {
struct hwrng *tail;

@@ -553,7 +684,8 @@ static int __init hwrng_modinit(void)
static void __exit hwrng_modexit(void)
{
mutex_lock(&rng_mutex);
- BUG_ON(current_rng);
+ WARN_ON(current_rng);
+ WARN_ON(current_prng);
kfree(rng_buffer);
kfree(rng_fillbuf);
mutex_unlock(&rng_mutex);
diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index 34a0dc1..5a5b8dc 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -50,6 +50,7 @@ struct hwrng {
struct list_head list;
struct kref ref;
struct completion cleanup_done;
+ bool isprng;
};

What do you think about those two solutions ?

Regards
Corentin Labbe

Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

> What do you think about those two solutions ?

I prefer the second solution's idea of using two files (/dev/hwrng and
/dev/hwprng). Upon having a quick glance it looks like (based on
current_rng == prng check) that your current implementation allows
only one rng device to be in use at a time. It would be better to have
both usable at the same time. So applications that need pseudo random
data at high speed can use /dev/prng while applications that require
true random number can use /dev/rng. Please feel free to correct if my
understanding of the code is incorrect. Along with this change I think
changing the algif_rng to use this code if this solution is going to
be used.

Regards,
PrasannaKumar

2016-12-13 15:34:00

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Tue, Dec 13, 2016 at 08:53:54PM +0530, PrasannaKumar Muralidharan wrote:
> > What do you think about those two solutions ?
>
> I prefer the second solution's idea of using two files (/dev/hwrng and
> /dev/hwprng). Upon having a quick glance it looks like (based on
> current_rng == prng check) that your current implementation allows
> only one rng device to be in use at a time. It would be better to have
> both usable at the same time. So applications that need pseudo random
> data at high speed can use /dev/prng while applications that require
> true random number can use /dev/rng. Please feel free to correct if my
> understanding of the code is incorrect. Along with this change I think
> changing the algif_rng to use this code if this solution is going to
> be used.
>

No, there could be both device at the same time.

2016-12-14 05:06:21

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Tue, Dec 13, 2016 at 03:10:59PM +0100, Corentin Labbe wrote:
>
> I have found two solutions:

No we already have algif_rng so let's not confuse things even
further by making hwrng take PRNGs.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-12-14 06:04:17

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Wed, Dec 14, 2016 at 01:05:51PM +0800, Herbert Xu wrote:
> On Tue, Dec 13, 2016 at 03:10:59PM +0100, Corentin Labbe wrote:
> >
> > I have found two solutions:
>
> No we already have algif_rng so let's not confuse things even
> further by making hwrng take PRNGs.
>

But algif_rng is not accessible from user space without any coding.
So no easy "random" data with some cat /dev/xxxx.

Clearly users of the 3 already intree hw_random PRNG will see that like a regresion.

Regards
Corentin Labbe

Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

>> I have found two solutions:
>
> No we already have algif_rng so let's not confuse things even
> further by making hwrng take PRNGs.

Even if both the solutions could not be adopted I think there must be
a way for applications to use similar API to get true rng or prng.
Given the case that no user complained about prng data when using
/dev/hwrng is it safe to assume that the random data generated is
acceptable for users? If so, the drivers can be left without any
modification.

Should there be a mandate that driver will be accepted only when it
passes 'rngtest'. This will make sure that prng drivers won't get
added in future.

Regards,
PrasannaKumar

2016-12-15 06:09:22

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG

On Thu, Dec 15, 2016 at 12:47:16AM +0530, PrasannaKumar Muralidharan wrote:
> Should there be a mandate that driver will be accepted only when it
> passes 'rngtest'. This will make sure that prng drivers won't get
> added in future.

You cannot use software to distinguish between a PRNG and an HRNG.
We can only rely on the veracity of the documentation.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt