* [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
* Re: AF_ALG broken?
From: Russell King - ARM Linux @ 2016-08-09 7:08 UTC (permalink / raw)
To: Herbert Xu; +Cc: noloader, linux-crypto
In-Reply-To: <20160809031820.GA4142@gondor.apana.org.au>
On Tue, Aug 09, 2016 at 11:18:20AM +0800, Herbert Xu wrote:
> Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> > Testing that code on 4.8-rc (and 4.7 fwiw) gives:
> >
> > 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
> > write(4, "abc", 3) = -1 ENOKEY (Required key not available)
> > read(4, 0xbec50508, 20) = -1 ENOKEY (Required key not available)
> >
> > IOW, the same problem - and it seems not to be a recent regression.
> >
> > Since the last time I tested CESA or CAAM was back in 4.4 times,
> > it's got to be something between 4.4 and 4.7.
> >
> > Looking at the history, my guess would be the setkey changes -
> > crypto: algif_skcipher - Require setkey before accept(2)
> > crypto: af_alg - Disallow bind/setkey/... after accept(2)
> > crypto: af_alg - Add nokey compatibility path
> > crypto: hash - Add crypto_ahash_has_setkey
> > crypto: algif_hash - Require setkey before accept(2)
>
> This is definitely supposed to work. Basically if the algorithm
> requires a key (e.g., HMAC) then you must set it. Otherwise it
> should never return ENOKEY.
>
> Which algorithm were you testing and what does /proc/crypto say?
Hi Herbert,
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
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 16
name : hmac(md5)
driver : hmac-md5-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 16
name : sha256
driver : sha256-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 32
name : hmac(sha256)
driver : hmac-sha256-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 32
name : sha224
driver : sha224-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 28
name : hmac(sha224)
driver : hmac-sha224-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 28
name : sha1
driver : sha1-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 20
name : hmac(sha1)
driver : hmac-sha1-caam
module : caamhash
priority : 3000
refcnt : 1
selftest : passed
internal : no
type : ahash
async : yes
blocksize : 64
digestsize : 20
name : ecb(aes)
driver : ecb(aes-asm)
module : kernel
priority : 200
refcnt : 3
selftest : passed
internal : no
type : blkcipher
blocksize : 16
min keysize : 16
max keysize : 32
ivsize : 0
geniv : <default>
name : ghash
driver : ghash-generic
module : kernel
priority : 100
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 16
digestsize : 16
name : jitterentropy_rng
driver : jitterentropy_rng
module : kernel
priority : 100
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_nopr_hmac_sha256
module : kernel
priority : 207
refcnt : 2
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_nopr_hmac_sha512
module : kernel
priority : 206
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_nopr_hmac_sha384
module : kernel
priority : 205
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_nopr_hmac_sha1
module : kernel
priority : 204
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : hmac(sha256)
driver : hmac(sha256-asm)
module : kernel
priority : 150
refcnt : 2
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 32
name : stdrng
driver : drbg_pr_hmac_sha256
module : kernel
priority : 203
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_pr_hmac_sha512
module : kernel
priority : 202
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_pr_hmac_sha384
module : kernel
priority : 201
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : stdrng
driver : drbg_pr_hmac_sha1
module : kernel
priority : 200
refcnt : 1
selftest : passed
internal : no
type : rng
seedsize : 0
name : lzo
driver : lzo-generic
module : kernel
priority : 0
refcnt : 2
selftest : passed
internal : no
type : compression
name : crct10dif
driver : crct10dif-generic
module : kernel
priority : 100
refcnt : 2
selftest : passed
internal : no
type : shash
blocksize : 1
digestsize : 2
name : crc32c
driver : crc32c-generic
module : kernel
priority : 100
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 1
digestsize : 4
name : deflate
driver : deflate-generic
module : kernel
priority : 0
refcnt : 2
selftest : passed
internal : no
type : compression
name : ecb(arc4)
driver : ecb(arc4)-generic
module : kernel
priority : 100
refcnt : 1
selftest : passed
internal : no
type : blkcipher
blocksize : 1
min keysize : 1
max keysize : 256
ivsize : 0
geniv : <default>
name : arc4
driver : arc4-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : cipher
blocksize : 1
min keysize : 1
max keysize : 256
name : aes
driver : aes-generic
module : kernel
priority : 100
refcnt : 1
selftest : passed
internal : no
type : cipher
blocksize : 16
min keysize : 16
max keysize : 32
name : sha224
driver : sha224-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 28
name : sha256
driver : sha256-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 32
name : sha1
driver : sha1-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 20
name : digest_null
driver : digest_null-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 1
digestsize : 0
name : compress_null
driver : compress_null-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : compression
name : ecb(cipher_null)
driver : ecb-cipher_null
module : kernel
priority : 100
refcnt : 1
selftest : passed
internal : no
type : blkcipher
blocksize : 1
min keysize : 0
max keysize : 0
ivsize : 0
geniv : <default>
name : cipher_null
driver : cipher_null-generic
module : kernel
priority : 0
refcnt : 1
selftest : passed
internal : no
type : cipher
blocksize : 1
min keysize : 0
max keysize : 0
name : sha512
driver : sha512-arm
module : kernel
priority : 250
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 128
digestsize : 64
name : sha384
driver : sha384-arm
module : kernel
priority : 250
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 128
digestsize : 48
name : sha224
driver : sha224-asm
module : kernel
priority : 150
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 28
name : sha256
driver : sha256-asm
module : kernel
priority : 150
refcnt : 2
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 32
name : sha1
driver : sha1-asm
module : kernel
priority : 150
refcnt : 1
selftest : passed
internal : no
type : shash
blocksize : 64
digestsize : 20
name : aes
driver : aes-asm
module : kernel
priority : 200
refcnt : 2
selftest : passed
internal : no
type : cipher
blocksize : 16
min keysize : 16
max keysize : 32
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
* [PATCH 2/2] ath9k: disable RNG by default
From: miaoqing @ 2016-08-09 7:02 UTC (permalink / raw)
To: kvalo
Cc: linux-wireless, ath9k-devel, linux-crypto, smueller, jason,
pouyans, Miaoqing Pan
In-Reply-To: <1470726147-30095-1-git-send-email-miaoqing@codeaurora.org>
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>
---
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)
--
1.9.1
^ permalink raw reply related
* [PATCH 1/2] ath9k: change entropy formula for easier understanding
From: miaoqing @ 2016-08-09 7:02 UTC (permalink / raw)
To: kvalo
Cc: linux-wireless, ath9k-devel, linux-crypto, smueller, jason,
pouyans, Miaoqing Pan
From: Miaoqing Pan <miaoqing@codeaurora.org>
The quality of ADC entropy is 10 bits of min-entropy for
a 32-bit value, change '(((x) * 8 * 320) >> 10)' to
'(((x) * 8 * 10) >> 5)' for easier understanding.
Signed-off-by: Miaoqing Pan <miaoqing@codeaurora.org>
---
drivers/net/wireless/ath/ath9k/rng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index d38e50f..568b1c6 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -22,7 +22,7 @@
#include "ar9003_phy.h"
#define ATH9K_RNG_BUF_SIZE 320
-#define ATH9K_RNG_ENTROPY(x) (((x) * 8 * 320) >> 10) /* quality: 320/1024 */
+#define ATH9K_RNG_ENTROPY(x) (((x) * 8 * 10) >> 5) /* quality: 10/32 */
static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size)
{
--
1.9.1
^ permalink raw reply related
* RE: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
From: Pan, Miaoqing @ 2016-08-09 6:30 UTC (permalink / raw)
To: Jason Cooper, Stephan Mueller
Cc: Ted Tso, Sepehrdad, Pouyan, herbert@gondor.apana.org.au,
linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
ath9k-devel, linux-wireless@vger.kernel.org,
ath9k-devel@lists.ath9k.org, Kalle Valo
In-Reply-To: <20160808172930.GD4511@io.lakedaemon.net>
Hi Jason, Stephan,
Agree with Jason's point, also understand Stephan's concern. The date rate can be roughly estimated by 'cat /dev/random |rngtest -c 1000', the average speed is 1111.294Kibits/s. I will sent the patch to disable ath9k RNG by default.
Thanks,
Miaoqing
-----Original Message-----
From: Jason Cooper [mailto:jason@lakedaemon.net]
Sent: Tuesday, August 09, 2016 1:30 AM
To: Stephan Mueller <smueller@chronox.de>
Cc: Pan, Miaoqing <miaoqing@qti.qualcomm.com>; Ted Tso <tytso@mit.edu>; Sepehrdad, Pouyan <pouyans@qti.qualcomm.com>; herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org; linux-crypto@vger.kernel.org; ath9k-devel <ath9k-devel@qca.qualcomm.com>; linux-wireless@vger.kernel.org; ath9k-devel@lists.ath9k.org; Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
Hi Stephan, Miaoqing Pan,
On Mon, Aug 08, 2016 at 08:41:36AM +0200, Stephan Mueller wrote:
> Am Montag, 8. August 2016, 02:03:36 CEST schrieb Pan, Miaoqing:
> > The entropy was evaluated by crypto expert, the analysis report
> > show the ADC with at least 10bits and up to 22 bits of min-entropy
> > for a 32 bits value, we conservatively assume the min-entropy is 10
> > bits out of 32 bits, so that's why set entropy quality to 320/1024 = 10/32.
Ok, so the relevant commit is:
ed14dc0af7cce ath9k: feeding entropy in kernel from ADC capture
Which refers to a previous commit:
6301566e0b2d ath9k: export HW random number generator
> > Also we have explained in the commit message why can't use the HW
> > RNG framework.
>From ed14dc0af7cce:
"""
Since ADC was not designed to be a dedicated HW RNG, we do not want to bind it to /dev/hwrng framework directly.
"""
> Where is the description of the RNG, where is the test implementation?
> >
> > Otherwise, your patch will cause high CPU load, as continuously
> > read ADC data if entropy bits under write_wakeup_threshold.
>
> The issue is that although you may have analyzed it, others are unable
> to measure the quality of the RNG and assess the design as well as the
> implementation of the RNG. This RNG is the only implementation of a
> hardware RNG that per default and without being able to change it at
> runtime injects data into the input_pool where the noise source cannot
> be audited. Note, even other respected RNG noise sources like the
> Intel RDRAND will not feed into / dev/random per default in a way that dominates all other noise sources.
>
> I would like to be able to deactivate that noise source to the extent
> that it does not cause /dev/random to unblock. The reason is that your
> noise source starts to dominate all other noise sources.
I think the short-term problem here is the config logic:
config ATH9K_HWRNG
bool "Random number generator support"
depends on ATH9K && (HW_RANDOM = y || HW_RANDOM = ATH9K)
default y
If you have *any* hwrngs you want to use and you have an ath9k card (HW_RANDOM = y and ATH9K != n), you get the behavior Stephan is pointing out.
Short term, we should just default no here.
> If you think that this patch is a challenge because your driver starts
> to spin, please help and offer another solution.
Well, I don't buy the reasoning listed above for not using the hwrng framework. Interrupt timings were never designed to be a source of entropy either. We need to grab it where ever we can find it, especially on embedded systems. Documentation/hw_random.txt even says:
"""
This data is NOT CHECKED by any fitness tests, and could potentially be bogus (if the hardware is faulty or has been tampered with).
"""
I really don't think there's a problem with adding these sorts of sources under char/hw_random/. I think the only thing we would be concerned about, other than the already addressed entropy estimation, would be constraining the data rate.
Is ath9k the only wireless card that exposes ADC registers? What about sound cards?
thx,
Jason.
^ permalink raw reply
* Re: AF_ALG broken?
From: Herbert Xu @ 2016-08-09 3:18 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: noloader, linux-crypto
In-Reply-To: <20160808181117.GD1041@n2100.armlinux.org.uk>
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> Testing that code on 4.8-rc (and 4.7 fwiw) gives:
>
> 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
> write(4, "abc", 3) = -1 ENOKEY (Required key not available)
> read(4, 0xbec50508, 20) = -1 ENOKEY (Required key not available)
>
> IOW, the same problem - and it seems not to be a recent regression.
>
> Since the last time I tested CESA or CAAM was back in 4.4 times,
> it's got to be something between 4.4 and 4.7.
>
> Looking at the history, my guess would be the setkey changes -
> crypto: algif_skcipher - Require setkey before accept(2)
> crypto: af_alg - Disallow bind/setkey/... after accept(2)
> crypto: af_alg - Add nokey compatibility path
> crypto: hash - Add crypto_ahash_has_setkey
> crypto: algif_hash - Require setkey before accept(2)
This is definitely supposed to work. Basically if the algorithm
requires a key (e.g., HMAC) then you must set it. Otherwise it
should never return ENOKEY.
Which algorithm were you testing and what does /proc/crypto say?
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: AF_ALG broken?
From: David Miller @ 2016-08-08 23:04 UTC (permalink / raw)
To: linux; +Cc: smueller, herbert, linux-crypto
In-Reply-To: <20160808225851.GE1041@n2100.armlinux.org.uk>
From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Mon, 8 Aug 2016 23:58:51 +0100
> I don't know, but this seems to go completely against Linus' no
> userspace regressions, which seems to be an absolute requirement of
> all kernel development... Linus flames people for arguing against
> that rule!
Reading the explanation for the change given to you, it's impossible
for some hash algorithms to function properly without being given the
key first, and would crash.
Therefore the requirement is reasonable, even though it is unfortunate
that a critical piece of userspace infrastructure was coded this way.
^ permalink raw reply
* Re: AF_ALG broken?
From: Russell King - ARM Linux @ 2016-08-08 22:58 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Herbert Xu, David S. Miller, linux-crypto
In-Reply-To: <5217898.dpPNhAO3PW@tauon.atsec.com>
On Mon, Aug 08, 2016 at 08:30:32PM +0200, Stephan Mueller wrote:
> Am Montag, 8. August 2016, 20:18:32 CEST schrieb Stephan Mueller:
>
> Hi Stephan,
>
> > Am Montag, 8. August 2016, 17:44:27 CEST schrieb Russell King - ARM Linux:
> >
> > Hi Russell,
> >
> > > Hi,
> > >
> > > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx
> > > caam, I get this:
> > >
> > > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5
> > > </bin/bash ...
> > > 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_IFREG|0755, st_size=666864, ...}) = 0
> > > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
> > > =
> > > 0xb6fab000 read(0,
> > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"...,
> > > 8192)
> > > = 8192 send(4,
> > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"...,
> > > 8192,
> > > MSG_MORE) = -1 ENOKEY (Required key not available)
> > >
> > > This used to work, so something in the kernel AF_ALG API has changed
> > > which has broken userspace. Any ideas what's up, or where to look?
> >
> > This seems to be the the change added by Herbert to fix a security issue.
> > This caused a similar stirr in the cryptsetup user space tool.
> >
> > I guess you are affected by 6de62f15b581f920ade22d758f4c338311c2f0d4
>
> Just to be clear: the settings on the tfmfd must be completed before an
> accept(). If make an operation on the tfmfd after the accept call, you get
> the ENOKEY.
As you can see from the above strace, there's no operations on fd 3
after the accept call. The only operation on the accepted fd (fd 4)
is the send(). So, I'm not sure I follow what you're saying.
I've also checked - there's no updates for af-alg-rr, so everyone
who's using af-alg-rr must have been broken by this change - if there
_are_ any users of AF_ALG with openssl. Maybe everyone just patches
cryptodev into their kernel?
I don't know, but this seems to go completely against Linus' no
userspace regressions, which seems to be an absolute requirement of
all kernel development... Linus flames people for arguing against
that rule!
--
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
* [PATCH v2] crypto: powerpc - CRYPT_CRC32C_VPMSUM should depend on ALTIVEC
From: Michael Ellerman @ 2016-08-08 22:46 UTC (permalink / raw)
To: linux-crypto; +Cc: linuxppc-dev, Anton Blanchard, Herbert Xu
The optimised crc32c implementation depends on VMX (aka. Altivec)
instructions, so the kernel must be built with Altivec support in order
for the crc32c code to build.
Fixes: 6dd7a82cc54e ("crypto: powerpc - Add POWER8 optimised crc32c")
Acked-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
crypto/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
v2: No change. Send to linux-crypto.
diff --git a/crypto/Kconfig b/crypto/Kconfig
index a9377bef25e3..84d71482bf08 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -439,7 +439,7 @@ config CRYPTO_CRC32C_INTEL
config CRYPT_CRC32C_VPMSUM
tristate "CRC32c CRC algorithm (powerpc64)"
- depends on PPC64
+ depends on PPC64 && ALTIVEC
select CRYPTO_HASH
select CRC32
help
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
From: Jason Cooper @ 2016-08-08 22:04 UTC (permalink / raw)
To: Stephan Mueller
Cc: Pan, Miaoqing, Ted Tso, Sepehrdad, Pouyan,
herbert@gondor.apana.org.au, linux-kernel@vger.kernel.org,
linux-crypto@vger.kernel.org, ath9k-devel,
linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org,
Kalle Valo
In-Reply-To: <20160808172930.GD4511@io.lakedaemon.net>
Hi Stephan,
On Mon, Aug 08, 2016 at 05:29:30PM +0000, Jason Cooper wrote:
> On Mon, Aug 08, 2016 at 08:41:36AM +0200, Stephan Mueller wrote:
...
> > If you think that this patch is a challenge because your driver starts to
> > spin, please help and offer another solution.
>
> Well, I don't buy the reasoning listed above for not using the hwrng
> framework. Interrupt timings were never designed to be a source of entropy
> either. We need to grab it where ever we can find it, especially on
> embedded systems. Documentation/hw_random.txt even says:
>
> """
> This data is NOT CHECKED by any fitness tests, and could potentially be
> bogus (if the hardware is faulty or has been tampered with).
> """
>
> I really don't think there's a problem with adding these sorts of
> sources under char/hw_random/. I think the only thing we would be
> concerned about, other than the already addressed entropy estimation,
> would be constraining the data rate.
Further research yields char/hw_random/timeriomem-rng.c
It could use an update to ->read() vice data_{present,read}(), but it's
functionally exactly what the ath9k rng is doing. :)
thx,
Jason.
^ permalink raw reply
* Re: AF_ALG broken?
From: Stephan Mueller @ 2016-08-08 18:30 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Herbert Xu, David S. Miller, linux-crypto
In-Reply-To: <5469036.jmYhrDdAzD@positron.chronox.de>
Am Montag, 8. August 2016, 20:18:32 CEST schrieb Stephan Mueller:
Hi Stephan,
> Am Montag, 8. August 2016, 17:44:27 CEST schrieb Russell King - ARM Linux:
>
> Hi Russell,
>
> > Hi,
> >
> > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx
> > caam, I get this:
> >
> > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5
> > </bin/bash ...
> > 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_IFREG|0755, st_size=666864, ...}) = 0
> > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
> > =
> > 0xb6fab000 read(0,
> > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"...,
> > 8192)
> > = 8192 send(4,
> > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"...,
> > 8192,
> > MSG_MORE) = -1 ENOKEY (Required key not available)
> >
> > This used to work, so something in the kernel AF_ALG API has changed
> > which has broken userspace. Any ideas what's up, or where to look?
>
> This seems to be the the change added by Herbert to fix a security issue.
> This caused a similar stirr in the cryptsetup user space tool.
>
> I guess you are affected by 6de62f15b581f920ade22d758f4c338311c2f0d4
Just to be clear: the settings on the tfmfd must be completed before an
accept(). If make an operation on the tfmfd after the accept call, you get
the ENOKEY.
This was my change to the issue:
https://github.com/smuellerDD/libkcapi/commit/
1d6c3b1b540caea784a95b1ca6e2cf38c174f585
Ciao
Stephan
^ permalink raw reply
* Re: AF_ALG broken?
From: Stephan Mueller @ 2016-08-08 18:18 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Herbert Xu, David S. Miller, linux-crypto
In-Reply-To: <20160808164427.GB1041@n2100.armlinux.org.uk>
Am Montag, 8. August 2016, 17:44:27 CEST schrieb Russell King - ARM Linux:
Hi Russell,
> Hi,
>
> When trying to use the openssl AF_ALG module with 4.8-rc1 with imx
> caam, I get this:
>
> $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5
> </bin/bash ...
> 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_IFREG|0755, st_size=666864, ...}) = 0
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
> 0xb6fab000 read(0,
> "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192)
> = 8192 send(4,
> "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192,
> MSG_MORE) = -1 ENOKEY (Required key not available)
>
> This used to work, so something in the kernel AF_ALG API has changed
> which has broken userspace. Any ideas what's up, or where to look?
This seems to be the the change added by Herbert to fix a security issue. This
caused a similar stirr in the cryptsetup user space tool.
I guess you are affected by 6de62f15b581f920ade22d758f4c338311c2f0d4
Ciao
Stephan
^ permalink raw reply
* Re: AF_ALG broken?
From: Russell King - ARM Linux @ 2016-08-08 18:11 UTC (permalink / raw)
To: Jeffrey Walton; +Cc: linux-crypto
In-Reply-To: <CAH8yC8nFhMCfXpdrcz1bJDPfbMHRGQpkV6yOZ7gy4yRHJwXkWw@mail.gmail.com>
On Mon, Aug 08, 2016 at 01:47:33PM -0400, Jeffrey Walton wrote:
> > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx
> > caam, I get this:
> >
> > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 </bin/bash
> > ...
> > 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_IFREG|0755, st_size=666864, ...}) = 0
> > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fab000
> > read(0, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192) = 8192
> > send(4, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192, MSG_MORE) = -1 ENOKEY (Required key not available)
>
> As far as I know from testing on x86, it has never worked as expected.
> I believe you have to use 'sendto' and 'recvfrom' because 'send' and
> 'recv' use default structures, and they configure the object
> incorrectly.
This used to work, because that's how I've tested my previous CAAM
and Marvell CESA patches. So, this is not a case of "never worked"
but is definitely a regression caused by some kernel change.
There's also people's presentations that illustrate example code:
https://events.linuxfoundation.org/sites/events/files/slides/lcj-2014-crypto-user.pdf
which can also be found at:
https://lwn.net/Articles/410833/
and that example code comes from Herbert, so one would assume that
it's tested and was working. Note that it doesn't use any send*,
but uses write().
Testing that code on 4.8-rc (and 4.7 fwiw) gives:
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
write(4, "abc", 3) = -1 ENOKEY (Required key not available)
read(4, 0xbec50508, 20) = -1 ENOKEY (Required key not available)
IOW, the same problem - and it seems not to be a recent regression.
Since the last time I tested CESA or CAAM was back in 4.4 times,
it's got to be something between 4.4 and 4.7.
Looking at the history, my guess would be the setkey changes -
crypto: algif_skcipher - Require setkey before accept(2)
crypto: af_alg - Disallow bind/setkey/... after accept(2)
crypto: af_alg - Add nokey compatibility path
crypto: hash - Add crypto_ahash_has_setkey
crypto: algif_hash - Require setkey before accept(2)
--
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox