Skip to contentSkip to conversation

crypto: hw_random - Add new Exynos RNG driver

PATCH Superseded Krzysztof Kozlowski 2 participants 7 messages Mar 24, 2017 – Mar 25, 2017

Subsystems (4)
Arm (32-bit) Crypto API Arm Samsung Exynos Hardware random number generator core

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Krzysztof Kozlowski

author maintainer
#0 lore raw

v2 cover crypto: hw_random - Add new Exynos RNG driver

Hi,

This is a follow up of my questions around exynos-rng [1].

Changes since v1:
=================
1. Re-work the code for seeding after system resume, following suggestions and review by Stephan M?ller.
The resume path was not tested.

2. Re-seed itself from time to time (every 100 ms), suggested by Stephan M?ller.

3. Use a define for retries (Bartlomiej Zolnierkiewicz).
4. Add some docs.

Description:
============
The existing exynos-rng has many issues. The most important one is that it is a pseudo RNG device but uses hw_random interface which does not allow proper seeding.

The RNG module on Exynos4 requires seeding. On newer SoCs (like Exynos5420) it can seed itself from a true RNG. Converting the existing driver to use TRNG would effectively drop support for Exynos4 and break compatibility with existing users.

Instead I decided to convert it to crypto API. In the future I hope to add support for seeding from TRNG module.

Tested with app [2].

Patches are independent. I will take the defconfig changes (2/3 and 3/3) through samsung-soc tree.

Best regards,
Krzysztof

[1] https://www.spinics.net/lists/arm-kernel/msg569641.html
[2] https://www.spinics.net/lists/arm-kernel/msg571184.html

Krzysztof Kozlowski (3):
crypto: hw_random - Add new Exynos RNG driver
ARM: exynos_defconfig: Enable Exynos RNG and user-space crypto API
ARM: multi_v7_defconfig: Enable Exynos RNG and user-space crypto API

 MAINTAINERS                         |   8 +
 arch/arm/configs/exynos_defconfig   |   6 +
 arch/arm/configs/multi_v7_defconfig |   6 +
 drivers/char/hw_random/Kconfig      |  14 --
 drivers/char/hw_random/Makefile     |   1 -
 drivers/char/hw_random/exynos-rng.c | 231 ---------------------
 drivers/crypto/Kconfig              |  15 ++
 drivers/crypto/Makefile             |   1 +
 drivers/crypto/exynos-rng.c         | 392 ++++++++++++++++++++++++++++++++++++
 9 files changed, 428 insertions(+), 246 deletions(-)
 delete mode 100644 drivers/char/hw_random/exynos-rng.c
 create mode 100644 drivers/crypto/exynos-rng.c
Signature · 1 line
2.9.3
author maintainer
#1 lore raw

PATCH v2 1/3 crypto: hw_random - Add new Exynos RNG driver

Replace existing hw_ranndom/exynos-rng driver with a new, reworked one. This is a driver for pseudo random number generator block which on Exynos4 chipsets must be seeded with some value. On newer Exynos5420 chipsets it might seed itself from true random number generator block but this is not implemented yet.

New driver is a complete rework to use the crypto ALGAPI instead of hw_random API. Rationale for the change:
1. hw_random interface is for true RNG devices.
2. The old driver was seeding itself with jiffies which is not a reliable source for randomness.
3. Device generates five random numbers in each pass but old driver was returning only one thus its performance was reduced.

Compatibility with DeviceTree bindings is preserved.

New driver does not use runtime power management but manually enables and disables the clock when needed. This is preferred approach because using runtime PM just to toggle clock is huge overhead.

Another difference is reseeding itself with generated random data periodically and during resuming from system suspend (previously driver was re-seeding itself again with jiffies).

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

The resume path is untested as my Odroid U3 fails to go online. Testing on Trats2 or other devices would be appreciated.

---
 MAINTAINERS                         |   8 +
 drivers/char/hw_random/Kconfig      |  14 --
 drivers/char/hw_random/Makefile     |   1 -
 drivers/char/hw_random/exynos-rng.c | 231 ---------------------
 drivers/crypto/Kconfig              |  15 ++
 drivers/crypto/Makefile             |   1 +
 drivers/crypto/exynos-rng.c         | 392 ++++++++++++++++++++++++++++++++++++
 7 files changed, 416 insertions(+), 246 deletions(-)
 delete mode 100644 drivers/char/hw_random/exynos-rng.c
 create mode 100644 drivers/crypto/exynos-rng.c
MAINTAINERS +8 -0
index affecc6d59f4..371fda859d43 100644
@@ -10977,6 +10977,14 @@ L:	alsa-devel at alsa-project.org (moderated for non-subscribers)
 S:	Supported
 F:	sound/soc/samsung/
 
+SAMSUNG EXYNOS PSEUDO RANDOM NUMBER GENERATOR (RNG) DRIVER
+M:	Krzysztof Kozlowski <[email protected]>
+L:	linux-crypto at vger.kernel.org
+L:	linux-samsung-soc at vger.kernel.org
+S:	Maintained
+F:	drivers/crypto/exynos-rng.c
+F:	Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
+
 SAMSUNG FRAMEBUFFER DRIVER
 M:	Jingoo Han <[email protected]>
 L:	linux-fbdev at vger.kernel.org
drivers/char/hw_random/Kconfig +0 -14
index 0cafe08919c9..bdae802e7154 100644
@@ -294,20 +294,6 @@ config HW_RANDOM_POWERNV
 
 	  If unsure, say Y.
 
-config HW_RANDOM_EXYNOS
-	tristate "EXYNOS HW random number generator support"
-	depends on ARCH_EXYNOS || COMPILE_TEST
-	depends on HAS_IOMEM
-	default HW_RANDOM
-	---help---
-	  This driver provides kernel-side support for the Random Number
-	  Generator hardware found on EXYNOS SOCs.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called exynos-rng.
-
-	  If unsure, say Y.
-
 config HW_RANDOM_TPM
 	tristate "TPM HW Random Number Generator support"
 	depends on TCG_TPM
drivers/char/hw_random/Makefile +0 -1
index 5f52b1e4e7be..6f1eecc2045c 100644
@@ -24,7 +24,6 @@ obj-$(CONFIG_HW_RANDOM_OCTEON) += octeon-rng.o
 obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
 obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
 obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
-obj-$(CONFIG_HW_RANDOM_EXYNOS)	+= exynos-rng.o
 obj-$(CONFIG_HW_RANDOM_HISI)	+= hisi-rng.o
 obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
 obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
drivers/char/hw_random/exynos-rng.c deleted +0 -231

231 lines not on this page · raw message

drivers/crypto/Kconfig +15 -0
index 1a60626937e4..7d1fdf6a751c 100644
@@ -388,6 +388,21 @@ config CRYPTO_DEV_MXC_SCC
 	  This option enables support for the Security Controller (SCC)
 	  found in Freescale i.MX25 chips.
 
+config CRYPTO_DEV_EXYNOS_RNG
+	tristate "EXYNOS HW pseudo random number generator support"
+	depends on ARCH_EXYNOS || COMPILE_TEST
+	depends on HAS_IOMEM
+	select CRYPTO_RNG
+	---help---
+	  This driver provides kernel-side support through the
+	  cryptographic API for the pseudo random number generator hardware
+	  found on Exynos SoCs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called exynos-rng.
+
+	  If unsure, say Y.
+
 config CRYPTO_DEV_S5P
 	tristate "Support for Samsung S5PV210/Exynos crypto accelerator"
 	depends on ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
drivers/crypto/Makefile +1 -0
index 41ca339e89d3..9603f1862b30 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += cavium/
 obj-$(CONFIG_CRYPTO_DEV_CCP) += ccp/
 obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
 obj-$(CONFIG_CRYPTO_DEV_CPT) += cavium/cpt/
+obj-$(CONFIG_CRYPTO_DEV_EXYNOS_RNG) += exynos-rng.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam/
 obj-$(CONFIG_CRYPTO_DEV_GEODE) += geode-aes.o
 obj-$(CONFIG_CRYPTO_DEV_HIFN_795X) += hifn_795x.o
drivers/crypto/exynos-rng.c new file +392 -0

392 lines not on this page · raw message

Signature · 1 line
2.9.3
#2 lore raw

Am Freitag, 24. M?rz 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

