Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH v6 0/5] /dev/random - a new approach
From: Pavel Machek @ 2016-08-17 21:42 UTC (permalink / raw)
  To: Theodore Ts'o, Stephan Mueller, herbert, sandyinchina,
	Jason Cooper, John Denker, H. Peter Anvin, Joe Perches,
	George Spelvin, linux-crypto, linux-kernel
In-Reply-To: <20160811213632.GL10626@thunk.org>

Hi!

> As far as whether or not you can gather enough entropy at boot time,
> what we're really talking about how how much entropy we want to assume
> can be gathered from interrupt timings, since what you do in your code
> is not all that different from what the current random driver is
> doing.  So it's pretty easy to turn a knob and say, "hey presto, we
> can get all of the entropy we need before userspace starts!"  But
> justifying this is much harder, and using statistical tests isn't
> really sufficient as far as I'm concerned.

Actually.. I'm starting to believe that getting enough entropy before
userspace starts is more important than pretty much anything else.

We only "need" 64-bits of entropy, AFAICT. If it passes statistical
tests, I'd use it... for initial bringup.

We can switch to more conservative estimates when system is fully
running. But IMO it is very important to get _some_ randomness at the
begining...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH] crypto: doc - fix documentation for bulk registration functions
From: Eric Biggers @ 2016-08-17 22:47 UTC (permalink / raw)
  To: herbert; +Cc: davem, linux-crypto, linux-doc, linux-kernel, Eric Biggers

Update the documentation for crypto_register_algs() and
crypto_unregister_algs() to match the actual behavior.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Documentation/DocBook/crypto-API.tmpl | 38 ++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/Documentation/DocBook/crypto-API.tmpl b/Documentation/DocBook/crypto-API.tmpl
index fb2a152..088b79c 100644
--- a/Documentation/DocBook/crypto-API.tmpl
+++ b/Documentation/DocBook/crypto-API.tmpl
@@ -797,7 +797,8 @@ kernel crypto API            |       Caller
      include/linux/crypto.h and their definition can be seen below.
      The former function registers a single transformation, while
      the latter works on an array of transformation descriptions.
-     The latter is useful when registering transformations in bulk.
+     The latter is useful when registering transformations in bulk,
+     for example when a driver implements multiple transformations.
     </para>
 
     <programlisting>
@@ -822,18 +823,31 @@ kernel crypto API            |       Caller
     </para>
 
     <para>
-     The bulk registration / unregistration functions require
-     that struct crypto_alg is an array of count size. These
-     functions simply loop over that array and register /
-     unregister each individual algorithm. If an error occurs,
-     the loop is terminated at the offending algorithm definition.
-     That means, the algorithms prior to the offending algorithm
-     are successfully registered. Note, the caller has no way of
-     knowing which cipher implementations have successfully
-     registered. If this is important to know, the caller should
-     loop through the different implementations using the single
-     instance *_alg functions for each individual implementation.
+     The bulk registration/unregistration functions
+     register/unregister each transformation in the given array of
+     length count.  They handle errors as follows:
     </para>
+    <itemizedlist>
+     <listitem>
+      <para>
+       crypto_register_algs() succeeds if and only if it
+       successfully registers all the given transformations. If an
+       error occurs partway through, then it rolls back successful
+       registrations before returning the error code. Note that if
+       a driver needs to handle registration errors for individual
+       transformations, then it will need to use the non-bulk
+       function crypto_register_alg() instead.
+      </para>
+     </listitem>
+     <listitem>
+      <para>
+       crypto_unregister_algs() tries to unregister all the given
+       transformations, continuing on error. It logs errors and
+       always returns zero.
+      </para>
+     </listitem>
+    </itemizedlist>
+
    </sect1>
 
    <sect1><title>Single-Block Symmetric Ciphers [CIPHER]</title>
-- 
2.8.0.rc3.226.g39d4020


^ permalink raw reply related

* Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: PrasannaKumar Muralidharan @ 2016-08-18  5:14 UTC (permalink / raw)
  To: Corentin LABBE
  Cc: mpm, Herbert Xu, robh+dt, mark.rutland, Ralf Baechle, davem,
	geert, Andrew Morton, Greg KH, mchehab, Guenter Roeck,
	boris.brezillon, harvey.hunt, alex.smith, Daniel Thompson,
	Lee Jones, Florian Fainelli, kieran, Krzysztof Kozlowski,
	joshua.henderson, yendapally.reddy, narmstrong, wangkefeng.wang,
	Christian Lamparter, Álvaro Fernández Rojas,
	Linus Walleij, pankaj.dev
In-Reply-To: <92a00062-9a87-0053-2c99-17bd1a304a4a@gmail.com>

> I have just some minor comments below

Appreciate your review.

>> diff --git a/drivers/char/hw_random/jz4780-rng.c b/drivers/char/hw_random/jz4780-rng.c
>> new file mode 100644
>> index 0000000..c9d2cde
>> --- /dev/null
>> +++ b/drivers/char/hw_random/jz4780-rng.c
>> @@ -0,0 +1,105 @@
>> +/*
>> + * jz4780-rng.c - Random Number Generator driver for J4780
>> + *
>> + * Copyright 2016 (C) PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
>> + *
>> + * This file is licensed under  the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include <linux/hw_random.h>
>> +#include <linux/device.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/io.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/err.h>
>
> You could sort them by alphabetical order

Sure, will do.

>> +
>> +#define REG_RNG_CTRL 0x0
>> +#define REG_RNG_DATA 0x4
>> +
>> +struct jz4780_rng {
>> +     struct device *dev;
>> +     struct hwrng rng;
>> +     void __iomem *mem;
>> +};
>> +
>> +static u32 jz4780_rng_readl(struct jz4780_rng *rng, u32 offset)
>> +{
>> +     return readl(rng->mem + offset);
>> +}
>> +
>> +static void jz4780_rng_writel(struct jz4780_rng *rng, u32 val, u32 offset)
>> +{
>> +     writel(val, rng->mem + offset);
>> +}
>> +
>> +static int jz4780_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
>> +{
>> +     struct jz4780_rng *jz4780_rng = container_of(rng, struct jz4780_rng,
>> +                                                     rng);
>> +     u32 *data = buf;
>> +     *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
>> +     return 4;
>> +}
>
> If max is less than 4, its bad

Data will be 4 bytes.

>> +
>> +static int jz4780_rng_probe(struct platform_device *pdev)
>> +{
>> +     struct jz4780_rng *jz4780_rng;
>> +     struct resource *res;
>> +     resource_size_t size;
>> +     int ret;
>> +
>> +     jz4780_rng = devm_kzalloc(&pdev->dev, sizeof(struct jz4780_rng),
>> +                                     GFP_KERNEL);
>
> You could write sizeof(*js480_rng)

Will do.

>> +     if (!jz4780_rng)
>> +             return -ENOMEM;
>> +
>> +     jz4780_rng->dev = &pdev->dev;
>> +     jz4780_rng->rng.name = "jz4780";
>> +     jz4780_rng->rng.read = jz4780_rng_read;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     size = resource_size(res);
>> +
>> +     jz4780_rng->mem = devm_ioremap(&pdev->dev, res->start, size);
> You could save code by using devm_ioremap_resource (don't need size)

Will do.

>> +     if (IS_ERR(jz4780_rng->mem))
>> +             return PTR_ERR(jz4780_rng->mem);
>> +
>> +     platform_set_drvdata(pdev, jz4780_rng);
>> +     jz4780_rng_writel(jz4780_rng, 1, REG_RNG_CTRL);
>> +     ret = hwrng_register(&jz4780_rng->rng);
>> +
>> +     return ret;
>> +}
> You could write directly return hwrng_register(..)

Will do.

^ permalink raw reply

* Re: [PATCH v2] crypto: XTS - remove test that will fail in FIPS mode
From: Stephan Mueller @ 2016-08-18  8:20 UTC (permalink / raw)
  To: Tapas Sarangi; +Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org
In-Reply-To: <D3D9E64F.30F1%tsarangi@trustwave.com>

Am Mittwoch, 17. August 2016, 15:09:11 CEST schrieb Tapas Sarangi:

Hi Tapas,

> Is that all the authenc() ciphers, or only some of them ? In my patch

I have not yet had the chance to fully dissect the authenc issue yet.

> where I had disabled .fips_allowed are mostly authenc() ciphers with
> cbc(des3_ede) algo. Not all the authenc() ciphers were needed to be
> disabled, but some.

Can you please point me to your patch?
> 
> For your XTS related findings and patches, are they going to 4.8 or 4.9 ?

The XTS patch set is for 4.8-rc1 and should therefore go into 4.9 if accepted 
by the maintainer.


Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: LABBE Corentin @ 2016-08-18 11:53 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, Herbert Xu,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Ralf Baechle, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, Andrew Morton, Greg KH,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, Guenter Roeck,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	harvey.hunt-1AXoQHu6uovQT0dZR+AlfA,
	alex.smith-1AXoQHu6uovQT0dZR+AlfA, Daniel Thompson, Lee Jones,
	Florian Fainelli, kieran-7hKh/agyDeatmTQ+vhA3Yw,
	Krzysztof Kozlowski, joshua.henderson-UWL1GkI3JZL3oGB3hsPCZA,
	yendapally.reddy-dY08KVG/lbpWk0Htik3J/w,
	narmstrong-rdvid1DuHRBWk0Htik3J/w,
	wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, Christian Lamparter,
	Álvaro Fernández Rojas, Linus Walleij,
	pankaj.dev-qxv4g6HH51o
In-Reply-To: <CANc+2y55ZCkauwKNtuuCxLx-WOtm8z+A_EBKsYSjEUdc+ZbZTQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Aug 18, 2016 at 10:44:18AM +0530, PrasannaKumar Muralidharan wrote:
> >> +static int jz4780_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> >> +{
> >> +     struct jz4780_rng *jz4780_rng = container_of(rng, struct jz4780_rng,
> >> +                                                     rng);
> >> +     u32 *data = buf;
> >> +     *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
> >> +     return 4;
> >> +}
> >
> > If max is less than 4, its bad
> 
> Data will be 4 bytes.
> 

No, according to comment in include/linux/hw_random.h "drivers can fill up to max bytes of data"
So you cannot write more than max bytes without risking buffer overflow.

And if max > 4, hwrng client need to recall your read function.
The better example I found is tpm_get_random() in drivers/char/tpm/tpm-interface.c for handling both problem.

Regards

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 2/2] crypto: marvell - Don't break chain for computable last ahash requests
From: Romain Perier @ 2016-08-18 12:12 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Thomas Petazzoni, Russell King, Herbert Xu, linux-crypto,
	Gregory Clement, David S. Miller, linux-arm-kernel
In-Reply-To: <1471522334-24839-1-git-send-email-romain.perier@free-electrons.com>

Currently, the driver breaks chain for all kind of hash requests in order
to don't override intermediate states of partial ahash updates. However,
some final ahash requests can be directly processed by the engine, and
so without intermediate state. This is typically the case for most for
the HMAC requests processed via IPSec.

This commits adds a TDMA descriptor to copy outer results for thise kind
of request into the "result" dma pool, then it allow to chain these
requests at the DMA level. The 'complete' operation is also updated to
retrieve the MAC digest from the right location.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 69 +++++++++++++++++++++++++++++++++----------
 1 file changed, 54 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 9f28468..1a91662 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -312,24 +312,48 @@ static void mv_cesa_ahash_complete(struct crypto_async_request *req)
 	int i;
 
 	digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(ahashreq));
-	for (i = 0; i < digsize / 4; i++)
-		creq->state[i] = readl_relaxed(engine->regs + CESA_IVDIG(i));
 
-	if (creq->last_req) {
+	if (mv_cesa_req_get_type(&creq->base) == CESA_DMA_REQ &&
+	    !(creq->base.chain.last->flags & CESA_TDMA_BREAK_CHAIN)) {
+		struct mv_cesa_tdma_desc *tdma = NULL;
+
+		for (tdma = creq->base.chain.first; tdma; tdma = tdma->next) {
+			u32 type = tdma->flags & CESA_TDMA_TYPE_MSK;
+			if (type ==  CESA_TDMA_RESULT)
+				break;
+		}
+
+		BUG_ON(!tdma);
+
 		/*
-		 * Hardware's MD5 digest is in little endian format, but
-		 * SHA in big endian format
+		 * Result is already in the correct endianess when the SA is
+		 * used
 		 */
-		if (creq->algo_le) {
-			__le32 *result = (void *)ahashreq->result;
+		__le32 *data = tdma->data + 0x40;
+		for (i = 0; i < digsize / 4; i++)
+			creq->state[i] = cpu_to_le32(data[i]);
 
-			for (i = 0; i < digsize / 4; i++)
-				result[i] = cpu_to_le32(creq->state[i]);
-		} else {
-			__be32 *result = (void *)ahashreq->result;
+		memcpy(ahashreq->result, data, digsize);
+	} else {
+		for (i = 0; i < digsize / 4; i++)
+			creq->state[i] = readl_relaxed(engine->regs +
+						       CESA_IVDIG(i));
+		if (creq->last_req) {
+			/*
+			* Hardware's MD5 digest is in little endian format, but
+			* SHA in big endian format
+			*/
+			if (creq->algo_le) {
+				__le32 *result = (void *)ahashreq->result;
+
+				for (i = 0; i < digsize / 4; i++)
+					result[i] = cpu_to_le32(creq->state[i]);
+			} else {
+				__be32 *result = (void *)ahashreq->result;
 
-			for (i = 0; i < digsize / 4; i++)
-				result[i] = cpu_to_be32(creq->state[i]);
+				for (i = 0; i < digsize / 4; i++)
+					result[i] = cpu_to_be32(creq->state[i]);
+			}
 		}
 	}
 
@@ -504,6 +528,11 @@ mv_cesa_ahash_dma_last_req(struct mv_cesa_tdma_chain *chain,
 						CESA_SA_DESC_CFG_LAST_FRAG,
 				      CESA_SA_DESC_CFG_FRAG_MSK);
 
+		ret = mv_cesa_dma_add_result_op(chain,
+						CESA_SA_MAC_IIV_SRAM_OFFSET, 96,
+						CESA_TDMA_SRC_IN_SRAM, flags);
+		if (ret)
+			return ERR_PTR(-ENOMEM);
 		return op;
 	}
 
@@ -564,6 +593,7 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx *op = NULL;
 	unsigned int frag_len;
 	int ret;
+	u32 type;
 
 	basereq->chain.first = NULL;
 	basereq->chain.last = NULL;
@@ -635,6 +665,8 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
 		goto err_free_tdma;
 	}
 
