Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Herbert Xu @ 2016-08-09  9:56 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Pan, Miaoqing, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <12791731.pG3fmEhvyp@tauon.atsec.com>

On Tue, Aug 09, 2016 at 11:56:08AM +0200, Stephan Mueller wrote:
> Am Dienstag, 9. August 2016, 17:46:56 CEST schrieb Herbert Xu:
> 
> Hi Herbert,
> > 
> > You're supposed to tweak the quality of the input.  In any case,
> 
> How is that tweak supposed to happen? The rngd does not allow changing the 
> amount of read data relative to the assumed entropy.

Hmm, I guess it depends on your distro.  Some do.

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: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-09  9:56 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Pan, Miaoqing, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <20160809094656.GB6529@gondor.apana.org.au>

Am Dienstag, 9. August 2016, 17:46:56 CEST schrieb Herbert Xu:

Hi Herbert,
> 
> You're supposed to tweak the quality of the input.  In any case,

How is that tweak supposed to happen? The rngd does not allow changing the 
amount of read data relative to the assumed entropy.

> this is not affected by whether we whiten the result.

I understand that.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] hwrng: core - Allow for multiple simultaneous active hwrng devices
From: Herbert Xu @ 2016-08-09  9:50 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-crypto, linux-kernel
In-Reply-To: <1469477255-26824-1-git-send-email-keithp@keithp.com>

On Mon, Jul 25, 2016 at 01:07:35PM -0700, Keith Packard wrote:
> Instead of having only one hwrng feeding /dev/random at a time, maintain
> a list of devices and cycle between them when filling the entropy pool.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

So you're cycling RNGs even for user-space reads? That could be
problematic because not all hardware RNGs carry the maximum amount
of entropy.  It would be rather annoying to be cycling between
RNGs of different qualities.

Perhaps only cycle for the kernel hwrngd?

Thanks,
-- 
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: [PATCH 0/7] Various fixes for the cesa driver
From: Thomas Petazzoni @ 2016-08-09  9:48 UTC (permalink / raw)
  To: Romain Perier
  Cc: Boris Brezillon, Arnaud Ebalard, Russell King, linux-crypto,
	Gregory Clement, David S. Miller, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

Hello,

Is it normal that Herbert, as the crypto maintainer, is not Cc'ed on
those patches?

On Tue,  9 Aug 2016 11:03:13 +0200, Romain Perier wrote:
> This patches series contains various fixes for ahash requests, dma
> operations and an important fixe in the core of the driver (cesa.c). It
> also includes some code cleanups.
> 
> Romain Perier (3):
>   crypto: marvell - Update transformation context for each dequeued req
>   crypto: marvell - Don't overwrite default creq->state during
>     initialization

I think those two patches should have come first in the series, to make
it clear that they are the two fixes that are important to merge for
the 4.8 release cycle.

>   crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req
> 
> Thomas Petazzoni (4):
>   crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
>   crypto: marvell: remove unused parameter in
>     mv_cesa_ahash_dma_add_cache()
>   crypto: marvell: turn mv_cesa_ahash_init() into a function returning
>     void
>   crypto: marvell: make mv_cesa_ahash_cache_req() return bool

Those 5 other patches (the four from me and the last one from you) are
more cleanups/improvements, which can wait the 4.9 release cycle.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Herbert Xu @ 2016-08-09  9:46 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Pan, Miaoqing, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <1645997.7cVzaEi3NG@tauon.atsec.com>

On Tue, Aug 09, 2016 at 11:37:39AM +0200, Stephan Mueller wrote:
> Am Dienstag, 9. August 2016, 17:17:55 CEST schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > On Tue, Aug 09, 2016 at 11:02:58AM +0200, Stephan Mueller wrote:
> > > But shouldn't the default of the rngd then be adjusted a bit?
> > 
> > Please elaborate.
> 
> in rngd_linux.c:random_add_entropy(void *buf, size_t size):
> 
>         entropy.ent_count = size * 8;
>         entropy.size = size;
>         memcpy(entropy.data, buf, size);
> 
>         if (ioctl(random_fd, RNDADDENTROPY, &entropy) != 0) {
> 
> ...
> 
> 
> in rngd.c:do_loop():
> 
>                         retval = iter->xread(buf, sizeof buf, iter);
> ...
>                         rc = update_kernel_random(random_step,
>                                              buf, iter->fipsctx);
> 
> where update_kernel_random simply invokes random_add_entropy in chunks.
> 
> Hence, the rngd reads some bytes from /dev/hwrand and injects it into /dev/
> random with an entropy estimate that is equal to the read bytes.
> 
> With less than perfect noise sources, entropy.ent_count should be much 
> smaller.

You're supposed to tweak the quality of the input.  In any case,
this is not affected by whether we whiten the result.

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: AF_ALG zero-size hash fails
From: Herbert Xu @ 2016-08-09  9:45 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: noloader, linux-crypto
In-Reply-To: <20160809091834.GH1041@n2100.armlinux.org.uk>

On Tue, Aug 09, 2016 at 10:18:34AM +0100, Russell King - ARM Linux wrote:
> Hi,
> 
> While testing AF_ALG with openssl af-alg-rr, I've found that:
> 
> OPENSSL_CONF=/shared/crypto/openssl-imx.cnf openssl dgst -sha1 </dev/null
> 
> fails with a zero hash result:
> 
> socket(PF_ALG, SOCK_SEQPACKET, 0)       = 3
> close(3)                                = 0
> socket(PF_ALG, SOCK_SEQPACKET, 0)       = 3
> bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0
> accept(3, 0, NULL)                      = 4
> fstat64(0, {st_mode=S_IFCHR|0666, st_rdev=makedev(1, 3), ...}) = 0
> ioctl(0, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 0xbed50d5c) = -1 ENOTTY (Inappropriate ioctl for device)
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe4000
> read(0, "", 8192)                       = 0
> read(4, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 20) = 20
> close(4)                                = 0
> close(3)                                = 0
> 
> tested with the Freescale CAAM driver with SHA1 and MD5 hashes, and
> the ARM SHA1 shash implementation.  Should there at least be a single
> write to the socket (of zero size) in this case, or should the kernel
> return the correct hash on the first read without a preceding
> write/send?

It's an oversight in the algif_hash code.  I'll get it fixed.

Thanks,
-- 
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: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-09  9:37 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Pan, Miaoqing, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <20160809091755.GA6370-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>

Am Dienstag, 9. August 2016, 17:17:55 CEST schrieb Herbert Xu:

Hi Herbert,

> On Tue, Aug 09, 2016 at 11:02:58AM +0200, Stephan Mueller wrote:
> > But shouldn't the default of the rngd then be adjusted a bit?
> 
> Please elaborate.

in rngd_linux.c:random_add_entropy(void *buf, size_t size):

        entropy.ent_count = size * 8;
        entropy.size = size;
        memcpy(entropy.data, buf, size);

        if (ioctl(random_fd, RNDADDENTROPY, &entropy) != 0) {

...


in rngd.c:do_loop():

                        retval = iter->xread(buf, sizeof buf, iter);
...
                        rc = update_kernel_random(random_step,
                                             buf, iter->fipsctx);

where update_kernel_random simply invokes random_add_entropy in chunks.

Hence, the rngd reads some bytes from /dev/hwrand and injects it into /dev/
random with an entropy estimate that is equal to the read bytes.

With less than perfect noise sources, entropy.ent_count should be much 
smaller.
> 
> Thanks,



Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* AF_ALG zero-size hash fails
From: Russell King - ARM Linux @ 2016-08-09  9:18 UTC (permalink / raw)
  To: Herbert Xu; +Cc: noloader, linux-crypto

Hi,

While testing AF_ALG with openssl af-alg-rr, I've found that:

OPENSSL_CONF=/shared/crypto/openssl-imx.cnf openssl dgst -sha1 </dev/null

fails with a zero hash result:

socket(PF_ALG, SOCK_SEQPACKET, 0)       = 3
close(3)                                = 0
socket(PF_ALG, SOCK_SEQPACKET, 0)       = 3
bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0
accept(3, 0, NULL)                      = 4
fstat64(0, {st_mode=S_IFCHR|0666, st_rdev=makedev(1, 3), ...}) = 0
ioctl(0, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 0xbed50d5c) = -1 ENOTTY (Inappropriate ioctl for device)
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fe4000
read(0, "", 8192)                       = 0
read(4, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 20) = 20
close(4)                                = 0
close(3)                                = 0

tested with the Freescale CAAM driver with SHA1 and MD5 hashes, and
the ARM SHA1 shash implementation.  Should there at least be a single
write to the socket (of zero size) in this case, or should the kernel
return the correct hash on the first read without a preceding
write/send?

I don't think this is a regression afaik - it's been around for some
time (at least from when I first started to poke at the ARM crypto
stuff around v4.2 or v4.3 time.)

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Herbert Xu @ 2016-08-09  9:17 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Pan, Miaoqing, Matt Mackall,
	miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Valo, Kalle,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel, linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, Sepehrdad, Pouyan
In-Reply-To: <2569442.q63FVBJjUH-gNvIQDDl/k7Ia13z/PHSgg@public.gmane.org>

On Tue, Aug 09, 2016 at 11:02:58AM +0200, Stephan Mueller wrote:
> 
> But shouldn't the default of the rngd then be adjusted a bit?

Please elaborate.

Thanks,
-- 
Email: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 7/7] crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

Don't use 64 'as is', as max block size in mv_cesa_ahash_cache_req. Use
CESA_MAX_HASH_BLOCK_SIZE instead, this is better for readability.

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

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 44a8abe..9f28468 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -408,7 +408,7 @@ static bool mv_cesa_ahash_cache_req(struct ahash_request *req)
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
 	bool cached = false;
 
-	if (creq->cache_ptr + req->nbytes < 64 && !creq->last_req) {
+	if (creq->cache_ptr + req->nbytes < CESA_MAX_HASH_BLOCK_SIZE && !creq->last_req) {
 		cached = true;
 
 		if (!req->nbytes)
-- 
2.8.1

^ permalink raw reply related

* [PATCH 5/7] crypto: marvell - Update transformation context for each dequeued req
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

So far, sub part of mv_cesa_int was responsible of dequeuing complete
requests, then call the 'cleanup' operation on these reqs and call the
crypto api callback 'complete'. The problem is that the transformation
context 'ctx' is retrieved only once before the while loop. Which means
that the wrong 'cleanup' operation might be called on the wrong type of
cesa requests, it can lead to memory corruptions with this message:

marvell-cesa f1090000.crypto: dma_pool_free cesa_padding, 5a5a5a5a/5a5a5a5a (bad dma)

This commit fixes the issue, by updating the transformation context for
each dequeued cesa request.

Fixes: commit 85030c5168f1 ("crypto: marvell - Add support for chai...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/cesa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index d64af86..37dadb2 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -166,6 +166,7 @@ static irqreturn_t mv_cesa_int(int irq, void *priv)
 			if (!req)
 				break;
 
+			ctx = crypto_tfm_ctx(req->tfm);
 			mv_cesa_complete_req(ctx, req, 0);
 		}
 	}
-- 
2.8.1

^ permalink raw reply related

* [PATCH 6/7] crypto: marvell - Don't overwrite default creq->state during initialization
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

Currently, in mv_cesa_{md5,sha1,sha256}_init creq->state is initialized
before the call to mv_cesa_ahash_init. This is wrong because this
function fills creq with zero by using memset, so its 'state' that
contains the default DIGEST is overwritten. This commit fixes the issue
by initializing creq->state just after the call to mv_cesa_ahash_init.

Fixes: commit b0ef51067cb4 ("crypto: marvell/cesa - initialize hash...")
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index cf8063d..44a8abe 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -800,13 +800,14 @@ static int mv_cesa_md5_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
+
+	mv_cesa_ahash_init(req, &tmpl, true);
+
 	creq->state[0] = MD5_H0;
 	creq->state[1] = MD5_H1;
 	creq->state[2] = MD5_H2;
 	creq->state[3] = MD5_H3;
 
-	mv_cesa_ahash_init(req, &tmpl, true);
-
 	return 0;
 }
 
@@ -868,14 +869,15 @@ static int mv_cesa_sha1_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA1);
+
+	mv_cesa_ahash_init(req, &tmpl, false);
+
 	creq->state[0] = SHA1_H0;
 	creq->state[1] = SHA1_H1;
 	creq->state[2] = SHA1_H2;
 	creq->state[3] = SHA1_H3;
 	creq->state[4] = SHA1_H4;
 
-	mv_cesa_ahash_init(req, &tmpl, false);
-
 	return 0;
 }
 
@@ -937,6 +939,9 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 	struct mv_cesa_op_ctx tmpl = { };
 
 	mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA256);
+
+	mv_cesa_ahash_init(req, &tmpl, false);
+
 	creq->state[0] = SHA256_H0;
 	creq->state[1] = SHA256_H1;
 	creq->state[2] = SHA256_H2;
@@ -946,8 +951,6 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 	creq->state[6] = SHA256_H6;
 	creq->state[7] = SHA256_H7;
 
-	mv_cesa_ahash_init(req, &tmpl, false);
-
 	return 0;
 }
 
-- 
2.8.1

^ permalink raw reply related

* [PATCH 3/7] crypto: marvell: turn mv_cesa_ahash_init() into a function returning void
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_ahash_init() function always returns 0, and the return
value is anyway never checked. Turn it into a function returning void.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 0d7f5f9..7291664 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -374,7 +374,7 @@ static const struct mv_cesa_req_ops mv_cesa_ahash_req_ops = {
 	.complete = mv_cesa_ahash_complete,
 };
 
-static int mv_cesa_ahash_init(struct ahash_request *req,
+static void mv_cesa_ahash_init(struct ahash_request *req,
 			      struct mv_cesa_op_ctx *tmpl, bool algo_le)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
@@ -390,8 +390,6 @@ static int mv_cesa_ahash_init(struct ahash_request *req,
 	creq->op_tmpl = *tmpl;
 	creq->len = 0;
 	creq->algo_le = algo_le;
-
-	return 0;
 }
 
 static inline int mv_cesa_ahash_cra_init(struct crypto_tfm *tfm)
-- 
2.8.1

^ permalink raw reply related

* [PATCH 4/7] crypto: marvell: make mv_cesa_ahash_cache_req() return bool
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_ahash_cache_req() function always returns 0, which makes
its return value pretty much useless. However, in addition to
returning a useless value, it also returns a boolean in a variable
passed by reference to indicate if the request was already cached.

So, this commit changes mv_cesa_ahash_cache_req() to return this
boolean. It consequently simplifies the only call site of
mv_cesa_ahash_cache_req(), where the "ret" variable is no longer
needed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 7291664..cf8063d 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -403,15 +403,16 @@ static inline int mv_cesa_ahash_cra_init(struct crypto_tfm *tfm)
 	return 0;
 }
 
-static int mv_cesa_ahash_cache_req(struct ahash_request *req, bool *cached)
+static bool mv_cesa_ahash_cache_req(struct ahash_request *req)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
+	bool cached = false;
 
 	if (creq->cache_ptr + req->nbytes < 64 && !creq->last_req) {
-		*cached = true;
+		cached = true;
 
 		if (!req->nbytes)
-			return 0;
+			return cached;
 
 		sg_pcopy_to_buffer(req->src, creq->src_nents,
 				   creq->cache + creq->cache_ptr,
@@ -420,7 +421,7 @@ static int mv_cesa_ahash_cache_req(struct ahash_request *req, bool *cached)
 		creq->cache_ptr += req->nbytes;
 	}
 
-	return 0;
+	return cached;
 }
 
 static struct mv_cesa_op_ctx *