Quoted 7 lines
drivers/crypto/exynos-rng.c new file +7 -0
+static unsigned int exynos_rng_copy_random(struct exynos_rng_dev *rng,
+					   u8 *dst, unsigned int dlen)
+{
+	unsigned int cnt = 0;
+	int i, j;
+	u32 val;
+

drivers/crypto/exynos-rng.c line 157

Quoted 10 lines
drivers/crypto/exynos-rng.c new file +10 -0
+	for (j = 0; j < EXYNOS_RNG_SEED_REGS; j++) {
+		val = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
+
+		for (i = 0; i < 4; i++) {
+			dst[cnt] = val & 0xff;
+			val >>= 8;
+			if (++cnt >= dlen)
+				return cnt;
+		}
+		rng->seed_save[j] = val;

Just to clarify: is this call right? Shouldn't that be removed? Any RNG that is given to a caller is tainted and should not serve as seed.

drivers/crypto/exynos-rng.c line 165

Quoted 8 lines
drivers/crypto/exynos-rng.c new file +8 -0
+	}
+
+	/*
+	 * Engine filled all output registers, so read the remaining registers
+	 * for storing data as future seed.
+	 */
+	for (; j < EXYNOS_RNG_SEED_REGS; j++)
+		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));

With this call, I guess the questioned line above could go away, right?

Ciao
Stephan

author maintainer
#3 lore raw
Quoted 16 lines · Stephan M?ller

Am Freitag, 24. M?rz 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

Quoted 12 lines
drivers/crypto/exynos-rng.c new file +12 -0
+static unsigned int exynos_rng_copy_random(struct exynos_rng_dev *rng,
+					   u8 *dst, unsigned int dlen)
+{
+	unsigned int cnt = 0;
+	int i, j;
+	u32 val;
+
+	for (j = 0; j < EXYNOS_RNG_SEED_REGS; j++) {
+		val = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
+
+		for (i = 0; i < 4; i++) {
+			dst[cnt] = val & 0xff;
Quoted 8 lines

drivers/crypto/exynos-rng.c line 157

Quoted 5 lines
drivers/crypto/exynos-rng.c new file +5 -0
+			val >>= 8;
+			if (++cnt >= dlen)
+				return cnt;
+		}
+		rng->seed_save[j] = val;

Just to clarify: is this call right? Shouldn't that be removed? Any RNG that is given to a caller is tainted and should not serve as seed.

In that case I could either re-use RNGs not passed to the caller (like in the block quoted below) or generate another round of them just for purpose of next seeding.

With the first approach the problem is that I might wait for such unused numbers pretty long. If user is requesting large amount of data, then I will always give him all five output numbers. I will not have unused numbers.

The second approach seems safe, but requires additional engine run which will slow down some of the generate() calls.

Quoted 10 lines

drivers/crypto/exynos-rng.c line 165

Quoted 8 lines
drivers/crypto/exynos-rng.c new file +8 -0
+	}
+
+	/*
+	 * Engine filled all output registers, so read the remaining registers
+	 * for storing data as future seed.
+	 */
+	for (; j < EXYNOS_RNG_SEED_REGS; j++)
+		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));

With this call, I guess the questioned line above could go away, right?

This is used in combination with the previous line so I will get five seeds (for five registers).

Best regards,
Krzysztof

#4 lore raw

Am Samstag, 25. M?rz 2017, 08:36:48 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

Quoted 30 lines · Stephan M?ller
Quoted 16 lines

Am Freitag, 24. M?rz 2017, 19:26:04 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,

Quoted 12 lines
drivers/crypto/exynos-rng.c new file +12 -0
+static unsigned int exynos_rng_copy_random(struct exynos_rng_dev *rng,
+					   u8 *dst, unsigned int dlen)
+{
+	unsigned int cnt = 0;
+	int i, j;
+	u32 val;
+
+	for (j = 0; j < EXYNOS_RNG_SEED_REGS; j++) {
+		val = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));
+
+		for (i = 0; i < 4; i++) {
+			dst[cnt] = val & 0xff;
Quoted 8 lines

drivers/crypto/exynos-rng.c line 157

Quoted 5 lines
drivers/crypto/exynos-rng.c new file +5 -0
+			val >>= 8;
+			if (++cnt >= dlen)
+				return cnt;
+		}
+		rng->seed_save[j] = val;