+	type = basereq->chain.last->flags & CESA_TDMA_TYPE_MSK;
+
 	if (op) {
 		/* Add dummy desc to wait for crypto operation end */
 		ret = mv_cesa_dma_add_dummy_end(&basereq->chain, flags);
@@ -648,8 +680,15 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
 	else
 		creq->cache_ptr = 0;
 
-	basereq->chain.last->flags |= (CESA_TDMA_END_OF_REQ |
-				       CESA_TDMA_BREAK_CHAIN);
+	basereq->chain.last->flags |= CESA_TDMA_END_OF_REQ;
+	/*
+	 * If results are copied via DMA, this means that this
+	 * request can be directly processed by the engine,
+	 * without partial updates. So we can chain it at the
+	 * DMA level with other requests.
+	 */
+	if (type != CESA_TDMA_RESULT)
+		basereq->chain.last->flags |= CESA_TDMA_BREAK_CHAIN;
 
 	return 0;
 
-- 
2.8.1

^ permalink raw reply related

* [PATCH 0/2] Improve DMA chaining for ahash requests
From: Romain Perier @ 2016-08-18 12:12 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: David S. Miller, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Russell King, linux-crypto, linux-arm-kernel

This series contain performance improvement regarding ahash requests.
So far, ahash requests were systematically not chained at the DMA level.
However, in some case, like this is the case by using IPSec, some ahash
requests can be processed directly by the engine, and don't have
intermediaire partial update states.

This series firstly re-work the way outer IVs are copied from the SRAM
into the dma pool. To do so, we introduce a common dma pool for all type
of requests that contains outer results (like IV or digest). Then, for
ahash requests that can be processed directly by the engine, outer
results are copied from the SRAM into the common dma pool. These requests
are then allowed to be chained at the DMA level.


Benchmarking results with iperf throught IPSec
==============================================
		ESP			AH

Before		373 Mbits/s		530 Mbits/s
After		413 Mbits/s		578 Mbits/s
Improvement	+11%			+9%


Romain Perier (2):
  crypto: marvell - Use an unique pool to copy results of requests
  crypto: marvell - Don't break chain for computable last ahash requests

 drivers/crypto/marvell/cesa.c   |  4 +--
 drivers/crypto/marvell/cesa.h   |  6 ++--
 drivers/crypto/marvell/cipher.c |  2 +-
 drivers/crypto/marvell/hash.c   | 69 ++++++++++++++++++++++++++++++++---------
 drivers/crypto/marvell/tdma.c   | 16 +++++-----
 5 files changed, 68 insertions(+), 29 deletions(-)

-- 
2.8.1

^ permalink raw reply

* [PATCH 1/2] crypto: marvell - Use an unique pool to copy results of requests
From: Romain Perier @ 2016-08-18 12:12 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: David S. Miller, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Russell King, linux-crypto, linux-arm-kernel
In-Reply-To: <1471522334-24839-1-git-send-email-romain.perier@free-electrons.com>

So far, we used a dedicated dma pool to copy the result of outer IV for
cipher requests. Instead of using a dma pool per outer data, we prefer
use a common dma pool that contains the part of the SRAM that is likely
to be used by the 'complete' operation, later. In this way, any type of
result can be retrieved by DMA for cipher or ahash requests.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/cesa.c   |  4 ++--
 drivers/crypto/marvell/cesa.h   |  6 +++---
 drivers/crypto/marvell/cipher.c |  2 +-
 drivers/crypto/marvell/tdma.c   | 16 ++++++++--------
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index 37dadb2..4d308ad 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -375,8 +375,8 @@ static int mv_cesa_dev_dma_init(struct mv_cesa_dev *cesa)
 	if (!dma->padding_pool)
 		return -ENOMEM;
 
-	dma->iv_pool = dmam_pool_create("cesa_iv", dev, 16, 1, 0);
-	if (!dma->iv_pool)
+	dma->result_pool = dmam_pool_create("cesa_result", dev, 96, 1, 0);
+	if (!dma->result_pool)
 		return -ENOMEM;
 
 	cesa->dma = dma;
diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
index e423d33..3be1aa3 100644
--- a/drivers/crypto/marvell/cesa.h
+++ b/drivers/crypto/marvell/cesa.h
@@ -277,7 +277,7 @@ struct mv_cesa_op_ctx {
 #define CESA_TDMA_DUMMY				0
 #define CESA_TDMA_DATA				1
 #define CESA_TDMA_OP				2
-#define CESA_TDMA_IV				3
+#define CESA_TDMA_RESULT			3
 
 /**
  * struct mv_cesa_tdma_desc - TDMA descriptor
@@ -393,7 +393,7 @@ struct mv_cesa_dev_dma {
 	struct dma_pool *op_pool;
 	struct dma_pool *cache_pool;
 	struct dma_pool *padding_pool;
-	struct dma_pool *iv_pool;
+	struct dma_pool *result_pool;
 };
 
 /**
@@ -839,7 +839,7 @@ mv_cesa_tdma_desc_iter_init(struct mv_cesa_tdma_chain *chain)
 	memset(chain, 0, sizeof(*chain));
 }
 
-int mv_cesa_dma_add_iv_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
+int mv_cesa_dma_add_result_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
 			  u32 size, u32 flags, gfp_t gfp_flags);
 
 struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
diff --git a/drivers/crypto/marvell/cipher.c b/drivers/crypto/marvell/cipher.c
index d19dc96..bd575b1 100644
--- a/drivers/crypto/marvell/cipher.c
+++ b/drivers/crypto/marvell/cipher.c
@@ -373,7 +373,7 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
 
 	/* Add output data for IV */
 	ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
-	ret = mv_cesa_dma_add_iv_op(&basereq->chain, CESA_SA_CRYPT_IV_SRAM_OFFSET,
+	ret = mv_cesa_dma_add_result_op(&basereq->chain, CESA_SA_CRYPT_IV_SRAM_OFFSET,
 				    ivsize, CESA_TDMA_SRC_IN_SRAM, flags);
 
 	if (ret)
diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
index 9fd7a5f..499a1d3 100644
--- a/drivers/crypto/marvell/tdma.c
+++ b/drivers/crypto/marvell/tdma.c
@@ -69,8 +69,8 @@ void mv_cesa_dma_cleanup(struct mv_cesa_req *dreq)
 		if (type == CESA_TDMA_OP)
 			dma_pool_free(cesa_dev->dma->op_pool, tdma->op,
 				      le32_to_cpu(tdma->src));
-		else if (type == CESA_TDMA_IV)
-			dma_pool_free(cesa_dev->dma->iv_pool, tdma->data,
+		else if (type == CESA_TDMA_RESULT)
+			dma_pool_free(cesa_dev->dma->result_pool, tdma->data,
 				      le32_to_cpu(tdma->dst));
 
 		tdma = tdma->next;
@@ -209,29 +209,29 @@ mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags)
 	return new_tdma;
 }
 
-int mv_cesa_dma_add_iv_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
+int mv_cesa_dma_add_result_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
 			  u32 size, u32 flags, gfp_t gfp_flags)
 {
 
 	struct mv_cesa_tdma_desc *tdma;
-	u8 *iv;
+	u8 *result;
 	dma_addr_t dma_handle;
 
 	tdma = mv_cesa_dma_add_desc(chain, gfp_flags);
 	if (IS_ERR(tdma))
 		return PTR_ERR(tdma);
 
-	iv = dma_pool_alloc(cesa_dev->dma->iv_pool, gfp_flags, &dma_handle);
-	if (!iv)
+	result = dma_pool_alloc(cesa_dev->dma->result_pool, gfp_flags, &dma_handle);
+	if (!result)
 		return -ENOMEM;
 
 	tdma->byte_cnt = cpu_to_le32(size | BIT(31));
 	tdma->src = src;
 	tdma->dst = cpu_to_le32(dma_handle);
-	tdma->data = iv;
+	tdma->data = result;
 
 	flags &= (CESA_TDMA_DST_IN_SRAM | CESA_TDMA_SRC_IN_SRAM);
-	tdma->flags = flags | CESA_TDMA_IV;
+	tdma->flags = flags | CESA_TDMA_RESULT;
 	return 0;
 }
 
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: Daniel Thompson @ 2016-08-18 12:19 UTC (permalink / raw)
  To: LABBE Corentin, PrasannaKumar Muralidharan
  Cc: mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, Herbert Xu,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Ralf Baechle, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, Andrew Morton, Greg KH,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, Guenter Roeck,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	harvey.hunt-1AXoQHu6uovQT0dZR+AlfA,
	alex.smith-1AXoQHu6uovQT0dZR+AlfA, Lee Jones, Florian Fainelli,
	kieran-7hKh/agyDeatmTQ+vhA3Yw, Krzysztof Kozlowski,
	joshua.henderson-UWL1GkI3JZL3oGB3hsPCZA,
	yendapally.reddy-dY08KVG/lbpWk0Htik3J/w,
	narmstrong-rdvid1DuHRBWk0Htik3J/w,
	wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, Christian Lamparter,
	Álvaro Fernández Rojas, Linus Walleij,
	pankaj.dev-qxv4g6HH51o, Mathieu Poirier
In-Reply-To: <20160818115300.GA6621@Red>

On 18/08/16 12:53, LABBE Corentin wrote:
> On Thu, Aug 18, 2016 at 10:44:18AM +0530, PrasannaKumar Muralidharan wrote:
>>>> +static int jz4780_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
>>>> +{
>>>> +     struct jz4780_rng *jz4780_rng = container_of(rng, struct jz4780_rng,
>>>> +                                                     rng);
>>>> +     u32 *data = buf;
>>>> +     *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
>>>> +     return 4;
>>>> +}
>>>
>>> If max is less than 4, its bad
>>
>> Data will be 4 bytes.
>>
>
> No, according to comment in include/linux/hw_random.h "drivers can fill up to max bytes of data"
> So you cannot write more than max bytes without risking buffer overflow.
>
> And if max > 4, hwrng client need to recall your read function.
> The better example I found is tpm_get_random() in drivers/char/tpm/tpm-interface.c for handling both problem.

Right now the core code will never actually ask a RNG driver for <4 
bytes so perhaps it would be better to update the comment in 
include/linux/hw_random.h !

For devices with 32-bit RNG registers the extra code to handle a special 
case that doesn't actually exist is a waste.

There are 14 drivers in drivers/char/hw_random that support the ->read() 
interface but only three of these actually support max == 1 (existing 
accepted behavior varies between return 0, return 2, return 4 and return 
-EIO).


Daniel.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] hw_random: Improve description of the ->read() interface
From: Daniel Thompson @ 2016-08-18 12:37 UTC (permalink / raw)
  To: Matt Mackall, Herbert Xu
  Cc: Daniel Thompson, linux-crypto, linux-kernel, patches,
	linaro-kernel, LABBE Corentin, PrasannaKumar Muralidharan

Currently, very few RNG drivers support single byte reads using the
->read() interface. Of the 14 drivers in drivers/char/hw_random that
support this interface only three of these actually support max == 1.
The other behaviours vary between return 0, return 2, return 4 and return
-EIO).

This is not a problem in practice because the core hw_random code never
performs a read shorter than 16 bytes. The documentation for this function
already contrains the alignment of the buffer pointer, so let's also
guarantee that the buffer is at least as large as its alignment.

This constraint is intended to be the weakest guarantee neccessary to
allow driver writers to safely simplify their code.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 include/linux/hw_random.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index 4f7d8f4b1e9a..34a0dc18f327 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -29,7 +29,9 @@
  *			Returns the number of lower random bytes in "data".
  *			Must not be NULL.    *OBSOLETE*
  * @read:		New API. drivers can fill up to max bytes of data
- *			into the buffer. The buffer is aligned for any type.
+ *			into the buffer. The buffer is aligned for any type
+ *			and max is guaranteed to be >= to that alignment
+ *			(either 4 or 8 depending on architecture).
  * @priv:		Private data, for use by the RNG driver.
  * @quality:		Estimation of true entropy in RNG's bitstream
  *			(per mill).
--
2.7.4

^ permalink raw reply related

* Re: [PATCH v6 0/5] /dev/random - a new approach
From: Pavel Machek @ 2016-08-18 18:39 UTC (permalink / raw)
  To: Theodore Ts'o, Stephan Mueller, herbert, sandyinchina,
	Jason Cooper, John Denker, H. Peter Anvin, Joe Perches,
	George Spelvin, linux-crypto, linux-kernel
In-Reply-To: <20160818172712.GA22054@thunk.org>

On Thu 2016-08-18 13:27:12, Theodore Ts'o wrote:
> On Wed, Aug 17, 2016 at 11:42:55PM +0200, Pavel Machek wrote:
> > 
> > Actually.. I'm starting to believe that getting enough entropy before
> > userspace starts is more important than pretty much anything else.
> > 
> > We only "need" 64-bits of entropy, AFAICT. If it passes statistical
> > tests, I'd use it... for initial bringup.
> 
> Definitely not 64 bits.  Back in *1996* the estimate was that we
> needed at least 75-bits in order to be protected against brute force
> attacks.  It's been two *deacdes* years later, and granted Moore's law
> has ceased to apply in the last couple of years, but I'm sure 64 bits
> is not enough.
> 
> What is your specific concern vis-a-vis when userspace starts?  We now
> print a warning if someone tries to draw from /dev/urandom, and so it
> should be easy to see if someone is doing something dangerous.  The

Well, warning is nice, but I'm afraid it is not going to stop everyone.

> have only been known cases (at last as far asI know where) where some
> software was doing something as *insane* as to create keys right out
> of the box was.  One was ssh, and at least on a modern Debian system,
> that doesn't happen until fairly late in the process:

It is more widespread than that:

rapsberry pi:
https://www.raspberrypi.org/forums/viewtopic.php?t=126892

But this is the scary part. Not limited to ssh. "We perform the
largest ever network survey of TLS and SSH servers and present
evidence that vulnerable keys are surprisingly widespread. We find
that 0.75% of TLS certificates share keys due to insufficient entropy
during key generation, and we suspect that another 1.70% come from the
same faulty implementations and may be susceptible to compromise.
Even more alarmingly, we are able to obtain RSA private keys for 0.50%
of TLS hosts and 0.03% of SSH hosts, because their public keys shared
nontrivial common factors due to entropy problems, and DSA private
keys for 1.03% of SSH hosts, because of insufficient signature
randomness"

https://factorable.net/weakkeys12.conference.pdf

Responsible devices were Gigaset SX762, ADTran Total Access
businessgrade phone/network routers, IBM RSA II remote administration
cards, BladeCenter devices, Juniper Networks Branch SRX devices,
... "We used the techniques described in Section 3.2 to identify
apparently vulnerable devices from 27 manufacturers.  These include
enterprise-grade routers from Cisco; server management cards from
Dell, Hewlett-Packard, and IBM; virtual-private-network (VPN) devices;
building security systems; network attached storage devices; and
several kinds of consumer routers and VoIP products."

> The other was HP, which was generating an RSA key very shortly after
> the first time the printer was powered on.

Its definitely more than two incidents.

> > We can switch to more conservative estimates when system is fully
> > running. But IMO it is very important to get _some_ randomness at the
> > begining...
> 
> We're doing this already in the latest getrandom(2) implementation.
> For the purposes of initializing the crng, we assume that each
> interrupt has a single bit of entropy.  So it requires 128 initerrupts
> for getrandom(2) to be fully initialized.  I'm actually worried that
> this is too high as it is for architectures that don't have a
> fine-grained clock.  Given that on many of these embedded platforms
> there is a oscillator which drives all of the clocks and subsystems,
> it just doesn't make *sense* that than each interrupt could result in
> 5-6 bits of entropy, no matter what a magical statistical formula
> might say.

>From my point of view, it would make sense to factor time from RTC and
mac addresses into the initial hash. Situation in the paper was so bad
some devices had _completely identical_ keys. We should be able to do
better than that.

BTW... 128 interrupts... that's 1.3 seconds, right? Would it make
sense to wait two seconds if urandom use is attempted before it is
ready?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH -next] crypto: fix missing unlock on error in sun4i_hash()
From: Wei Yongjun @ 2016-08-18 22:42 UTC (permalink / raw)
  To: Corentin Labbe, Herbert Xu, David S. Miller, Maxime Ripard,
	Chen-Yu Tsai
  Cc: Wei Yongjun, linux-crypto, linux-arm-kernel

Add the missing unlock before return from function sun4i_hash()
in the error handling case.

Fixes: 477d9b2e591b ("crypto: sun4i-ss - unify update/final function")
Signed-off-by: Wei Yongjun <weiyj.lk@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
index 2ee3b59..de66f47 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
@@ -245,6 +245,7 @@ int sun4i_hash(struct ahash_request *areq)
 		if (end > areq->nbytes || areq->nbytes - end > 63) {
 			dev_err(ss->dev, "ERROR: Bound error %u %u\n",
 				end, areq->nbytes);
+			spin_unlock(&ss->slock);
 			return -EINVAL;
 		}
 	} else {

^ permalink raw reply related

* Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: Rob Herring @ 2016-08-19  0:59 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: mpm, herbert, mark.rutland, ralf, davem, geert, akpm, gregkh,
	mchehab, linux, boris.brezillon, harvey.hunt, alex.smith,
	daniel.thompson, lee.jones, f.fainelli, kieran, krzk,
	joshua.henderson, yendapally.reddy, narmstrong, wangkefeng.wang,
	chunkeey, noltari, linus.walleij, pankaj.dev, mathieu.poirier,
	linux-crypto, devicetree, linux-kernel, linux-mips
In-Reply-To: <1471448151-20850-1-git-send-email-prasannatsmkumar@gmail.com>

On Wed, Aug 17, 2016 at 09:05:51PM +0530, PrasannaKumar Muralidharan wrote:
> This patch adds support for hardware random number generator present in
> JZ4780 SoC.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  .../devicetree/bindings/rng/ingenic,jz4780-rng.txt |  12 +++

Acked-by: Rob Herring <robh@kernel.org>

>  MAINTAINERS                                        |   5 +
>  arch/mips/boot/dts/ingenic/jz4780.dtsi             |   7 +-
>  drivers/char/hw_random/Kconfig                     |  14 +++
>  drivers/char/hw_random/Makefile                    |   1 +
>  drivers/char/hw_random/jz4780-rng.c                | 105 +++++++++++++++++++++
>  6 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
>  create mode 100644 drivers/char/hw_random/jz4780-rng.c

^ permalink raw reply

* Re: [PATCH v6 0/5] /dev/random - a new approach
From: Theodore Ts'o @ 2016-08-18 17:27 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Stephan Mueller, herbert, sandyinchina, Jason Cooper, John Denker,
	H. Peter Anvin, Joe Perches, George Spelvin, linux-crypto,
	linux-kernel
In-Reply-To: <20160817214254.GA22438@amd>

On Wed, Aug 17, 2016 at 11:42:55PM +0200, Pavel Machek wrote:
> 
> Actually.. I'm starting to believe that getting enough entropy before
> userspace starts is more important than pretty much anything else.
> 
> We only "need" 64-bits of entropy, AFAICT. If it passes statistical
> tests, I'd use it... for initial bringup.

Definitely not 64 bits.  Back in *1996* the estimate was that we
needed at least 75-bits in order to be protected against brute force
attacks.  It's been two *deacdes* years later, and granted Moore's law
has ceased to apply in the last couple of years, but I'm sure 64 bits
is not enough.

What is your specific concern vis-a-vis when userspace starts?  We now
print a warning if someone tries to draw from /dev/urandom, and so it
should be easy to see if someone is doing something dangerous.  The
have only been known cases (at last as far asI know where) where some
software was doing something as *insane* as to create keys right out
of the box was.  One was ssh, and at least on a modern Debian system,
that doesn't happen until fairly late in the process:

% systemd-analyze critical-chain ssh.service
The time after the unit is active or started is printed after the "@" character.
The time the unit takes to start is printed after the "+" character.

ssh.service +888ms
└─network.target @31.473s
  └─wpa_supplicant.service @32.958s +770ms
    └─basic.target @19.479s
      └─sockets.target @19.479s
        └─acpid.socket @19.479s
          └─sysinit.target @19.414s
            └─systemd-timesyncd.service @18.079s +1.330s
              └─systemd-tmpfiles-setup.service @17.512s +78ms
                └─local-fs.target @17.501s
                  └─run-user-15806.mount @43.047s
                    └─local-fs-pre.target @16.616s
                      └─systemd-tmpfiles-setup-dev.service @755ms +930ms
                        └─kmod-static-nodes.service @729ms +17ms
                          └─system.slice @653ms
                            └─-.slice @608ms

The other was HP, which was generating an RSA key very shortly after
the first time the printer was powered on.

> We can switch to more conservative estimates when system is fully
> running. But IMO it is very important to get _some_ randomness at the
> begining...

We're doing this already in the latest getrandom(2) implementation.
For the purposes of initializing the crng, we assume that each
interrupt has a single bit of entropy.  So it requires 128 initerrupts
for getrandom(2) to be fully initialized.  I'm actually worried that
this is too high as it is for architectures that don't have a
fine-grained clock.  Given that on many of these embedded platforms
there is a oscillator which drives all of the clocks and subsystems,
it just doesn't make *sense* that than each interrupt could result in
5-6 bits of entropy, no matter what a magical statistical formula
might say.

(Creation of some completely determinsitic sequences that cause the
magical statistcal formulas to claim a vast number of entropy bits is
left as an exercise to the reader.)

Cheers, 

							- Ted

^ permalink raw reply

* [PATCH] crypto: qat - fix aes-xts key sizes
From: Giovanni Cabiddu @ 2016-08-18 18:53 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, Giovanni Cabiddu

Increase value of supported key sizes for qat_aes_xts.
aes-xts keys consists of keys of equal size concatenated.

Reported-by: Wenqian Yu <wenqian.yu@intel.com>
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_common/qat_algs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 769148d..20f35df 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -1260,8 +1260,8 @@ static struct crypto_alg qat_algs[] = { {
 			.setkey = qat_alg_ablkcipher_xts_setkey,
 			.decrypt = qat_alg_ablkcipher_decrypt,
 			.encrypt = qat_alg_ablkcipher_encrypt,
-			.min_keysize = AES_MIN_KEY_SIZE,
-			.max_keysize = AES_MAX_KEY_SIZE,
+			.min_keysize = 2 * AES_MIN_KEY_SIZE,
+			.max_keysize = 2 * AES_MAX_KEY_SIZE,
 			.ivsize = AES_BLOCK_SIZE,
 		},
 	},
-- 
1.7.4.1

^ permalink raw reply related

* Re: [PATCH v6 0/5] /dev/random - a new approach
From: Theodore Ts'o @ 2016-08-19  2:49 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Stephan Mueller, herbert, sandyinchina, Jason Cooper, John Denker,
	H. Peter Anvin, Joe Perches, George Spelvin, linux-crypto,
	linux-kernel
In-Reply-To: <20160818183923.GA24817@amd>

On Thu, Aug 18, 2016 at 08:39:23PM +0200, Pavel Machek wrote:
> 
> But this is the scary part. Not limited to ssh. "We perform the
> largest ever network survey of TLS and SSH servers and present
> evidence that vulnerable keys are surprisingly widespread. We find
> that 0.75% of TLS certificates share keys due to insufficient entropy
> during key generation, and we suspect that another 1.70% come from the
> same faulty implementations and may be susceptible to compromise.
> Even more alarmingly, we are able to obtain RSA private keys for 0.50%
> of TLS hosts and 0.03% of SSH hosts, because their public keys shared
> nontrivial common factors due to entropy problems, and DSA private
> keys for 1.03% of SSH hosts, because of insufficient signature
> randomness"
> 
> https://factorable.net/weakkeys12.conference.pdf

That's a very old paper, and we've made a lot of changes since then.
Before that we weren't accumulating entropy from the interrupt
handler, but only from spinning disk drives, some network interrupts
(but not from all NIC's; it was quite arbitrary), and keyboard and
mouse interrupts.  So hours and hours could go by and you still
wouldn't have accumulated much entropy.

> From my point of view, it would make sense to factor time from RTC and
> mac addresses into the initial hash. Situation in the paper was so bad
> some devices had _completely identical_ keys. We should be able to do
> better than that.

We fixed that **years** ago.  In fact, the authors shared with me an
early look at that paper and I implemented add_device_entropy() over
the July 4th weekend back in 2012.  So we are indeed mixing in MAC
addresses and the hardware clock (if it is initialized that early).
In fact that was one of the first things that I did.  Note that this
doesn't really add much entropy, but it does prevent the GCD attack
from demonstrating completely identical keys.  Hence, we had
remediations in the mainline kernel before the factorable.net paper
was published (not that really helped with devices with embedded
Linux, especially since device manufactures don't see anything wrong
with shipping machines with kernels that are years and years out of
date --- OTOH, these systems were probably also shipping with dozens
of known exploitable holes in userspace, if that's any comfort.
Probably not much if you were planning on deploying lots of IOT
devices in your home network.  :-)

> BTW... 128 interrupts... that's 1.3 seconds, right? Would it make
> sense to wait two seconds if urandom use is attempted before it is
> ready?

That really depends on the system.  We can't assume that people are
using systems with a 100Hz clock interrupt.  More often than not
people are using tickless kernels these days.  That's actually the
problem with changing /dev/urandom to block until things are
initialized.

If you do that, then on some system Python will use /dev/urandom to
initialize a salt used by the Python dictionaries, to protect against
DOS attacks when Python is used to run web scripts.  This is a
completely irrelevant reason when Python is being used for systemd
generator scripts in early boot, and if /dev/urandom were to block,
then the system ends up doing nothing, and on a tickless kernels hours
and hours can go by on a VM and Python would still be blocked on
/dev/urandom.  And since none of the system scripts are running, there
are no interrupts, and so Python ends up blocking on /dev/urandom for
a very long time.  (Eventually someone will start trying to brute
force passwords on the VM's ssh port, assuming that the VM's firewall
rules allow this, and that will cause interrupts that will eventually
initialize /dev/urandom.  But that could take hours.)

And this, boys and girls, is why we can't make /dev/urandom block
until its pool is initialized.  There's too great of a chance that we
will break userspace, and then Linus will yell at us and revert the
commit.

						- Ted

^ permalink raw reply

* Re: [PATCH -next] crypto: fix missing unlock on error in sun4i_hash()
From: Corentin LABBE @ 2016-08-19  5:40 UTC (permalink / raw)
  To: Wei Yongjun, Herbert Xu, David S. Miller, Maxime Ripard,
	Chen-Yu Tsai
  Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <1471560130-5265-1-git-send-email-weiyj.lk@gmail.com>

On 19/08/2016 00:42, Wei Yongjun wrote:
> Add the missing unlock before return from function sun4i_hash()
> in the error handling case.
> 
> Fixes: 477d9b2e591b ("crypto: sun4i-ss - unify update/final function")
> Signed-off-by: Wei Yongjun <weiyj.lk@gmail.com>
> ---
>  drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
> index 2ee3b59..de66f47 100644
> --- a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
> @@ -245,6 +245,7 @@ int sun4i_hash(struct ahash_request *areq)
>  		if (end > areq->nbytes || areq->nbytes - end > 63) {
>  			dev_err(ss->dev, "ERROR: Bound error %u %u\n",
>  				end, areq->nbytes);
> +			spin_unlock(&ss->slock);
>  			return -EINVAL;
>  		}
>  	} else {
> 

Hello

Thanks for the finding, but it is better in that case to use the goto release_ss since it need also to stop the device.

Regards

LABBE Corentin

^ permalink raw reply

* Re: [PATCH v6 0/5] /dev/random - a new approach
From: Herbert Xu @ 2016-08-19  5:56 UTC (permalink / raw)
  To: Theodore Ts'o, Pavel Machek, Stephan Mueller, sandyinchina,
	Jason Cooper, John Denker, H. Peter Anvin, Joe Perches,
	George Spelvin, linux-crypto, linux-kernel
In-Reply-To: <20160819024947.GA10888@thunk.org>

On Thu, Aug 18, 2016 at 10:49:47PM -0400, Theodore Ts'o wrote:
>
> That really depends on the system.  We can't assume that people are
> using systems with a 100Hz clock interrupt.  More often than not
> people are using tickless kernels these days.  That's actually the
> problem with changing /dev/urandom to block until things are
> initialized.

Couldn't we disable tickless until urandom has been seeded? In fact
perhaps we should accelerate the timer interrupt rate until it has
been seeded?

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: [PATCHv3 net-next 0/4] crypto/chcr: Add support for Chelsio Crypto Driver
From: David Miller @ 2016-08-19  6:11 UTC (permalink / raw)
  To: hariprasad
  Cc: netdev, linux-crypto, herbert, atul.gupta, yeshaswi, jlulla,
	harsh
In-Reply-To: <1471417386-14026-1-git-send-email-hariprasad@chelsio.com>

From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Wed, 17 Aug 2016 12:33:02 +0530

> This patch series adds support for Chelsio Crypto driver. 

Herbert, what do you want to do with this?  I can push it via
net-next if you like.

^ permalink raw reply

* Re: [PATCHv3 net-next 3/4] chcr: Support for Chelsio's Crypto Hardware
From: Herbert Xu @ 2016-08-19  6:15 UTC (permalink / raw)
  To: Hariprasad Shenai
  Cc: netdev, linux-crypto, davem, atul.gupta, yeshaswi, jlulla, harsh
In-Reply-To: <1471417386-14026-4-git-send-email-hariprasad@chelsio.com>

On Wed, Aug 17, 2016 at 12:33:05PM +0530, Hariprasad Shenai wrote:
> The Chelsio's Crypto Hardware can perform the following operations:
> SHA1, SHA224, SHA256, SHA384 and SHA512, HMAC(SHA1), HMAC(SHA224),
> HMAC(SHA256), HMAC(SHA384), HAMC(SHA512), AES-128-CBC, AES-192-CBC,
> AES-256-CBC, AES-128-XTS, AES-256-XTS
> 
> This patch implements the driver for above mentioned features. This
> driver is an Upper Layer Driver which is attached to Chelsio's LLD
> (cxgb4) and uses the queue allocated by the LLD for sending the crypto
> requests to the Hardware and receiving the responses from it.
> 
> The crypto operations can be performed by Chelsio's hardware from the
> userspace applications and/or from within the kernel space using the
> kernel's crypto API.
> 
> The above mentioned crypto features have been tested using kernel's
> tests mentioned in testmgr.h. They also have been tested from user
> space using libkcapi and Openssl.
> 
> Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
-- 
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: [PATCHv3 net-next 0/4] crypto/chcr: Add support for Chelsio Crypto Driver
From: Herbert Xu @ 2016-08-19  6:15 UTC (permalink / raw)
  To: David Miller
  Cc: hariprasad, netdev, linux-crypto, atul.gupta, yeshaswi, jlulla,
	harsh
In-Reply-To: <20160818.231101.728993368913367217.davem@davemloft.net>

On Thu, Aug 18, 2016 at 11:11:01PM -0700, David Miller wrote:
> From: Hariprasad Shenai <hariprasad@chelsio.com>
> Date: Wed, 17 Aug 2016 12:33:02 +0530
> 
> > This patch series adds support for Chelsio Crypto driver. 
> 
> Herbert, what do you want to do with this?  I can push it via
> net-next if you like.

Sure thing, the crypto part looks good to me.  Thanks Dave!
-- 
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: [PATCHv3 net-next 0/4] crypto/chcr: Add support for Chelsio Crypto Driver
From: David Miller @ 2016-08-19  7:01 UTC (permalink / raw)
  To: herbert
  Cc: hariprasad, netdev, linux-crypto, atul.gupta, yeshaswi, jlulla,
	harsh
In-Reply-To: <20160819061543.GA20764@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 19 Aug 2016 14:15:43 +0800

> On Thu, Aug 18, 2016 at 11:11:01PM -0700, David Miller wrote:
>> From: Hariprasad Shenai <hariprasad@chelsio.com>
>> Date: Wed, 17 Aug 2016 12:33:02 +0530
>> 
>> > This patch series adds support for Chelsio Crypto driver. 
>> 
>> Herbert, what do you want to do with this?  I can push it via
>> net-next if you like.
> 
> Sure thing, the crypto part looks good to me.  Thanks Dave!

Great, done.

^ permalink raw reply

* Re: [PATCH v6 0/5] /dev/random - a new approach
From: Pavel Machek @ 2016-08-19  7:48 UTC (permalink / raw)
  To: Theodore Ts'o, Stephan Mueller, herbert, sandyinchina,
	Jason Cooper, John Denker, H. Peter Anvin, Joe Perches,
	George Spelvin, linux-crypto, linux-kernel
In-Reply-To: <20160819024947.GA10888@thunk.org>

Hi!

> > From my point of view, it would make sense to factor time from RTC and
> > mac addresses into the initial hash. Situation in the paper was so bad
> > some devices had _completely identical_ keys. We should be able to do
> > better than that.
> 
> We fixed that **years** ago.  In fact, the authors shared with me an
> early look at that paper and I implemented add_device_entropy() over
> the July 4th weekend back in 2012.  So we are indeed mixing in MAC
> addresses and the hardware clock (if it is initialized that early).
> In fact that was one of the first things that I did.  Note that this

Ok, thanks.

> > BTW... 128 interrupts... that's 1.3 seconds, right? Would it make
> > sense to wait two seconds if urandom use is attempted before it is
> > ready?
> 
> That really depends on the system.  We can't assume that people are
> using systems with a 100Hz clock interrupt.  More often than not
> people are using tickless kernels these days.  That's actually the
> problem with changing /dev/urandom to block until things are
> initialized.

Ok, let me check:

config HZ_PERIODIC
config NO_HZ_IDLE
config NO_HZ_FULL

in HZ_PERIODIC, there should be no problem.

NO_HZ_IDLE... should not be a problem either. We can easily make sure
that cpu's are not idle, something like 

     while (not_enough_entropy())
     	   schedule()

NO_HZ_FULL.... first, help text seems to imply that timer ticks still
happen when cpu is in kernel, and second, there is always one CPU that
handles timer ticks. So we are still ok.

So I believe we should add the wait to urandom. One second delay in
rare cases sounds better than alternatives.

Best regards,
									Pavel
PS: Are there systems where the timer interrupt is the only source of time?
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: Jeffrey Walton @ 2016-08-19  9:47 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: linux-crypto, devicetree, linux-kernel, linux-mips
In-Reply-To: <1471448151-20850-1-git-send-email-prasannatsmkumar@gmail.com>

On Wed, Aug 17, 2016 at 11:35 AM, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.com> wrote:
> This patch adds support for hardware random number generator present in
> JZ4780 SoC.
>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  .../devicetree/bindings/rng/ingenic,jz4780-rng.txt |  12 +++
>  MAINTAINERS                                        |   5 +
>  arch/mips/boot/dts/ingenic/jz4780.dtsi             |   7 +-
>  drivers/char/hw_random/Kconfig                     |  14 +++
>  drivers/char/hw_random/Makefile                    |   1 +
>  drivers/char/hw_random/jz4780-rng.c                | 105 +++++++++++++++++++++
>  6 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
>  create mode 100644 drivers/char/hw_random/jz4780-rng.c
>
> diff --git a/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt b/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
> new file mode 100644
> index 0000000..03abf56
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
> @@ -0,0 +1,12 @@
> +Ingenic jz4780 RNG driver
> +
> +Required properties:
> +- compatible : Should be "ingenic,jz4780-rng"
> +- reg : Specifies base physical address and size of the registers.
> +
> +Example:
> +
> +rng: rng@100000D8 {
> +       compatible = "ingenic,jz4780-rng";
> +       reg = <0x100000D8 0x8>;
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 08e9efe..c0c66eb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6002,6 +6002,11 @@ M:       Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>  S:     Maintained
>  F:     drivers/dma/dma-jz4780.c
>
> +INGENIC JZ4780 HW RNG Driver
> +M:     PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> +S:     Maintained
> +F:     drivers/char/hw_random/jz4780-rng.c
> +
>  INTEGRITY MEASUREMENT ARCHITECTURE (IMA)
>  M:     Mimi Zohar <zohar@linux.vnet.ibm.com>
>  M:     Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index b868b42..f11d139 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -36,7 +36,7 @@
>
>         cgu: jz4780-cgu@10000000 {
>                 compatible = "ingenic,jz4780-cgu";
> -               reg = <0x10000000 0x100>;
> +               reg = <0x10000000 0xD8>;
>
>                 clocks = <&ext>, <&rtc>;
>                 clock-names = "ext", "rtc";
> @@ -44,6 +44,11 @@
>                 #clock-cells = <1>;
>         };
>
> +       rng: jz4780-rng@100000D8 {
> +               compatible = "ingenic,jz4780-rng";
> +               reg = <0x100000D8 0x8>;
> +       };
> +
>         uart0: serial@10030000 {
>                 compatible = "ingenic,jz4780-uart";
>                 reg = <0x10030000 0x100>;
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 56ad5a59..c336fe8 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -294,6 +294,20 @@ config HW_RANDOM_POWERNV
>
>           If unsure, say Y.
>
> +config HW_RANDOM_JZ4780
> +       tristate "JZ4780 HW random number generator support"
> +       depends on MACH_INGENIC
> +       depends on HAS_IOMEM
> +       default HW_RANDOM
> +       ---help---
> +         This driver provides kernel-side support for the Random Number
> +         Generator hardware found on JZ4780 SOCs.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called jz4780-rng.
> +
> +         If unsure, say Y.
> +
>  config HW_RANDOM_EXYNOS
>         tristate "EXYNOS HW random number generator support"
>         depends on ARCH_EXYNOS || COMPILE_TEST
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 04bb0b0..a155066 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -26,6 +26,7 @@ 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_JZ4780) += jz4780-rng.o
>  obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
>  obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
>  obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o
> diff --git a/drivers/char/hw_random/jz4780-rng.c b/drivers/char/hw_random/jz4780-rng.c
> new file mode 100644
> index 0000000..c9d2cde
> --- /dev/null
> +++ b/drivers/char/hw_random/jz4780-rng.c
> @@ -0,0 +1,105 @@
> +/*
> + * jz4780-rng.c - Random Number Generator driver for J4780
> + *
> + * Copyright 2016 (C) PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> + *
> + * This file is licensed under  the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/hw_random.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +
> +#define REG_RNG_CTRL   0x0
> +#define REG_RNG_DATA   0x4
> +
> +struct jz4780_rng {
> +       struct device *dev;
> +       struct hwrng rng;
> +       void __iomem *mem;
> +};
> +
> +static u32 jz4780_rng_readl(struct jz4780_rng *rng, u32 offset)
> +{
> +       return readl(rng->mem + offset);
> +}
> +
> +static void jz4780_rng_writel(struct jz4780_rng *rng, u32 val, u32 offset)
> +{
> +       writel(val, rng->mem + offset);
> +}
> +
> +static int jz4780_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> +       struct jz4780_rng *jz4780_rng = container_of(rng, struct jz4780_rng,
> +                                                       rng);
> +       u32 *data = buf;
> +       *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
> +       return 4;
> +}
> +
> +static int jz4780_rng_probe(struct platform_device *pdev)
> +{
> +       struct jz4780_rng *jz4780_rng;
> +       struct resource *res;
> +       resource_size_t size;
> +       int ret;
> +
> +       jz4780_rng = devm_kzalloc(&pdev->dev, sizeof(struct jz4780_rng),
> +                                       GFP_KERNEL);
> +       if (!jz4780_rng)
> +               return -ENOMEM;
> +
> +       jz4780_rng->dev = &pdev->dev;
> +       jz4780_rng->rng.name = "jz4780";
> +       jz4780_rng->rng.read = jz4780_rng_read;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       size = resource_size(res);
> +
> +       jz4780_rng->mem = devm_ioremap(&pdev->dev, res->start, size);
> +       if (IS_ERR(jz4780_rng->mem))
> +               return PTR_ERR(jz4780_rng->mem);
> +
> +       platform_set_drvdata(pdev, jz4780_rng);
> +       jz4780_rng_writel(jz4780_rng, 1, REG_RNG_CTRL);
> +       ret = hwrng_register(&jz4780_rng->rng);
> +
> +       return ret;
> +}
> +
> +static int jz4780_rng_remove(struct platform_device *pdev)
> +{
> +       struct jz4780_rng *jz4780_rng = platform_get_drvdata(pdev);
> +
> +       jz4780_rng_writel(jz4780_rng, 0, REG_RNG_CTRL);
> +       hwrng_unregister(&jz4780_rng->rng);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id jz4780_rng_dt_match[] = {
> +       { .compatible = "ingenic,jz4780-rng", },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, jz4780_rng_dt_match);
> +
> +static struct platform_driver jz4780_rng_driver = {
> +       .driver         = {
> +               .name   = "jz4780-rng",
> +               .of_match_table = jz4780_rng_dt_match,
> +       },
> +       .probe          = jz4780_rng_probe,
> +       .remove         = jz4780_rng_remove,
> +};
> +module_platform_driver(jz4780_rng_driver);
> +
> +MODULE_DESCRIPTION("Ingenic JZ4780 H/W Random Number Generator driver");
> +MODULE_AUTHOR("PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>");
> +MODULE_LICENSE("GPL");
> --

Please forgive my ignorance Prasanna...

For the JZ4780 I have, there are two registers in play. The first is
the control register which enables/disables the RNG. The control
register is named ERNG. The second register is the data register, and
it produces the random stream. The data register is named RNG. ERNG is
located at 0x100000D8 and RNG is located at 0x100000DC. This kind of
confuses me because I don't see where 0x100000D8 is ever added to
those values (maybe its in the descriptor?):

+#define REG_RNG_CTRL   0x0
+#define REG_RNG_DATA   0x4


Also, testing with a userland PoC for the device, you have to throttle
reads from RNG register. If reads occur with a 0 delay, then the
random value appears fixed. If the delay is too small, then you can
watch random values being shifted-in in a barrel like fashion.
Unfortunately, the manual did not discuss how long to wait for a value
to be ready. I found spinning in a loop for 5000 was too small and
witnessed the shifting; while spinning in a loop for 10000 avoided the
shift observation. I don't what number of JIFFIES that translates to.


Finally, from looking at the native Ingenic driver (which was not very
impressive), they enabled/disabled the RNG register on demand. There
was also a [possible related] note in the manual about not applying
VCC for over a second. I can only say "possibly related" because I was
not sure if the register was part of the controller they were
discussing. The userland PoC worked fine when enabling/disabling the
RNG register. So I'm not sure about this (from jz4780_rng_probe):

+       platform_set_drvdata(pdev, jz4780_rng);
+       jz4780_rng_writel(jz4780_rng, 1, REG_RNG_CTRL);
+       ret = hwrng_register(&jz4780_rng->rng);

And this (from jz4780_rng_remove):

+       jz4780_rng_writel(jz4780_rng, 0, REG_RNG_CTRL);
+       hwrng_unregister(&jz4780_rng->rng);

Anyway, I hope that helps you avoid some land mines (if they are present).

Jeff

^ permalink raw reply

* Re: [PATCH] Add Ingenic JZ4780 hardware RNG driver
From: Jeffrey Walton @ 2016-08-19 10:55 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: linux-crypto, devicetree, linux-kernel, linux-mips
In-Reply-To: <1471448151-20850-1-git-send-email-prasannatsmkumar@gmail.com>

On Wed, Aug 17, 2016 at 11:35 AM, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.com> wrote:
> This patch adds support for hardware random number generator present in
> JZ4780 SoC.
>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  ...
> +static int jz4780_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> +       struct jz4780_rng *jz4780_rng = container_of(rng, struct jz4780_rng,
> +                                                       rng);
> +       u32 *data = buf;
> +       *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);
> +       return 4;
> +}

My bad, I should have spotted this earlier....

i686, x86_64 and some ARM will sometimes define a macro indicating
unaligned data access is allowed. For example, see
__ARM_FEATURE_UNALIGNED (cf.,
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0774f/chr1383660321827.html)
. MIPSEL does not define such a macro.

    # MIPS ci20 creator with GCC 4.6
    $ gcc -march=native -dM -E - </dev/null | grep -i align
    #define __BIGGEST_ALIGNMENT__ 8

If the MIPS CPU does not tolerate unaligned data access, then the
following could SIGBUS:

> +       u32 *data = buf;
> +       *data = jz4780_rng_readl(jz4780_rng, REG_RNG_DATA);

If GCC emits code that uses the MIPS unaligned load and store
instructions, then there's probably going to be a performance penalty.

Regardless of what the CPU tolerates, I believe unaligned data access
is undefined behavior in C/C++. I believe you should memcpy the value
into the buffer.

Jeff

^ permalink raw reply


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