@@ -665,7 +666,6 @@ err:
 static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
 {
 	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
-	int ret;
 
 	creq->src_nents = sg_nents_for_len(req->src, req->nbytes);
 	if (creq->src_nents < 0) {
@@ -673,17 +673,15 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
 		return creq->src_nents;
 	}
 
-	ret = mv_cesa_ahash_cache_req(req, cached);
-	if (ret)
-		return ret;
+	*cached = mv_cesa_ahash_cache_req(req);
 
 	if (*cached)
 		return 0;
 
 	if (cesa_dev->caps->has_tdma)
-		ret = mv_cesa_ahash_dma_req_init(req);
-
-	return ret;
+		return mv_cesa_ahash_dma_req_init(req);
+	else
+		return 0;
 }
 
 static int mv_cesa_ahash_queue_req(struct ahash_request *req)
-- 
2.8.1

^ permalink raw reply related

* [PATCH 2/7] crypto: marvell: remove unused parameter in mv_cesa_ahash_dma_add_cache()
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The dma_iter parameter of mv_cesa_ahash_dma_add_cache() is never used,
so get rid of it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 82e0f4e6..0d7f5f9 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -455,7 +455,6 @@ mv_cesa_dma_add_frag(struct mv_cesa_tdma_chain *chain,
 
 static int
 mv_cesa_ahash_dma_add_cache(struct mv_cesa_tdma_chain *chain,
-			    struct mv_cesa_ahash_dma_iter *dma_iter,
 			    struct mv_cesa_ahash_req *creq,
 			    gfp_t flags)
 {
@@ -586,7 +585,7 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
 	 * Add the cache (left-over data from a previous block) first.
 	 * This will never overflow the SRAM size.
 	 */
-	ret = mv_cesa_ahash_dma_add_cache(&basereq->chain, &iter, creq, flags);
+	ret = mv_cesa_ahash_dma_add_cache(&basereq->chain, creq, flags);
 	if (ret)
 		goto err_free_tdma;
 
-- 
2.8.1

^ permalink raw reply related

* [PATCH 0/7] Various fixes for the cesa driver
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel

This patches series contains various fixes for ahash requests, dma
operations and an important fixe in the core of the driver (cesa.c). It
also includes some code cleanups.

Romain Perier (3):
  crypto: marvell - Update transformation context for each dequeued req
  crypto: marvell - Don't overwrite default creq->state during
    initialization
  crypto: marvell - Don't hardcode block size in mv_cesa_ahash_cache_req

Thomas Petazzoni (4):
  crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
  crypto: marvell: remove unused parameter in
    mv_cesa_ahash_dma_add_cache()
  crypto: marvell: turn mv_cesa_ahash_init() into a function returning
    void
  crypto: marvell: make mv_cesa_ahash_cache_req() return bool

 drivers/crypto/marvell/cesa.c |  1 +
 drivers/crypto/marvell/hash.c | 44 +++++++++++++++++++++----------------------
 drivers/crypto/marvell/tdma.c |  1 +
 3 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.8.1

^ permalink raw reply

* [PATCH 1/7] crypto: marvell: be explicit about destination in mv_cesa_dma_add_op()
From: Romain Perier @ 2016-08-09  9:03 UTC (permalink / raw)
  To: Boris Brezillon, Arnaud Ebalard
  Cc: Gregory Clement, Thomas Petazzoni, David S. Miller, Russell King,
	linux-crypto, linux-arm-kernel
In-Reply-To: <1470733400-23621-1-git-send-email-romain.perier@free-electrons.com>

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

The mv_cesa_dma_add_op() function builds a mv_cesa_tdma_desc structure
to copy the operation description to the SRAM, but doesn't explicitly
initialize the destination of the copy. It works fine because the
operatin description must be copied at the beginning of the SRAM, and
the mv_cesa_tdma_desc structure is initialized to zero when
allocated. However, it is somewhat confusing to not have a destination
defined.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/crypto/marvell/tdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
index 86a065b..9fd7a5f 100644
--- a/drivers/crypto/marvell/tdma.c
+++ b/drivers/crypto/marvell/tdma.c
@@ -261,6 +261,7 @@ struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
 	tdma->op = op;
 	tdma->byte_cnt = cpu_to_le32(size | BIT(31));
 	tdma->src = cpu_to_le32(dma_handle);
+	tdma->dst = CESA_SA_CFG_SRAM_OFFSET;
 	tdma->flags = CESA_TDMA_DST_IN_SRAM | CESA_TDMA_OP;
 
 	return op;
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-09  9:02 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Pan, Miaoqing, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <20160809085858.GA6172@gondor.apana.org.au>

Am Dienstag, 9. August 2016, 16:58:58 CEST schrieb Herbert Xu:

Hi Herbert,

> On Tue, Aug 09, 2016 at 10:07:29AM +0200, Stephan Mueller wrote:
> > Herbert, Matt, should such noise sources be added to the HW random
> > framework? The thing is that the in-kernel HW random to input_pool link
> > per default uses a more conservative entropy estimate than the user space
> > rngd. I would think that the in-kernel link would appropriate for that
> > rng. But the user space rngd tool with its default behavior is not really
> > suited here.
> 
> Yes hwrng would be the best fit, with a quality of zero to be safe.
> 
> Contrary to the quoted thread, there is no need to whiten the output
> /dev/hw_random.  It was always meant to go through some intermediate
> processing such as rngd before it is used.

But shouldn't the default of the rngd then be adjusted a bit?
> 
> Cheers,



Ciao
Stephan

^ permalink raw reply

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Herbert Xu @ 2016-08-09  8:58 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Pan, Miaoqing, Matt Mackall, miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <4731753.YZZKimtrHM@tauon.atsec.com>

On Tue, Aug 09, 2016 at 10:07:29AM +0200, Stephan Mueller wrote:
> 
> Herbert, Matt, should such noise sources be added to the HW random framework? 
> The thing is that the in-kernel HW random to input_pool link per default uses 
> a more conservative entropy estimate than the user space rngd. I would think 
> that the in-kernel link would appropriate for that rng. But the user space 
> rngd tool with its default behavior is not really suited here.

Yes hwrng would be the best fit, with a quality of zero to be safe.

Contrary to the quoted thread, there is no need to whiten the output
/dev/hw_random.  It was always meant to go through some intermediate
processing such as rngd before it is used.

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: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-09  8:07 UTC (permalink / raw)
  To: Pan, Miaoqing, herbert, Matt Mackall
  Cc: miaoqing@codeaurora.org, Valo, Kalle,
	linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <69af156ac93a4fe6ab1844dfcc35c266@aptaiexm02f.ap.qualcomm.com>

Am Dienstag, 9. August 2016, 07:35:33 CEST schrieb Pan, Miaoqing:

Hi Miaoqing, Herbert, Matt,

> Hi Stephan,
> 
> So your suggestion is to use HW Random framework ?   Actually, which was
> done by the commit 6301566e0b2d ("ath9k: export HW random number
> generator"), but it was reverted, you can refer to
> https://www.mail-archive.com/linux-crypto%40vger.kernel.org/msg15483.html
> for more information.

I see, it is the same RNG we talked about earlier. The issue is that the 
suggested rngd per default assumes one bit of entropy with every data bit. 
This is not given with this noise source. This is the basis of my reply last 
time.

Herbert, Matt, should such noise sources be added to the HW random framework? 
The thing is that the in-kernel HW random to input_pool link per default uses 
a more conservative entropy estimate than the user space rngd. I would think 
that the in-kernel link would appropriate for that rng. But the user space 
rngd tool with its default behavior is not really suited here.

Thanks
Stephan

^ permalink raw reply

* RE: [PATCH 2/2] ath9k: disable RNG by default
From: Pan, Miaoqing @ 2016-08-09  7:35 UTC (permalink / raw)
  To: Stephan Mueller, miaoqing@codeaurora.org
  Cc: Valo, Kalle, linux-wireless@vger.kernel.org, ath9k-devel,
	linux-crypto@vger.kernel.org, jason@lakedaemon.net,
	Sepehrdad, Pouyan
In-Reply-To: <4627645.oz3l49ICL3@tauon.atsec.com>

Hi Stephan,

So your suggestion is to use HW Random framework ?   Actually, which was done by the commit 6301566e0b2d ("ath9k: export HW random number generator"), but it was reverted, you can refer to https://www.mail-archive.com/linux-crypto%40vger.kernel.org/msg15483.html for more information.

--
Miaoqing


-----Original Message-----
From: Stephan Mueller [mailto:smueller@chronox.de] 
Sent: Tuesday, August 09, 2016 3:15 PM
To: miaoqing@codeaurora.org
Cc: Valo, Kalle <kvalo@qca.qualcomm.com>; linux-wireless@vger.kernel.org; ath9k-devel <ath9k-devel@qca.qualcomm.com>; linux-crypto@vger.kernel.org; jason@lakedaemon.net; Sepehrdad, Pouyan <pouyans@qti.qualcomm.com>
Subject: Re: [PATCH 2/2] ath9k: disable RNG by default

Am Dienstag, 9. August 2016, 15:02:27 CEST schrieb miaoqing@codeaurora.org:

Hi Miaoqing,

> From: Miaoqing Pan <miaoqing@codeaurora.org>
> 
> ath9k RNG will dominates all the noise sources from the real HW RNG, 
> disable it by default. But we strongly recommand to enable it if the 
> system without HW RNG, especially on embedded systems.
> 
> Signed-off-by: Miaoqing Pan <miaoqing@codeaurora.org>

As a short term solution:

Acked-by: Stephan Mueller <smueller@chronox.de>

But as Jason outlined, there should be nothing that prevents using this code with the HW Random framework. This framework also has logic to limit the rate of injection and allows the setting of the entropy threshold at runtime.

> ---
>  drivers/net/wireless/ath/ath9k/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/Kconfig
> b/drivers/net/wireless/ath/ath9k/Kconfig index f68cb00..8f231c6 100644
> --- a/drivers/net/wireless/ath/ath9k/Kconfig
> +++ b/drivers/net/wireless/ath/ath9k/Kconfig
> @@ -180,7 +180,7 @@ config ATH9K_HTC_DEBUGFS  config ATH9K_HWRNG
>  	bool "Random number generator support"
>  	depends on ATH9K && (HW_RANDOM = y || HW_RANDOM = ATH9K)
> -	default y
> +	default n
>  	---help---
>  	  This option incorporates the ADC register output as a source of
>  	  randomness into Linux entropy pool (/dev/urandom and /dev/random)



Ciao
Stephan

^ permalink raw reply

* [PATCH] crypto: caam - avoid kernel warnings on probe failure
From: Russell King @ 2016-08-09  7:30 UTC (permalink / raw)
  To: Herbert Xu, Fabio Estevam, horia.geanta; +Cc: linux-crypto

While debugging setkey issues, the following warnings were found while
trying to reinsert the caam module.  Fix this by avoiding the duplicated
cleanup in the probe path after caam_remove(), which has already cleaned
up the resources.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 2346 at /home/rmk/git/linux-rmk/mm/vmalloc.c:1490 __vunmap+0xcc/0xf4
Trying to vfree() nonexistent vm area (f2400000)
Modules linked in: caam(+) cbc rfcomm bnep bluetooth nfsd em28xx_rc si2157 si2168 em28xx_dvb uvcvideo snd_soc_imx_sgtl5000 em28xx snd_soc_imx_spdif tveeprom snd_soc_fsl_asoc_card snd_soc_imx_audmux snd_soc_sgtl5000 imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_dma_contig snd_soc_fsl_ssi rc_cec snd_soc_fsl_spdif imx_pcm_dma videobuf2_vmalloc videobuf2_memops imx_thermal dw_hdmi_ahb_audio dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd [last unloaded: caam]
CPU: 0 PID: 2346 Comm: modprobe Tainted: G        W       4.8.0-rc1+ #2014
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[<c0013bb0>] (dump_backtrace) from [<c0013d4c>] (show_stack+0x18/0x1c)
[<c0013d34>] (show_stack) from [<c0357c00>] (dump_stack+0xa4/0xdc)
[<c0357b5c>] (dump_stack) from [<c002e650>] (__warn+0xdc/0x108)
[<c002e574>] (__warn) from [<c002e734>] (warn_slowpath_fmt+0x40/0x48)
[<c002e6f8>] (warn_slowpath_fmt) from [<c0151708>] (__vunmap+0xcc/0xf4)
[<c015163c>] (__vunmap) from [<c015177c>] (vunmap+0x4c/0x54)
[<c0151730>] (vunmap) from [<c001f48c>] (__iounmap+0x2c/0x30)
[<c001f460>] (__iounmap) from [<c001f118>] (iounmap+0x1c/0x20)
[<c001f0fc>] (iounmap) from [<bf247ae4>] (caam_probe+0x3dc/0x1498 [caam])
[<bf247708>] (caam_probe [caam]) from [<c042da8c>] (platform_drv_probe+0x58/0xb8)
[<c042da34>] (platform_drv_probe) from [<c042bb4c>] (driver_probe_device+0x1fc/0x2b8)
[<c042b950>] (driver_probe_device) from [<c042bcc4>] (__driver_attach+0xbc/0xc0) r10:00000000 r8:bf24b000 r7:00000000 r6:ef215844 r5:bf2490c4 r4:ef215810
[<c042bc08>] (__driver_attach) from [<c0429f14>] (bus_for_each_dev+0x5c/0x90)
[<c0429eb8>] (bus_for_each_dev) from [<c042b358>] (driver_attach+0x24/0x28)
[<c042b334>] (driver_attach) from [<c042b058>] (bus_add_driver+0xf4/0x200)
[<c042af64>] (bus_add_driver) from [<c042cadc>] (driver_register+0x80/0xfc)
[<c042ca5c>] (driver_register) from [<c042d960>] (__platform_driver_register+0x48/0x4c)
[<c042d918>] (__platform_driver_register) from [<bf24b018>] (caam_driver_init+0x18/0x24 [caam])
[<bf24b000>] (caam_driver_init [caam]) from [<c00098ac>] (do_one_initcall+0x44/0x178)
[<c0009868>] (do_one_initcall) from [<c010e034>] (do_init_module+0x68/0x1d8)
[<c010dfcc>] (do_init_module) from [<c00c8fbc>] (load_module+0x1974/0x20b0)
[<c00c7648>] (load_module) from [<c00c98d0>] (SyS_finit_module+0x94/0xa0)
[<c00c983c>] (SyS_finit_module) from [<c000fda0>] (ret_fast_syscall+0x0/0x1c)
---[ end trace 34e3370d88bb1786 ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 2346 at /home/rmk/git/linux-rmk/drivers/clk/clk.c:594 clk_core_disable+0xe4/0x26c
Modules linked in: caam(+) cbc rfcomm bnep bluetooth nfsd em28xx_rc si2157 si2168 em28xx_dvb uvcvideo snd_soc_imx_sgtl5000 em28xx snd_soc_imx_spdif tveeprom snd_soc_fsl_asoc_card snd_soc_imx_audmux snd_soc_sgtl5000 imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_dma_contig snd_soc_fsl_ssi rc_cec snd_soc_fsl_spdif imx_pcm_dma videobuf2_vmalloc videobuf2_memops imx_thermal dw_hdmi_ahb_audio dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd [last unloaded: caam]
CPU: 0 PID: 2346 Comm: modprobe Tainted: G        W       4.8.0-rc1+ #2014
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[<c0013bb0>] (dump_backtrace) from [<c0013d4c>] (show_stack+0x18/0x1c)
[<c0013d34>] (show_stack) from [<c0357c00>] (dump_stack+0xa4/0xdc)
[<c0357b5c>] (dump_stack) from [<c002e650>] (__warn+0xdc/0x108)
[<c002e574>] (__warn) from [<c002e6a4>] (warn_slowpath_null+0x28/0x30)
[<c002e67c>] (warn_slowpath_null) from [<c05b113c>] (clk_core_disable+0xe4/0x26c)
[<c05b1058>] (clk_core_disable) from [<c05b2e3c>] (clk_core_disable_lock+0x20/0x2c)
[<c05b2e1c>] (clk_core_disable_lock) from [<c05b2e6c>] (clk_disable+0x24/0x28)
[<c05b2e48>] (clk_disable) from [<bf247b04>] (caam_probe+0x3fc/0x1498 [caam])
[<bf247708>] (caam_probe [caam]) from [<c042da8c>] (platform_drv_probe+0x58/0xb8)
[<c042da34>] (platform_drv_probe) from [<c042bb4c>] (driver_probe_device+0x1fc/0x2b8)
[<c042b950>] (driver_probe_device) from [<c042bcc4>] (__driver_attach+0xbc/0xc0) r10:00000000 r8:bf24b000 r7:00000000 r6:ef215844 r5:bf2490c4 r4:ef215810
[<c042bc08>] (__driver_attach) from [<c0429f14>] (bus_for_each_dev+0x5c/0x90)
[<c0429eb8>] (bus_for_each_dev) from [<c042b358>] (driver_attach+0x24/0x28)
[<c042b334>] (driver_attach) from [<c042b058>] (bus_add_driver+0xf4/0x200)
[<c042af64>] (bus_add_driver) from [<c042cadc>] (driver_register+0x80/0xfc)
[<c042ca5c>] (driver_register) from [<c042d960>] (__platform_driver_register+0x48/0x4c)
[<c042d918>] (__platform_driver_register) from [<bf24b018>] (caam_driver_init+0x18/0x24 [caam])
[<bf24b000>] (caam_driver_init [caam]) from [<c00098ac>] (do_one_initcall+0x44/0x178)
[<c0009868>] (do_one_initcall) from [<c010e034>] (do_init_module+0x68/0x1d8)
[<c010dfcc>] (do_init_module) from [<c00c8fbc>] (load_module+0x1974/0x20b0)
[<c00c7648>] (load_module) from [<c00c98d0>] (SyS_finit_module+0x94/0xa0)
[<c00c983c>] (SyS_finit_module) from [<c000fda0>] (ret_fast_syscall+0x0/0x1c)
---[ end trace 34e3370d88bb1787 ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 2346 at /home/rmk/git/linux-rmk/drivers/clk/clk.c:476 clk_core_unprepare+0x204/0x388
Modules linked in: caam(+) cbc rfcomm bnep bluetooth nfsd em28xx_rc si2157 si2168 em28xx_dvb uvcvideo snd_soc_imx_sgtl5000 em28xx snd_soc_imx_spdif tveeprom snd_soc_fsl_asoc_card snd_soc_imx_audmux snd_soc_sgtl5000 imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_dma_contig snd_soc_fsl_ssi rc_cec snd_soc_fsl_spdif imx_pcm_dma videobuf2_vmalloc videobuf2_memops imx_thermal dw_hdmi_ahb_audio dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd [last unloaded: caam]
CPU: 0 PID: 2346 Comm: modprobe Tainted: G        W       4.8.0-rc1+ #2014
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[<c0013bb0>] (dump_backtrace) from [<c0013d4c>] (show_stack+0x18/0x1c)
[<c0013d34>] (show_stack) from [<c0357c00>] (dump_stack+0xa4/0xdc)
[<c0357b5c>] (dump_stack) from [<c002e650>] (__warn+0xdc/0x108)
[<c002e574>] (__warn) from [<c002e6a4>] (warn_slowpath_null+0x28/0x30)
[<c002e67c>] (warn_slowpath_null) from [<c05b0834>] (clk_core_unprepare+0x204/0x388)
[<c05b0630>] (clk_core_unprepare) from [<c05b4c0c>] (clk_unprepare+0x2c/0x34)
[<c05b4be0>] (clk_unprepare) from [<bf247b0c>] (caam_probe+0x404/0x1498 [caam])
[<bf247708>] (caam_probe [caam]) from [<c042da8c>] (platform_drv_probe+0x58/0xb8)
[<c042da34>] (platform_drv_probe) from [<c042bb4c>] (driver_probe_device+0x1fc/0x2b8)
[<c042b950>] (driver_probe_device) from [<c042bcc4>] (__driver_attach+0xbc/0xc0) r10:00000000 r8:bf24b000 r7:00000000 r6:ef215844 r5:bf2490c4 r4:ef215810
[<c042bc08>] (__driver_attach) from [<c0429f14>] (bus_for_each_dev+0x5c/0x90)
[<c0429eb8>] (bus_for_each_dev) from [<c042b358>] (driver_attach+0x24/0x28)
[<c042b334>] (driver_attach) from [<c042b058>] (bus_add_driver+0xf4/0x200)
[<c042af64>] (bus_add_driver) from [<c042cadc>] (driver_register+0x80/0xfc)
[<c042ca5c>] (driver_register) from [<c042d960>] (__platform_driver_register+0x48/0x4c)
[<c042d918>] (__platform_driver_register) from [<bf24b018>] (caam_driver_init+0x18/0x24 [caam])
[<bf24b000>] (caam_driver_init [caam]) from [<c00098ac>] (do_one_initcall+0x44/0x178)
[<c0009868>] (do_one_initcall) from [<c010e034>] (do_init_module+0x68/0x1d8)
[<c010dfcc>] (do_init_module) from [<c00c8fbc>] (load_module+0x1974/0x20b0)
[<c00c7648>] (load_module) from [<c00c98d0>] (SyS_finit_module+0x94/0xa0)
[<c00c983c>] (SyS_finit_module) from [<c000fda0>] (ret_fast_syscall+0x0/0x1c)
---[ end trace 34e3370d88bb1788 ]---

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/crypto/caam/ctrl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 0ec112ee5204..f4c044f5bcb2 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -826,6 +826,8 @@ static int caam_probe(struct platform_device *pdev)
 
 caam_remove:
 	caam_remove(pdev);
+	return ret;
+
 iounmap_ctrl:
 	iounmap(ctrl);
 disable_caam_emi_slow:
-- 
2.1.0

^ permalink raw reply related

* Re: AF_ALG broken?
From: Russell King - ARM Linux @ 2016-08-09  7:27 UTC (permalink / raw)
  To: Herbert Xu; +Cc: noloader, linux-crypto
In-Reply-To: <20160809071402.GA5466@gondor.apana.org.au>

On Tue, Aug 09, 2016 at 03:14:02PM +0800, Herbert Xu wrote:
> On Tue, Aug 09, 2016 at 08:08:59AM +0100, Russell King - ARM Linux wrote:
> > 
> > I thought I gave the commands and link to your example code.  The
> > openssl case is md5, though sha* also gives the same result.  Your
> > example code was sha1 iirc.  I guess none of these would be using
> > HMAC - the openssl cases used to give results compatible with the
> > md5sum/ sha1sum etc userspace commands.
> > 
> > /proc/crypto:
> > 
> > name         : md5
> > driver       : md5-caam
> 
> Right, caam is providing a setkey function for md5, which leads the
> API to think that a key is required.  We should fix it so that setkey
> is only set for the HMAC-variant.

Thanks, that works nicely again, and passes my tests.

8<====
From: Russell King <rmk+kernel@armlinux.org.uk>
Subject: [PATCH] crypto: caam - fix non-hmac hashes

Since 6de62f15b581 ("crypto: algif_hash - Require setkey before
accept(2)"), the AF_ALG interface requires userspace to provide a key
to any algorithm that has a setkey method.  However, the non-HMAC
algorithms are not keyed, so setting a key is unnecessary.

Fix this by removing the setkey method from the non-keyed hash
algorithms.

Fixes: 6de62f15b581 ("crypto: algif_hash - Require setkey before accept(2)")
Cc: <stable@vger.kernel.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/crypto/caam/caamhash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index ea284e3909ef..9d7fc9ec0b7e 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1950,6 +1950,7 @@ caam_hash_alloc(struct caam_hash_template *template,
 			 template->name);
 		snprintf(alg->cra_driver_name, CRYPTO_MAX_ALG_NAME, "%s",
 			 template->driver_name);
+		t_alg->ahash_alg.setkey = NULL;
 	}
 	alg->cra_module = THIS_MODULE;
 	alg->cra_init = caam_hash_cra_init;
-- 
2.1.0

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply related

* Re: [PATCH 2/2] ath9k: disable RNG by default
From: Stephan Mueller @ 2016-08-09  7:14 UTC (permalink / raw)
  To: miaoqing-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: kvalo-A+ZNKFmMK5xy9aJCnZT0Uw,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	ath9k-devel-A+ZNKFmMK5xy9aJCnZT0Uw,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, jason-NLaQJdtUoK4Be96aLqz0jA,
	pouyans-Rm6X0d1/PG5y9aJCnZT0Uw
In-Reply-To: <1470726147-30095-2-git-send-email-miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Am Dienstag, 9. August 2016, 15:02:27 CEST schrieb miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org:

Hi Miaoqing,

> From: Miaoqing Pan <miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> 
> ath9k RNG will dominates all the noise sources from the real HW
> RNG, disable it by default. But we strongly recommand to enable
> it if the system without HW RNG, especially on embedded systems.
> 
> Signed-off-by: Miaoqing Pan <miaoqing-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

As a short term solution:

Acked-by: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>

But as Jason outlined, there should be nothing that prevents using this code 
with the HW Random framework. This framework also has logic to limit the rate 
of injection and allows the setting of the entropy threshold at runtime.

> ---
>  drivers/net/wireless/ath/ath9k/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/Kconfig
> b/drivers/net/wireless/ath/ath9k/Kconfig index f68cb00..8f231c6 100644
> --- a/drivers/net/wireless/ath/ath9k/Kconfig
> +++ b/drivers/net/wireless/ath/ath9k/Kconfig
> @@ -180,7 +180,7 @@ config ATH9K_HTC_DEBUGFS
>  config ATH9K_HWRNG
>  	bool "Random number generator support"
>  	depends on ATH9K && (HW_RANDOM = y || HW_RANDOM = ATH9K)
> -	default y
> +	default n
>  	---help---
>  	  This option incorporates the ADC register output as a source of
>  	  randomness into Linux entropy pool (/dev/urandom and /dev/random)



Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* Re: AF_ALG broken?
From: Herbert Xu @ 2016-08-09  7:14 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: noloader, linux-crypto
In-Reply-To: <20160809070859.GF1041@n2100.armlinux.org.uk>

On Tue, Aug 09, 2016 at 08:08:59AM +0100, Russell King - ARM Linux wrote:
> 
> I thought I gave the commands and link to your example code.  The
> openssl case is md5, though sha* also gives the same result.  Your
> example code was sha1 iirc.  I guess none of these would be using
> HMAC - the openssl cases used to give results compatible with the
> md5sum/ sha1sum etc userspace commands.
> 
> /proc/crypto:
> 
> name         : md5
> driver       : md5-caam

Right, caam is providing a setkey function for md5, which leads the
API to think that a key is required.  We should fix it so that setkey
is only set for the HMAC-variant.

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


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