Just to clarify: is this call right? Shouldn't that be removed? Any RNG that is given to a caller is tainted and should not serve as seed.

In that case I could either re-use RNGs not passed to the caller (like in the block quoted below) or generate another round of them just for purpose of next seeding.

Quoted 7 lines

With the first approach the problem is that I might wait for such unused numbers pretty long. If user is requesting large amount of data, then I will always give him all five output numbers. I will not have unused numbers.

The second approach seems safe, but requires additional engine run which will slow down some of the generate() calls.

Random numbers should never be used twice.

Quoted 12 lines
Quoted 11 lines

drivers/crypto/exynos-rng.c line 165

Quoted 9 lines
drivers/crypto/exynos-rng.c new file +8 -0
+	}
+
+	/*
+	 * Engine filled all output registers, so read the remaining registers
+	 * for storing data as future seed.
+	 */
+	for (; j < EXYNOS_RNG_SEED_REGS; j++)
+		rng->seed_save[j] = exynos_rng_readl(rng, EXYNOS_RNG_OUT(j));

With this call, I guess the questioned line above could go away, right?

Quoted 5 lines

This is used in combination with the previous line so I will get five seeds (for five registers).

Best regards,
Krzysztof

Ciao
Stephan

author maintainer
#5 lore raw

PATCH v2 2/3 ARM: exynos_defconfig: Enable Exynos RNG and user-space crypto API

In reply to Krzysztof Kozlowski

Enable the new Exynos pseudo random number generator driver and user-space API to access crypto algorithms and devices (not only RNG).

Signed-off-by Krzysztof Kozlowski <[email protected]>
---
 arch/arm/configs/exynos_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)
arch/arm/configs/exynos_defconfig +6 -0
index 6dc661c4a2c1..960d55445e05 100644
@@ -265,6 +265,12 @@ CONFIG_DEBUG_RT_MUTEXES=y
 CONFIG_DEBUG_SPINLOCK=y
 CONFIG_DEBUG_MUTEXES=y
 CONFIG_DEBUG_USER=y
+CONFIG_CRYPTO_USER=m
+CONFIG_CRYPTO_USER_API_HASH=m
+CONFIG_CRYPTO_USER_API_SKCIPHER=m
+CONFIG_CRYPTO_USER_API_RNG=m
+CONFIG_CRYPTO_USER_API_AEAD=m
+CONFIG_CRYPTO_DEV_EXYNOS_RNG=y
 CONFIG_CRYPTO_DEV_S5P=y
 CONFIG_ARM_CRYPTO=y
 CONFIG_CRYPTO_SHA1_ARM_NEON=m
Signature · 1 line
2.9.3
author maintainer
#6 lore raw

PATCH v2 3/3 ARM: multi_v7_defconfig: Enable Exynos RNG and user-space crypto API

In reply to Krzysztof Kozlowski

Enable the new Exynos pseudo random number generator driver and user-space API to access crypto algorithms and devices (not only RNG).

Signed-off-by Krzysztof Kozlowski <[email protected]>
---
 arch/arm/configs/multi_v7_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)
arch/arm/configs/multi_v7_defconfig +6 -0
index 36c1b39cfb54..9bc1bd7f2afa 100644
@@ -935,7 +935,13 @@ CONFIG_CPUFREQ_DT=y
 CONFIG_KEYSTONE_IRQ=y
 CONFIG_HW_RANDOM=y
 CONFIG_HW_RANDOM_ST=y
+CONFIG_CRYPTO_USER=m
+CONFIG_CRYPTO_USER_API_HASH=m
+CONFIG_CRYPTO_USER_API_SKCIPHER=m
+CONFIG_CRYPTO_USER_API_RNG=m
+CONFIG_CRYPTO_USER_API_AEAD=m
 CONFIG_CRYPTO_DEV_MARVELL_CESA=m
+CONFIG_CRYPTO_DEV_EXYNOS_RNG=m
 CONFIG_CRYPTO_DEV_S5P=m
 CONFIG_CRYPTO_DEV_SUN4I_SS=m
 CONFIG_CRYPTO_DEV_ROCKCHIP=m
Signature · 1 line
2.9.3