Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH -next] hwrng: amd - Fix return value check in mod_init()
From: PrasannaKumar Muralidharan @ 2016-09-16 13:13 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Matt Mackall, Herbert Xu, Corentin LABBE, Wei Yongjun,
	linux-crypto
In-Reply-To: <1473990581-18602-1-git-send-email-weiyj.lk@gmail.com>

> In case of error, the function devm_kzalloc() or devm_ioport_map()
> return NULL pointer not ERR_PTR(). The IS_ERR() test in the return
> value check should be replaced with NULL test.
>
> Fixes: 31b2a73c9c5f ("hwrng: amd - Migrate to managed API")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/char/hw_random/amd-rng.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
> index 4dbc5aa..4a99ac7 100644
> --- a/drivers/char/hw_random/amd-rng.c
> +++ b/drivers/char/hw_random/amd-rng.c
> @@ -149,8 +149,8 @@ static int __init mod_init(void)
>                 return -EIO;
>
>         priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> -       if (IS_ERR(priv))
> -               return PTR_ERR(priv);
> +       if (!priv)
> +               return -ENOMEM;
>
>         if (!devm_request_region(&pdev->dev, pmbase + PMBASE_OFFSET,
>                                 PMBASE_SIZE, DRV_NAME)) {
> @@ -161,9 +161,9 @@ static int __init mod_init(void)
>
>         priv->iobase = devm_ioport_map(&pdev->dev, pmbase + PMBASE_OFFSET,
>                         PMBASE_SIZE);
> -       if (IS_ERR(priv->iobase)) {
> +       if (!priv->iobase) {
>                 pr_err(DRV_NAME "Cannot map ioport\n");
> -               return PTR_ERR(priv->iobase);
> +               return -ENOMEM;
>         }
>
>         amd_rng.priv = (unsigned long)priv;
>

My change introduced this issue. Thanks for fixing it.

^ permalink raw reply

* Re: [PATCH v3 0/8] Add support for SafeXcel IP-76 to OMAP RNG
From: Romain Perier @ 2016-09-16 12:52 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Nadav Haklai, Omri Itach,
	Shadi Ammouri, Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits,
	Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

Hello,

Le 16/09/2016 12:08, Romain Perier a écrit :
> The driver omap-rng has a lot of similarity with the IP block SafeXcel
> IP-76. A lot of registers are the same and the way that the driver works
> is very closed the description of the TRNG EIP76 in its datasheet.
>
> This series refactorize the driver, add support for generating bigger
> output random data and add a device variant for SafeXcel IP-76, found
> in Armada 8K.
>
> Romain Perier (8):
>    dt-bindings: Add vendor prefix for INSIDE Secure
>    dt-bindings: omap-rng: Document SafeXcel IP-76 device variant
>    hwrng: omap - Switch to non-obsolete read API implementation
>    hwrng: omap - Remove global definition of hwrng
>    hwrng: omap - Add support for 128-bit output of data
>    hwrng: omap - Don't prefix the probe message with OMAP
>    hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K
>    arm64: dts: marvell: add TRNG description for Armada 8K CP
>
>   Documentation/devicetree/bindings/rng/omap_rng.txt |  14 +-
>   .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>   .../boot/dts/marvell/armada-cp110-master.dtsi      |   8 +
>   .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi |   8 +
>   drivers/char/hw_random/Kconfig                     |   2 +-
>   drivers/char/hw_random/omap-rng.c                  | 162 ++++++++++++++++-----
>   6 files changed, 152 insertions(+), 43 deletions(-)
>

If possible, I would like to get "Tested-by" tags by the omap guys 
before this set of patches is merged, to be sure that there are no 
regressions for the OMAP variant.

Thanks,
Regards,
-- 
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] crypto: gcm - Fix IV buffer size in crypto_gcm_setkey
From: Ondrej Mosnáček @ 2016-09-16 12:07 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto

The cipher block size for GCM is 16 bytes, and thus the CTR transform
used in crypto_gcm_setkey() will also expect a 16-byte IV. However,
the code currently reserves only 8 bytes for the IV, causing
an out-of-bounds access in the CTR transform. This patch fixes
the issue by setting the size of the IV buffer to 16 bytes.

Fixes: 84c911523020 ("[CRYPTO] gcm: Add support for async ciphers")
Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
---
I randomly noticed this while going over igcm.c for an unrelated
reason. It seems the wrong buffer size never caused any noticeable
problems (it's been there since 2007), but it should be corrected
nonetheless...

 crypto/gcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/gcm.c b/crypto/gcm.c
index 70a892e8..f624ac9 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -117,7 +117,7 @@ static int crypto_gcm_setkey(struct crypto_aead
*aead, const u8 *key,
 	struct crypto_skcipher *ctr = ctx->ctr;
 	struct {
 		be128 hash;
-		u8 iv[8];
+		u8 iv[16];

 		struct crypto_gcm_setkey_result result;

-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 7/8] hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 6241 bytes --]

This commits adds a device variant for Safexcel,EIP76 found in Marvell
Armada 8k. It defines registers mapping with the good offset and add a
specific initialization function.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---

Changes in v2:
 - Call pm_runtime_put_sync from the label err_register. When there is an
   EPROBE_DEFER, strange errors can happen because the call to pm_runtime_*
   is not well balanced.

 drivers/char/hw_random/Kconfig    |  2 +-
 drivers/char/hw_random/omap-rng.c | 86 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 56ad5a5..aea3613 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -168,7 +168,7 @@ config HW_RANDOM_IXP4XX
 
 config HW_RANDOM_OMAP
 	tristate "OMAP Random Number Generator support"
-	depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS
+	depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS || ARCH_MVEBU
 	default HW_RANDOM
  	---help---
  	  This driver provides kernel-side support for the Random Number
diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index bbbce16..43ee8b3 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/interrupt.h>
+#include <linux/clk.h>
 
 #include <asm/io.h>
 
@@ -63,6 +64,7 @@
 
 #define OMAP2_RNG_OUTPUT_SIZE			0x4
 #define OMAP4_RNG_OUTPUT_SIZE			0x8
+#define EIP76_RNG_OUTPUT_SIZE			0x10
 
 enum {
 	RNG_OUTPUT_0_REG = 0,
@@ -108,6 +110,23 @@ static const u16 reg_map_omap4[] = {
 	[RNG_SYSCONFIG_REG]	= 0x1FE4,
 };
 
+static const u16 reg_map_eip76[] = {
+	[RNG_OUTPUT_0_REG]	= 0x0,
+	[RNG_OUTPUT_1_REG]	= 0x4,
+	[RNG_OUTPUT_2_REG]	= 0x8,
+	[RNG_OUTPUT_3_REG]	= 0xc,
+	[RNG_STATUS_REG]	= 0x10,
+	[RNG_INTACK_REG]	= 0x10,
+	[RNG_CONTROL_REG]	= 0x14,
+	[RNG_CONFIG_REG]	= 0x18,
+	[RNG_ALARMCNT_REG]	= 0x1c,
+	[RNG_FROENABLE_REG]	= 0x20,
+	[RNG_FRODETUNE_REG]	= 0x24,
+	[RNG_ALARMMASK_REG]	= 0x28,
+	[RNG_ALARMSTOP_REG]	= 0x2c,
+	[RNG_REV_REG]		= 0x7c,
+};
+
 struct omap_rng_dev;
 /**
  * struct omap_rng_pdata - RNG IP block-specific data
@@ -130,6 +149,7 @@ struct omap_rng_dev {
 	struct device			*dev;
 	const struct omap_rng_pdata	*pdata;
 	struct hwrng rng;
+	struct clk 			*clk;
 };
 
 static inline u32 omap_rng_read(struct omap_rng_dev *priv, u16 reg)
@@ -221,6 +241,38 @@ static inline u32 omap4_rng_data_present(struct omap_rng_dev *priv)
 	return omap_rng_read(priv, RNG_STATUS_REG) & RNG_REG_STATUS_RDY;
 }
 
+static int eip76_rng_init(struct omap_rng_dev *priv)
+{
+	u32 val;
+
+	/* Return if RNG is already running. */
+	if (omap_rng_read(priv, RNG_CONTROL_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
+		return 0;
+
+	/*  Number of 512 bit blocks of raw Noise Source output data that must
+	 *  be processed by either the Conditioning Function or the
+	 *  SP 800-90 DRBG ‘BC_DF’ functionality to yield a ‘full entropy’
+	 *  output value.
+	 */
+	val = 0x5 << RNG_CONFIG_MIN_REFIL_CYCLES_SHIFT;
+
+	/* Number of FRO samples that are XOR-ed together into one bit to be
+	 * shifted into the main shift register
+	 */
+	val |= RNG_CONFIG_MAX_REFIL_CYCLES << RNG_CONFIG_MAX_REFIL_CYCLES_SHIFT;
+	omap_rng_write(priv, RNG_CONFIG_REG, val);
+
+	/* Enable all available FROs */
+	omap_rng_write(priv, RNG_FRODETUNE_REG, 0x0);
+	omap_rng_write(priv, RNG_FROENABLE_REG, RNG_REG_FROENABLE_MASK);
+
+	/* Enable TRNG */
+	val = RNG_CONTROL_ENABLE_TRNG_MASK;
+	omap_rng_write(priv, RNG_CONTROL_REG, val);
+
+	return 0;
+}
+
 static int omap4_rng_init(struct omap_rng_dev *priv)
 {
 	u32 val;
@@ -290,6 +342,14 @@ static struct omap_rng_pdata omap4_rng_pdata = {
 	.cleanup	= omap4_rng_cleanup,
 };
 
+static struct omap_rng_pdata eip76_rng_pdata = {
+	.regs		= (u16 *)reg_map_eip76,
+	.data_size	= EIP76_RNG_OUTPUT_SIZE,
+	.data_present	= omap4_rng_data_present,
+	.init		= eip76_rng_init,
+	.cleanup	= omap4_rng_cleanup,
+};
+
 static const struct of_device_id omap_rng_of_match[] = {
 		{
 			.compatible	= "ti,omap2-rng",
@@ -299,6 +359,10 @@ static const struct of_device_id omap_rng_of_match[] = {
 			.compatible	= "ti,omap4-rng",
 			.data		= &omap4_rng_pdata,
 		},
+		{
+			.compatible	= "inside-secure,safexcel-eip76",
+			.data		= &eip76_rng_pdata,
+		},
 		{},
 };
 MODULE_DEVICE_TABLE(of, omap_rng_of_match);
@@ -317,7 +381,8 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 	}
 	priv->pdata = match->data;
 
-	if (of_device_is_compatible(dev->of_node, "ti,omap4-rng")) {
+	if (of_device_is_compatible(dev->of_node, "ti,omap4-rng") ||
+	    of_device_is_compatible(dev->of_node, "inside-secure,safexcel-eip76")) {
 		irq = platform_get_irq(pdev, 0);
 		if (irq < 0) {
 			dev_err(dev, "%s: error getting IRQ resource - %d\n",
@@ -333,6 +398,16 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 			return err;
 		}
 		omap_rng_write(priv, RNG_INTMASK_REG, RNG_SHUTDOWN_OFLO_MASK);
+
+		priv->clk = of_clk_get(pdev->dev.of_node, 0);
+		if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		if (!IS_ERR(priv->clk)) {
+			err = clk_prepare_enable(priv->clk);
+			if (err)
+				dev_err(&pdev->dev, "unable to enable the clk, "
+						    "err = %d\n", err);
+		}
 	}
 	return 0;
 }
@@ -394,7 +469,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	ret = (dev->of_node) ? of_get_omap_rng_device_details(priv, pdev) :
 				get_omap_rng_device_details(priv);
 	if (ret)
-		goto err_ioremap;
+		goto err_register;
 
 	ret = hwrng_register(&priv->rng);
 	if (ret)
@@ -407,7 +482,11 @@ static int omap_rng_probe(struct platform_device *pdev)
 
 err_register:
 	priv->base = NULL;
+	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
 err_ioremap:
 	dev_err(dev, "initialization failed.\n");
 	return ret;
@@ -424,6 +503,9 @@ static int omap_rng_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
+
 	return 0;
 }
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 8/8] arm64: dts: marvell: add TRNG description for Armada 8K CP
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

This commits adds the devicetree description of the SafeXcel IP-76 TRNG
found in the two Armada CP110.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 8 ++++++++
 arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi  | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
index da31bbb..aaffa24 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
@@ -164,6 +164,14 @@
 				clocks = <&cpm_syscon0 1 21>;
 				status = "disabled";
 			};
+
+			cpm_trng: trng@760000 {
+				compatible = "marvell,armada-8k-rng", "inside-secure,safexcel-eip76";
+				reg = <0x760000 0x7d>;
+				interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&cpm_syscon0 1 25>;
+				status = "okay";
+			};
 		};
 
 		cpm_pcie0: pcie@f2600000 {
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index 6ff1201..216de12 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -164,6 +164,14 @@
 				clocks = <&cps_syscon0 1 21>;
 				status = "disabled";
 			};
+
+			cps_trng: trng@760000 {
+				compatible = "marvell,armada-8k-rng", "inside-secure,safexcel-eip76";
+				reg = <0x760000 0x7d>;
+				interrupts = <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&cps_syscon0 1 25>;
+				status = "okay";
+			};
 		};
 
 		cps_pcie0: pcie@f4600000 {
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 6/8] hwrng: omap - Don't prefix the probe message with OMAP
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

So far, this driver was only used for OMAP SoCs. However, if a device
variant is added for an IP block that has nothing to do with the OMAP
platform, the message "OMAP Random Number Generator Ver" is displayed
anyway. Instead of hardcoding "OMAP" into this message, we decide to
only display "Random Number Generator". As dev_info is already
pre-pending the message with the name of the device, we have enough
informations.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index a84ab49..bbbce16 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -400,7 +400,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_register;
 
-	dev_info(&pdev->dev, "OMAP Random Number Generator ver. %02x\n",
+	dev_info(&pdev->dev, "Random Number Generator ver. %02x\n",
 		 omap_rng_read(priv, RNG_REV_REG));
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 5/8] hwrng: omap - Add support for 128-bit output of data
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

So far, this driver only supports up to 64 bits of output data generated
by an RNG. Some IP blocks, like the SafeXcel IP-76 supports up to 128
bits of output data. This commits renames registers descriptions
OUTPUT_L_REG and OUTPUT_H_REG to OUTPUT_0_REG and OUPUT_1_REG,
respectively. It also adds two new values to the enumeration of existing
registers: OUTPUT_2_REG and OUTPUT_3_REG.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 5ea804d..a84ab49 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -65,8 +65,10 @@
 #define OMAP4_RNG_OUTPUT_SIZE			0x8
 
 enum {
-	RNG_OUTPUT_L_REG = 0,
-	RNG_OUTPUT_H_REG,
+	RNG_OUTPUT_0_REG = 0,
+	RNG_OUTPUT_1_REG,
+	RNG_OUTPUT_2_REG,
+	RNG_OUTPUT_3_REG,
 	RNG_STATUS_REG,
 	RNG_INTMASK_REG,
 	RNG_INTACK_REG,
@@ -82,7 +84,7 @@ enum {
 };
 
 static const u16 reg_map_omap2[] = {
-	[RNG_OUTPUT_L_REG]	= 0x0,
+	[RNG_OUTPUT_0_REG]	= 0x0,
 	[RNG_STATUS_REG]	= 0x4,
 	[RNG_CONFIG_REG]	= 0x28,
 	[RNG_REV_REG]		= 0x3c,
@@ -90,8 +92,8 @@ static const u16 reg_map_omap2[] = {
 };
 
 static const u16 reg_map_omap4[] = {
-	[RNG_OUTPUT_L_REG]	= 0x0,
-	[RNG_OUTPUT_H_REG]	= 0x4,
+	[RNG_OUTPUT_0_REG]	= 0x0,
+	[RNG_OUTPUT_1_REG]	= 0x4,
 	[RNG_STATUS_REG]	= 0x8,
 	[RNG_INTMASK_REG]	= 0xc,
 	[RNG_INTACK_REG]	= 0x10,
@@ -163,7 +165,7 @@ static int omap_rng_do_read(struct hwrng *rng, void *data, size_t max,
 	if (!present)
 		return 0;
 
-	memcpy_fromio(data, priv->base + priv->pdata->regs[RNG_OUTPUT_L_REG],
+	memcpy_fromio(data, priv->base + priv->pdata->regs[RNG_OUTPUT_0_REG],
 		      priv->pdata->data_size);
 
 	if (priv->pdata->regs[RNG_INTACK_REG])
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 4/8] hwrng: omap - Remove global definition of hwrng
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

The omap-rng driver currently assumes that there will only ever be a
single instance of an RNG device. For this reason, there is a statically
allocated struct hwrng, with a fixed name. However, registering two
struct hwrng with the same isn't accepted by the RNG framework, so we
need to switch to a dynamically allocated struct hwrng, each using a
different name. Then, we define the name of this hwrng to "dev_name(dev)",
so the name of the data structure is unique per device.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---

Changes in v2:
 - Fix the goto label used when there is an error for devm_kstrdup

 drivers/char/hw_random/omap-rng.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index b3f6047..5ea804d 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -127,6 +127,7 @@ struct omap_rng_dev {
 	void __iomem			*base;
 	struct device			*dev;
 	const struct omap_rng_pdata	*pdata;
+	struct hwrng rng;
 };
 
 static inline u32 omap_rng_read(struct omap_rng_dev *priv, u16 reg)
@@ -187,12 +188,6 @@ static void omap_rng_cleanup(struct hwrng *rng)
 	priv->pdata->cleanup(priv);
 }
 
-static struct hwrng omap_rng_ops = {
-	.name		= "omap",
-	.read 		= omap_rng_do_read,
-	.init		= omap_rng_init,
-	.cleanup	= omap_rng_cleanup,
-};
 
 static inline u32 omap2_rng_data_present(struct omap_rng_dev *priv)
 {
@@ -365,7 +360,11 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	omap_rng_ops.priv = (unsigned long)priv;
+	priv->rng.read = omap_rng_do_read;
+	priv->rng.init = omap_rng_init;
+	priv->rng.cleanup = omap_rng_cleanup;
+
+	priv->rng.priv = (unsigned long)priv;
 	platform_set_drvdata(pdev, priv);
 	priv->dev = dev;
 
@@ -376,6 +375,12 @@ static int omap_rng_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
+	priv->rng.name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
+	if (!priv->rng.name) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret) {
@@ -389,7 +394,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_ioremap;
 
-	ret = hwrng_register(&omap_rng_ops);
+	ret = hwrng_register(&priv->rng);
 	if (ret)
 		goto err_register;
 
@@ -410,7 +415,7 @@ static int omap_rng_remove(struct platform_device *pdev)
 {
 	struct omap_rng_dev *priv = platform_get_drvdata(pdev);
 
-	hwrng_unregister(&omap_rng_ops);
+	hwrng_unregister(&priv->rng);
 
 	priv->pdata->cleanup(priv);
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 3/8] hwrng: omap - Switch to non-obsolete read API implementation
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

The ".data_present" and ".data_read" operations are marked as OBSOLETE
in the hwrng API. We have to use the ".read" operation instead. It makes
the driver simpler and moves the busy loop, that waits until enough data
is generated, to the read function. We simplify this step by only
checking the status of the engine, if there is data, we copy the data to
the output buffer and the amout of copied data is returned to the caller,
otherwise zero is returned. The hwrng core will re-call the read operation
as many times as required until enough data has been copied.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---

Changes in v3:
 - Re-add code for busy loop that waits until enough data is generated. When no
   data is available, the busy loop is tried several time until the function
   returns to the hw_random core and then schedule_timeout_interruptible(1) is
   called. in v2, schedule_timeout_interruptible(1) was called each time data
   was not available, which added more latency.

 drivers/char/hw_random/omap-rng.c | 41 ++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 01d4be2..b3f6047 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -140,41 +140,35 @@ static inline void omap_rng_write(struct omap_rng_dev *priv, u16 reg,
 	__raw_writel(val, priv->base + priv->pdata->regs[reg]);
 }
 
-static int omap_rng_data_present(struct hwrng *rng, int wait)
+
+static int omap_rng_do_read(struct hwrng *rng, void *data, size_t max,
+			    bool wait)
 {
 	struct omap_rng_dev *priv;
-	int data, i;
+	int i, present;
 
 	priv = (struct omap_rng_dev *)rng->priv;
 
+	if (max < priv->pdata->data_size)
+		return 0;
+
 	for (i = 0; i < 20; i++) {
-		data = priv->pdata->data_present(priv);
-		if (data || !wait)
+		present = priv->pdata->data_present(priv);
+		if (present || !wait)
 			break;
-		/* RNG produces data fast enough (2+ MBit/sec, even
-		 * during "rngtest" loads, that these delays don't
-		 * seem to trigger.  We *could* use the RNG IRQ, but
-		 * that'd be higher overhead ... so why bother?
-		 */
+
 		udelay(10);
 	}
-	return data;
-}
-
-static int omap_rng_data_read(struct hwrng *rng, u32 *data)
-{
-	struct omap_rng_dev *priv;
-	u32 data_size, i;
-
-	priv = (struct omap_rng_dev *)rng->priv;
-	data_size = priv->pdata->data_size;
+	if (!present)
+		return 0;
 
-	for (i = 0; i < data_size / sizeof(u32); i++)
-		data[i] = omap_rng_read(priv, RNG_OUTPUT_L_REG + i);
+	memcpy_fromio(data, priv->base + priv->pdata->regs[RNG_OUTPUT_L_REG],
+		      priv->pdata->data_size);
 
 	if (priv->pdata->regs[RNG_INTACK_REG])
 		omap_rng_write(priv, RNG_INTACK_REG, RNG_REG_INTACK_RDY_MASK);
-	return data_size;
+
+	return priv->pdata->data_size;
 }
 
 static int omap_rng_init(struct hwrng *rng)
@@ -195,8 +189,7 @@ static void omap_rng_cleanup(struct hwrng *rng)
 
 static struct hwrng omap_rng_ops = {
 	.name		= "omap",
-	.data_present	= omap_rng_data_present,
-	.data_read	= omap_rng_data_read,
+	.read 		= omap_rng_do_read,
 	.init		= omap_rng_init,
 	.cleanup	= omap_rng_cleanup,
 };
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 2/8] dt-bindings: omap-rng: Document SafeXcel IP-76 device variant
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

This commits add missing fields in the documentation that are used
by the new device variant. It also includes DT example to show how
the variant should be used.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 Documentation/devicetree/bindings/rng/omap_rng.txt | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/omap_rng.txt b/Documentation/devicetree/bindings/rng/omap_rng.txt
index 6a62acd..4714772 100644
--- a/Documentation/devicetree/bindings/rng/omap_rng.txt
+++ b/Documentation/devicetree/bindings/rng/omap_rng.txt
@@ -1,4 +1,4 @@
-OMAP SoC HWRNG Module
+OMAP SoC and Inside-Secure HWRNG Module
 
 Required properties:
 
@@ -6,11 +6,13 @@ Required properties:
   RNG versions:
   - "ti,omap2-rng" for OMAP2.
   - "ti,omap4-rng" for OMAP4, OMAP5 and AM33XX.
+  - "inside-secure,safexcel-eip76" for SoCs with EIP76 IP block
   Note that these two versions are incompatible.
 - ti,hwmods: Name of the hwmod associated with the RNG module
 - reg : Offset and length of the register set for the module
 - interrupts : the interrupt number for the RNG module.
-		Only used for "ti,omap4-rng".
+		Used for "ti,omap4-rng" and "inside-secure,safexcel-eip76"
+- clocks: the trng clock source
 
 Example:
 /* AM335x */
@@ -20,3 +22,11 @@ rng: rng@48310000 {
 	reg = <0x48310000 0x2000>;
 	interrupts = <111>;
 };
+
+/* SafeXcel IP-76 */
+trng: rng@f2760000 {
+	compatible = "inside-secure,safexcel-eip76";
+	reg = <0xf2760000 0x7d>;
+	interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&cpm_syscon0 1 25>;
+};
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 1/8] dt-bindings: Add vendor prefix for INSIDE Secure
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160916100856.31727-1-romain.perier@free-electrons.com>

This commits adds a vendor for the company INSIDE Secure.
See https://www.insidesecure.com, for more details.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 1992aa9..6a5e872 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -132,6 +132,7 @@ infineon Infineon Technologies
 inforce	Inforce Computing
 ingenic	Ingenic Semiconductor
 innolux	Innolux Corporation
+inside-secure	INSIDE Secure
 intel	Intel Corporation
 intercontrol	Inter Control Group
 invensense	InvenSense Inc.
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 0/8] Add support for SafeXcel IP-76 to OMAP RNG
From: Romain Perier @ 2016-09-16 10:08 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

The driver omap-rng has a lot of similarity with the IP block SafeXcel
IP-76. A lot of registers are the same and the way that the driver works
is very closed the description of the TRNG EIP76 in its datasheet.

This series refactorize the driver, add support for generating bigger
output random data and add a device variant for SafeXcel IP-76, found
in Armada 8K.

Romain Perier (8):
  dt-bindings: Add vendor prefix for INSIDE Secure
  dt-bindings: omap-rng: Document SafeXcel IP-76 device variant
  hwrng: omap - Switch to non-obsolete read API implementation
  hwrng: omap - Remove global definition of hwrng
  hwrng: omap - Add support for 128-bit output of data
  hwrng: omap - Don't prefix the probe message with OMAP
  hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K
  arm64: dts: marvell: add TRNG description for Armada 8K CP

 Documentation/devicetree/bindings/rng/omap_rng.txt |  14 +-
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 .../boot/dts/marvell/armada-cp110-master.dtsi      |   8 +
 .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi |   8 +
 drivers/char/hw_random/Kconfig                     |   2 +-
 drivers/char/hw_random/omap-rng.c                  | 162 ++++++++++++++++-----
 6 files changed, 152 insertions(+), 43 deletions(-)

-- 
2.9.3

^ permalink raw reply

* Re: [PATCH v2 3/8] hwrng: omap - Switch to non-obsolete read API implementation
From: Romain Perier @ 2016-09-16 10:11 UTC (permalink / raw)
  To: Herbert Xu
  Cc: dsaxena, mpm, Gregory Clement, Thomas Petazzoni, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto
In-Reply-To: <20160913094825.GA30645@gondor.apana.org.au>

Hi,

Le 13/09/2016 11:48, Herbert Xu a écrit :
> On Wed, Sep 07, 2016 at 05:57:38PM +0200, Romain Perier wrote:
>> +
>> +static int omap_rng_do_read(struct hwrng *rng, void *data, size_t max,
>> +			    bool wait)
>>   {
>>   	struct omap_rng_dev *priv;
>> -	int data, i;
>>
>>   	priv = (struct omap_rng_dev *)rng->priv;
>>
>> -	for (i = 0; i < 20; i++) {
>> -		data = priv->pdata->data_present(priv);
>> -		if (data || !wait)
>> -			break;
>> -		/* RNG produces data fast enough (2+ MBit/sec, even
>> -		 * during "rngtest" loads, that these delays don't
>> -		 * seem to trigger.  We *could* use the RNG IRQ, but
>> -		 * that'd be higher overhead ... so why bother?
>> -		 */
>> -		udelay(10);
>
> So in the wait case you're changing the driver's behaviour.  Instead
> of waiting for 1us you'll now wait for 1s if there is no data.  Is
> this what really what you want?
>
> Cheers,
>

Mhhh, you're right, in this specific case it will add more latency... 
with busy loop, on average, 20 retries will be enough to have data...

Thanks!
-- 
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] crypto: caam - fix sg dump
From: Catalin Vasile @ 2016-09-16  9:06 UTC (permalink / raw)
  To: linux-crypto
  Cc: linux-crypto-owner, horia.geanta, alexandru.porosanu,
	Catalin Vasile

Ensure scatterlists have a virtual memory mapping before dumping.

Signed-off-by: Catalin Vasile <cata.vasile@nxp.com>
---
 drivers/crypto/caam/caamalg.c | 65 +++++++++++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 6dc5971..49f1ea7 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -111,6 +111,41 @@
 #else
 #define debug(format, arg...)
 #endif
+
+#ifdef DEBUG
+#include <linux/highmem.h>
+
+static void dbg_dump_sg(const char *level, const char *prefix_str,
+			int prefix_type, int rowsize, int groupsize,
+			struct scatterlist *sg, size_t tlen, bool ascii)
+{
+	struct scatterlist *it;
+	size_t len;
+	void *buf;
+
+	for (it = sg; it != NULL && tlen > 0 ; it = sg_next(sg)) {
+		/*
+		 * make sure the scatterlist's page
+		 * has a valid virtual memory mapping
+		 */
+		buf = kmap(sg_page(it));
+
+		len = min(tlen, it->length);
+		print_hex_dump(level, prefix_str, prefix_type, rowsize,
+			       groupsize, sg_virt(it), len, ascii);
+		tlen -= len;
+
+		kunmap(sg_page(it));
+	}
+}
+#else
+static void dbg_dump_sg(const char *level, const char *prefix_str,
+			int prefix_type, int rowsize, int groupsize,
+			struct scatterlist *sg, size_t tlen, bool ascii)
+{
+}
+#endif
+
 static struct list_head alg_list;
 
 struct caam_alg_entry {
@@ -1984,9 +2019,9 @@ static void ablkcipher_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
 	print_hex_dump(KERN_ERR, "dstiv  @"__stringify(__LINE__)": ",
 		       DUMP_PREFIX_ADDRESS, 16, 4, req->info,
 		       edesc->src_nents > 1 ? 100 : ivsize, 1);
-	print_hex_dump(KERN_ERR, "dst    @"__stringify(__LINE__)": ",
-		       DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
-		       edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
+	dbg_dump_sg(KERN_ERR, "dst    @"__stringify(__LINE__)": ",
+		    DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
+		    edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
 #endif
 
 	ablkcipher_unmap(jrdev, edesc, req);
@@ -2016,9 +2051,9 @@ static void ablkcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
 	print_hex_dump(KERN_ERR, "dstiv  @"__stringify(__LINE__)": ",
 		       DUMP_PREFIX_ADDRESS, 16, 4, req->info,
 		       ivsize, 1);
-	print_hex_dump(KERN_ERR, "dst    @"__stringify(__LINE__)": ",
-		       DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
-		       edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
+	dbg_dump_sg(KERN_ERR, "dst    @"__stringify(__LINE__)": ",
+		    DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
+		    edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
 #endif
 
 	ablkcipher_unmap(jrdev, edesc, req);
@@ -2176,9 +2211,9 @@ static void init_ablkcipher_job(u32 *sh_desc, dma_addr_t ptr,
 	print_hex_dump(KERN_ERR, "presciv@"__stringify(__LINE__)": ",
 		       DUMP_PREFIX_ADDRESS, 16, 4, req->info,
 		       ivsize, 1);
-	print_hex_dump(KERN_ERR, "src    @"__stringify(__LINE__)": ",
-		       DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
-		       edesc->src_nents ? 100 : req->nbytes, 1);
+	dbg_dump_sg(KERN_ERR, "src    @"__stringify(__LINE__)": ",
+		    DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+		    edesc->src_nents ? 100 : req->nbytes, 1);
 #endif
 
 	len = desc_len(sh_desc);
@@ -2233,9 +2268,9 @@ static void init_ablkcipher_giv_job(u32 *sh_desc, dma_addr_t ptr,
 	print_hex_dump(KERN_ERR, "presciv@" __stringify(__LINE__) ": ",
 		       DUMP_PREFIX_ADDRESS, 16, 4, req->info,
 		       ivsize, 1);
-	print_hex_dump(KERN_ERR, "src    @" __stringify(__LINE__) ": ",
-		       DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
-		       edesc->src_nents ? 100 : req->nbytes, 1);
+	dbg_dump_sg(KERN_ERR, "src    @" __stringify(__LINE__) ": ",
+		    DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+		    edesc->src_nents ? 100 : req->nbytes, 1);
 #endif
 
 	len = desc_len(sh_desc);
@@ -2512,9 +2547,9 @@ static int aead_decrypt(struct aead_request *req)
 		return PTR_ERR(edesc);
 
 #ifdef DEBUG
-	print_hex_dump(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
-		       DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
-		       req->assoclen + req->cryptlen, 1);
+	dbg_dump_sg(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
+		    DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+		    req->assoclen + req->cryptlen, 1);
 #endif
 
 	/* Create and submit job descriptor*/
-- 
1.8.3.1

^ permalink raw reply related

* Re: [RFC] revamp fips_allowed flag
From: Herbert Xu @ 2016-09-16  5:56 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto
In-Reply-To: <9465267.bxih8SxCui@tauon.atsec.com>

On Fri, Sep 16, 2016 at 07:42:13AM +0200, Stephan Mueller wrote:
>
> > For such templates we could move that info into the generic
> > template implementation code and have them declare themselves
> > as such that for any X if X is FIPS allowed then so is T(X).
> > 
> > This info can then be used in testmgr.
> 
> I can see that, but I do not believe it makes our life in the testmgr easier. 
> The reason is that in a lot of cases, the template parts (read: block chaining 
> modes) are moved into implementations. The most notable examples are the x86 
> Intel and the S390 CPACF implementations. For those, the template handling 
> code is not triggered and thus we still would need testmgr entries with the 
> block chaining modes.

That's not what I meant.  The template is only responsible for
providing that info to testmgr.  The actual rule will then apply
to any cra_name which matches it.  That is, if template T says
that it is OK under FIPS, then every algorithm of type T(X) will
be allowed if X is allowed.

> Considering that we now have RSA in the kernel and that we could expect RSA 
> implementations in hardware (in fact, we already have them), there is another 
> complication: FIPS only allows RSA according to FIPS 186-4 and not according 
> to FIPS 186-2 (ok, the main difference is in key gen, which we do not have). 
> What I want to say here is that with the more complex style ciphers, it may 
> very well be possible that only the respective implementation knows whether it 
> is FIPS compliant.

Please give a concrete example.

> Thus, I am still thinking that moving the fips_allowed flag into the structs 
> that are evaluated during register time is more helpful. I definitely see that 
> this would imply that all, say, AES implementations need that flag. But IMHO 
> it is a much cleaner solution, because if the register is rejected, the cipher 
> does not show up in /proc/crypto and everybody knows that a particular cipher 
> is not in use.

That's crazy.  For AES we already have dozens of implementations,
think about the amount of churn that will occur when one AES
implementation is obsoleted by FIPS.

> My particular example is that in one test with FIPS mode enabled, 
> seqiv(rfc4106(gcm-base(...))) was successfully loaded and marked as selftest 
> passed in /proc/crypto. Yet, an allocation of the test failed with ENOENT. 
> Rebooting the system without FIPS mode allowed me to allocate the cipher. As 
> there is no test for this particular cipher name in testmgr, I highly suspect 
> that the allocation code did not find it because somehow there was no test. In 
> any case, it is definitely not clear why that particular cipher name cannot be 
> allocated in FIPS mode given the entries in /proc/crypto.

Sounds like our testmgr dealing with FIPS is buggy.  It probably
shouldn't mark it as passed.

Cheers,
-- 
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

* Re: [RFC] revamp fips_allowed flag
From: Stephan Mueller @ 2016-09-16  5:42 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20160915062629.GA14950@gondor.apana.org.au>

Am Donnerstag, 15. September 2016, 14:26:29 CEST schrieb Herbert Xu:

Hi Herbert,

> On Thu, Sep 15, 2016 at 08:23:05AM +0200, Stephan Mueller wrote:
> > Where shall we draw the line here? Shall that be only for authenc, or
> > seqiv? Or shall we also consider rfc4106 too, knowing that there are
> > implementations which provide a full rfc4106 GCM combo (x86 for example).
> > What about the current pkcspad1 template where we could expect that there
> > may be entire HW implementations with that?
> 
> That's something that only you can tell us :)

Well, as a baseline we can say  that:

- with the exception of a few block chaining modes (e.g. xcbc, pcbc, lrw) all 
templates are allowed in FIPS mode

- almost all "non-approved" ciphers (i.e those that should not have a 
fips_allowed flag) are due to the raw base cipher (i.e. only SHA1/2, AES and 
TDES is allowed)

- all compression algos are allowed
> 
> For such templates we could move that info into the generic
> template implementation code and have them declare themselves
> as such that for any X if X is FIPS allowed then so is T(X).
> 
> This info can then be used in testmgr.

I can see that, but I do not believe it makes our life in the testmgr easier. 
The reason is that in a lot of cases, the template parts (read: block chaining 
modes) are moved into implementations. The most notable examples are the x86 
Intel and the S390 CPACF implementations. For those, the template handling 
code is not triggered and thus we still would need testmgr entries with the 
block chaining modes.

Considering that we now have RSA in the kernel and that we could expect RSA 
implementations in hardware (in fact, we already have them), there is another 
complication: FIPS only allows RSA according to FIPS 186-4 and not according 
to FIPS 186-2 (ok, the main difference is in key gen, which we do not have). 
What I want to say here is that with the more complex style ciphers, it may 
very well be possible that only the respective implementation knows whether it 
is FIPS compliant.

Thus, I am still thinking that moving the fips_allowed flag into the structs 
that are evaluated during register time is more helpful. I definitely see that 
this would imply that all, say, AES implementations need that flag. But IMHO 
it is a much cleaner solution, because if the register is rejected, the cipher 
does not show up in /proc/crypto and everybody knows that a particular cipher 
is not in use.

My particular example is that in one test with FIPS mode enabled, 
seqiv(rfc4106(gcm-base(...))) was successfully loaded and marked as selftest 
passed in /proc/crypto. Yet, an allocation of the test failed with ENOENT. 
Rebooting the system without FIPS mode allowed me to allocate the cipher. As 
there is no test for this particular cipher name in testmgr, I highly suspect 
that the allocation code did not find it because somehow there was no test. In 
any case, it is definitely not clear why that particular cipher name cannot be 
allocated in FIPS mode given the entries in /proc/crypto.

Ciao
Stephan

^ permalink raw reply

* [PATCH -next] hwrng: geode - fix return value check in mod_init()
From: Wei Yongjun @ 2016-09-16  1:50 UTC (permalink / raw)
  To: Matt Mackall, Herbert Xu, PrasannaKumar Muralidharan
  Cc: Wei Yongjun, linux-geode, linux-crypto

From: Wei Yongjun <weiyongjun1@huawei.com>

In case of error, the function devm_ioremap() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should
be replaced with NULL test.

Fixes: 6e9b5e76882c ("hwrng: geode - Migrate to managed API")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/char/hw_random/geode-rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/geode-rng.c b/drivers/char/hw_random/geode-rng.c
index 0cae210..e7a2459 100644
--- a/drivers/char/hw_random/geode-rng.c
+++ b/drivers/char/hw_random/geode-rng.c
@@ -95,8 +95,8 @@ static int __init mod_init(void)
 				return -ENODEV;
 
 			mem = devm_ioremap(&pdev->dev, rng_base, 0x58);
-			if (IS_ERR(mem))
-				return PTR_ERR(mem);
+			if (!mem)
+				return -ENOMEM;
 			geode_rng.priv = (unsigned long)mem;
 
 			pr_info("AMD Geode RNG detected\n");

^ permalink raw reply related

* [PATCH -next] hwrng: amd - Fix return value check in mod_init()
From: Wei Yongjun @ 2016-09-16  1:49 UTC (permalink / raw)
  To: Matt Mackall, Herbert Xu, Corentin LABBE,
	PrasannaKumar Muralidharan
  Cc: Wei Yongjun, linux-crypto

From: Wei Yongjun <weiyongjun1@huawei.com>

In case of error, the function devm_kzalloc() or devm_ioport_map()
return NULL pointer not ERR_PTR(). The IS_ERR() test in the return
value check should be replaced with NULL test.

Fixes: 31b2a73c9c5f ("hwrng: amd - Migrate to managed API")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/char/hw_random/amd-rng.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 4dbc5aa..4a99ac7 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -149,8 +149,8 @@ static int __init mod_init(void)
 		return -EIO;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (IS_ERR(priv))
-		return PTR_ERR(priv);
+	if (!priv)
+		return -ENOMEM;
 
 	if (!devm_request_region(&pdev->dev, pmbase + PMBASE_OFFSET,
 				PMBASE_SIZE, DRV_NAME)) {
@@ -161,9 +161,9 @@ static int __init mod_init(void)
 
 	priv->iobase = devm_ioport_map(&pdev->dev, pmbase + PMBASE_OFFSET,
 			PMBASE_SIZE);
-	if (IS_ERR(priv->iobase)) {
+	if (!priv->iobase) {
 		pr_err(DRV_NAME "Cannot map ioport\n");
-		return PTR_ERR(priv->iobase);
+		return -ENOMEM;
 	}
 
 	amd_rng.priv = (unsigned long)priv;

^ permalink raw reply related

* Re: [PATCH] Fix Kconfig dependencies for FIPS
From: NTU @ 2016-09-15 22:45 UTC (permalink / raw)
  To: linux-crypto
In-Reply-To: <CAM5Ud7Ph1LR_8F7Pm9x55B9bozuQL+=6NZfVMjPtTPiDswfUHw@mail.gmail.com>

I hope I got this right, on my screen the first "depends on" line is
incorrectly truncated, hope this will not become problematic..

Alec Ari

^ permalink raw reply

* [PATCH] Fix Kconfig dependencies for FIPS
From: NTU @ 2016-09-15 22:41 UTC (permalink / raw)
  To: linux-crypto

Currently FIPS depends on MODULE_SIG, even if MODULES is disabled.
This change allows the enabling of FIPS without support for modules.

If module loading support is enabled, only then does
FIPS require MODULE_SIG.

Signed-off-by: Alec Ari <neotheuser@gmail.com>
---
 crypto/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 84d7148..fd28805 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -24,7 +24,7 @@ comment "Crypto core or helper"
 config CRYPTO_FIPS
     bool "FIPS 200 compliance"
     depends on (CRYPTO_ANSI_CPRNG || CRYPTO_DRBG) &&
!CRYPTO_MANAGER_DISABLE_TESTS
-    depends on MODULE_SIG
+    depends on (MODULE_SIG || !MODULES)
     help
       This options enables the fips boot option which is
       required if you want to system to operate in a FIPS 200
-- 
2.7.3

^ permalink raw reply related

* Re: CONFIG_FIPS without module loading support?
From: Stephan Mueller @ 2016-09-15 18:31 UTC (permalink / raw)
  To: NTU; +Cc: linux-crypto
In-Reply-To: <CAM5Ud7PsjjrR0Ta6RFJbxcxXqVj7aEwzWF__7TEKpxun96NhdQ@mail.gmail.com>

Am Donnerstag, 15. September 2016, 12:06:20 CEST schrieb NTU:

Hi NTU,

> What did I miss from the SubmittingPatches page? Some constructive

The patch should be inline to the email -- see all other patch submissions. 
Then, the email subject should be appropriate.

> criticism please? Step 1 is skipped due to the fact I'm using git,
> patch is in proper form. Step 2, I described the patch. 3, it's one
> line, so it cannot be separated. Step 4, checkpatch.pl says it's good.
> The section in 5 confused me a little bit. 6, the patch is plain text.
> 7, it is under 300k (easily.) 8, doing it right now. 9, ok. 10, PATCH
> is included in the subject. 11, it is signed, says signed off at the
> bottom of the comment section. 12 I don't think is applicable to this?
> 13, not applicable again? 14, it is in canonical format, comment lines
> do not exceed 70 characters, it has a marker line, diff output etc. 15
> confused me a little. 16 it is not a series of patches.
> 
> If ANSI_CPRNG is not approved anymore for FIPS, the help section
> should be updated then.
> 
> As for converting the DRBG booleans to choice, it is so that way users
> cannot have both options disabled, in the case they don't read the
> help for it.
> 
> Alec
> 
> On Wed, Sep 14, 2016 at 11:58 PM, Stephan Mueller <smueller@chronox.de> 
wrote:
> > Am Mittwoch, 14. September 2016, 19:18:43 CEST schrieb NTU:
> > 
> > Hi NTU,
> > 
> >> Hello,
> >> 
> >> I've never written a patch before to the official kernel mailing list
> >> (that I remember) so please forgive me if I didn't send this in
> >> properly. I've generated this using git format-patch HEAD~ --stdout &>
> >> kconfig_fix_for_fips.patch and have attached the file in this email,
> >> gathering as much as I could from the Documentation/SubmittingPatches
> >> page.
> > 
> > Please read Documentation/SubmittingPatches
> > 
> >> Few more things, in the help option for CRYPTO_ANSI_CPRNG, it says it
> >> must be enabled if FIPS is selected, but in the dependencies for FIPS,
> >> if DRBG is selected, then CRYPTO_ANSI_CPRNG doesn't need to be
> >> enabled. Which one is true?
> > 
> > The latter one. The X9.31 DRNG is not approved in FIPS mode any more.
> > 
> >> Secondly, in the help option for CRYPTO_DRBG_MENU, it says that one or
> >> more of the DRBG types must be selected. If this is indeed true,
> >> shouldn't the options within CRYPTO_DRBG_MENU be converted to
> >> choice/endchoice versus just booleans? One selection for
> >> CRYPTO_DRBG_HASH, another for CRYPTO_DRBG_CTR, and then a third option
> >> for both? Should I submit patches for these as well,
> >> feedback/thoughts?
> > 
> > Not sure what you want to gain from it. All that the booleans do is to
> > mark
> > which types of DRBG are to be compliled. The selector whether the DRBG is
> > compiled at all is CRYPTO_DRBG.
> > 
> > Ciao
> > Stephan
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



Ciao
Stephan

^ permalink raw reply

* Re: CONFIG_FIPS without module loading support?
From: NTU @ 2016-09-15 17:06 UTC (permalink / raw)
  To: linux-crypto
In-Reply-To: <2523179.y8adHMcoDs@tauon.atsec.com>

What did I miss from the SubmittingPatches page? Some constructive
criticism please? Step 1 is skipped due to the fact I'm using git,
patch is in proper form. Step 2, I described the patch. 3, it's one
line, so it cannot be separated. Step 4, checkpatch.pl says it's good.
The section in 5 confused me a little bit. 6, the patch is plain text.
7, it is under 300k (easily.) 8, doing it right now. 9, ok. 10, PATCH
is included in the subject. 11, it is signed, says signed off at the
bottom of the comment section. 12 I don't think is applicable to this?
13, not applicable again? 14, it is in canonical format, comment lines
do not exceed 70 characters, it has a marker line, diff output etc. 15
confused me a little. 16 it is not a series of patches.

If ANSI_CPRNG is not approved anymore for FIPS, the help section
should be updated then.

As for converting the DRBG booleans to choice, it is so that way users
cannot have both options disabled, in the case they don't read the
help for it.

Alec

On Wed, Sep 14, 2016 at 11:58 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Mittwoch, 14. September 2016, 19:18:43 CEST schrieb NTU:
>
> Hi NTU,
>
>> Hello,
>>
>> I've never written a patch before to the official kernel mailing list
>> (that I remember) so please forgive me if I didn't send this in
>> properly. I've generated this using git format-patch HEAD~ --stdout &>
>> kconfig_fix_for_fips.patch and have attached the file in this email,
>> gathering as much as I could from the Documentation/SubmittingPatches
>> page.
>
> Please read Documentation/SubmittingPatches
>>
>> Few more things, in the help option for CRYPTO_ANSI_CPRNG, it says it
>> must be enabled if FIPS is selected, but in the dependencies for FIPS,
>> if DRBG is selected, then CRYPTO_ANSI_CPRNG doesn't need to be
>> enabled. Which one is true?
>
> The latter one. The X9.31 DRNG is not approved in FIPS mode any more.
>>
>> Secondly, in the help option for CRYPTO_DRBG_MENU, it says that one or
>> more of the DRBG types must be selected. If this is indeed true,
>> shouldn't the options within CRYPTO_DRBG_MENU be converted to
>> choice/endchoice versus just booleans? One selection for
>> CRYPTO_DRBG_HASH, another for CRYPTO_DRBG_CTR, and then a third option
>> for both? Should I submit patches for these as well,
>> feedback/thoughts?
>
> Not sure what you want to gain from it. All that the booleans do is to mark
> which types of DRBG are to be compliled. The selector whether the DRBG is
> compiled at all is CRYPTO_DRBG.
>
> Ciao
> Stephan

^ permalink raw reply

* Re: crypto-caamhash: Fine-tuning for several function implementations
From: Horia Geanta Neag @ 2016-09-15 15:30 UTC (permalink / raw)
  To: SF Markus Elfring, linux-crypto@vger.kernel.org, David S. Miller,
	Herbert Xu, Labbe Corentin, Russell King
  Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <970e9e1b-c1dc-eb28-b380-92c15e9b1961@users.sourceforge.net>

On 9/15/2016 5:37 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 15 Sep 2016 16:27:23 +0200
> 
> Some update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (6):
>   Use kmalloc_array() in ahash_setkey()
>   Rename jump labels in ahash_setkey()
>   Rename a jump label in five functions
>   Return a value directly in caam_hash_cra_init()
>   Delete an unnecessary initialisation in seven functions
>   Move common error handling code in two functions
> 
>  drivers/crypto/caam/caamhash.c | 111 +++++++++++++++++++----------------------
>  1 file changed, 52 insertions(+), 59 deletions(-)
> 
Thanks Markus!

For patches 2-6:
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

though headline prefix should be changed to "crypto: caam -"

Horia


^ permalink raw reply

* Re: [PATCH 1/6] crypto-caamhash: Use kmalloc_array() in ahash_setkey()
From: Horia Geanta Neag @ 2016-09-15 15:12 UTC (permalink / raw)
  To: SF Markus Elfring, linux-crypto@vger.kernel.org, David S. Miller,
	Herbert Xu, Labbe Corentin, Russell King
  Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <4e1bc780-fa00-cfc9-c058-240bb899d701@users.sourceforge.net>

On 9/15/2016 5:43 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 15 Sep 2016 11:20:09 +0200
> 
> * A multiplication for the size determination of a memory allocation
>   indicated that an array data structure should be processed.
>   Thus use the corresponding function "kmalloc_array".
> 
>   This issue was detected by using the Coccinelle software.
> 
> * Replace the specification of a data type by a pointer dereference
>   to make the corresponding size determination a bit safer according to
>   the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/crypto/caam/caamhash.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
> index 9d7fc9e..f19df8f 100644
> --- a/drivers/crypto/caam/caamhash.c
> +++ b/drivers/crypto/caam/caamhash.c
> @@ -525,8 +525,9 @@ static int ahash_setkey(struct crypto_ahash *ahash,
>  #endif
>  
>  	if (keylen > blocksize) {
> -		hashed_key = kmalloc(sizeof(u8) * digestsize, GFP_KERNEL |
> -				     GFP_DMA);
> +		hashed_key = kmalloc_array(digestsize,
> +					   sizeof(*hashed_key),
> +					   GFP_KERNEL | GFP_DMA);
While correct, instead I would go with kmalloc() and get rid of sizeof(u8).

Horia

^ permalink raw reply

* [PATCH 6/6] crypto-caamhash: Move common error handling code in two functions
From: SF Markus Elfring @ 2016-09-15 14:45 UTC (permalink / raw)
  To: linux-crypto, David S. Miller, Herbert Xu, Labbe Corentin,
	Russell King
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <970e9e1b-c1dc-eb28-b380-92c15e9b1961@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 15 Sep 2016 16:00:55 +0200

Move statements for error handling which were identical
in two if branches to the end of these functions.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/crypto/caam/caamhash.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index adb8b19..660dc20 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1231,9 +1231,7 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 	state->buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);
 	if (dma_mapping_error(jrdev, state->buf_dma)) {
 		dev_err(jrdev, "unable to map src\n");
-		ahash_unmap(jrdev, edesc, req, digestsize);
-		kfree(edesc);
-		return -ENOMEM;
+		goto unmap;
 	}
 
 	append_seq_in_ptr(desc, state->buf_dma, buflen, 0);
@@ -1242,9 +1240,7 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
-		ahash_unmap(jrdev, edesc, req, digestsize);
-		kfree(edesc);
-		return -ENOMEM;
+		goto unmap;
 	}
 	edesc->src_nents = 0;
 
@@ -1262,6 +1258,11 @@ static int ahash_final_no_ctx(struct ahash_request *req)
 	}
 
 	return ret;
+ unmap:
+	ahash_unmap(jrdev, edesc, req, digestsize);
+	kfree(edesc);
+	return -ENOMEM;
+
 }
 
 /* submit ahash update if it the first job descriptor after update */
@@ -1453,18 +1454,14 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 				  req->nbytes);
 	if (ret) {
 		dev_err(jrdev, "unable to map S/G table\n");
-		ahash_unmap(jrdev, edesc, req, digestsize);
-		kfree(edesc);
-		return -ENOMEM;
+		goto unmap;
 	}
 
 	edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
 						digestsize);
 	if (dma_mapping_error(jrdev, edesc->dst_dma)) {
 		dev_err(jrdev, "unable to map dst\n");
-		ahash_unmap(jrdev, edesc, req, digestsize);
-		kfree(edesc);
-		return -ENOMEM;
+		goto unmap;
 	}
 
 #ifdef DEBUG
@@ -1481,6 +1478,11 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
 	}
 
 	return ret;
+ unmap:
+	ahash_unmap(jrdev, edesc, req, digestsize);
+	kfree(edesc);
+	return -ENOMEM;
+
 }
 
 /* submit first update job descriptor after init */
-- 
2.10.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox