Re: [PATCH] crypto: cryptd - Remove unused but set variable 'tfm'

2016-10-31 Thread Herbert Xu 
On Mon, Oct 31, 2016 at 03:42:43PM +0100, Tobias Klauser wrote:
> Remove the unused but set variable tfm in cryptd_enqueue_request to fix
> the following warning when building with 'W=1':
> 
> crypto/cryptd.c:125:21: warning: variable 'tfm' set but not used 
> [-Wunused-but-set-variable]
> 
> Signed-off-by: Tobias Klauser 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: skcipher - Get rid of crypto_grab_skcipher2()

2016-10-31 Thread Herbert Xu 
On Fri, Oct 28, 2016 at 09:51:13AM -0700, Eric Biggers wrote:
> Since commit 3a01d0ee2b99 ("crypto: skcipher - Remove top-level
> givcipher interface"), crypto_grab_skcipher2() and
> crypto_grab_skcipher() are equivalent.  So switch callers of
> crypto_grab_skcipher2() to crypto_grab_skcipher() and remove it.
> 
> Signed-off-by: Eric Biggers 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: skcipher - Get rid of crypto_spawn_skcipher2()

2016-10-31 Thread Herbert Xu 
On Fri, Oct 28, 2016 at 09:52:19AM -0700, Eric Biggers wrote:
> Since commit 3a01d0ee2b99 ("crypto: skcipher - Remove top-level
> givcipher interface"), crypto_spawn_skcipher2() and
> crypto_spawn_skcipher() are equivalent.  So switch callers of
> crypto_spawn_skcipher2() to crypto_spawn_skcipher() and remove it.
> 
> Signed-off-by: Eric Biggers 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] char: hw_random: atmel-rng: disable TRNG during suspend

2016-10-31 Thread Herbert Xu 
On Fri, Oct 28, 2016 at 04:00:46PM +0800, Wenyou Yang wrote:
> To fix the over consumption on the VDDCore due to the TRNG enabled,
> disable the TRNG during suspend, not only disable the user interface
> clock (which is controlled by PMC). Because the user interface clock
> is independent from any clock that may be used in the entropy source
> logic circuitry.
> 
> Signed-off-by: Wenyou Yang 
> Acked-by: Nicolas Ferre 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [cryptodev:master 41/47] ERROR: "crypto_acomp_scomp_free_ctx" [crypto/acompress.ko] undefined!

2016-10-31 Thread Herbert Xu 
On Wed, Oct 26, 2016 at 10:56:45AM +0100, Giovanni Cabiddu wrote:
> Hi,
> 
> On Wed, Oct 26, 2016 at 08:47:16AM +0800, kbuild test robot wrote:
> > >> ERROR: "crypto_acomp_scomp_free_ctx" [crypto/acompress.ko] undefined!
> > >> ERROR: "crypto_acomp_scomp_alloc_ctx" [crypto/acompress.ko] undefined!
> > >> ERROR: "crypto_init_scomp_ops_async" [crypto/acompress.ko] undefined!
> 
> Thanks for the report. There is a problem with a dependency between
> acomp and scomp. This patch should fix the issue.

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: caam: fix type mismatch warning

2016-10-31 Thread Herbert Xu 
On Tue, Oct 25, 2016 at 11:29:10PM +0200, Arnd Bergmann wrote:
> Building the caam driver on arm64 produces a harmless warning:
> 
> drivers/crypto/caam/caamalg.c:140:139: warning: comparison of distinct 
> pointer types lacks a cast
> 
> We can use min_t to tell the compiler which type we want it to use
> here.
> 
> Fixes: 5ecf8ef9103c ("crypto: caam - fix sg dump")
> Signed-off-by: Arnd Bergmann 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] crypto: drop pointless static qualifier in atmel_aes_probe()

2016-10-31 Thread Herbert Xu 
On Mon, Oct 24, 2016 at 02:51:22PM +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> There is no need to have the 'struct atmel_aes_dev *aes_dd' variable
> static since new value always be assigned before use it.
> 
> Signed-off-by: Wei Yongjun 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: sahara: fix typo "Decidated" -> "Dedicated"

2016-10-31 Thread Herbert Xu 
On Tue, Oct 25, 2016 at 12:07:27PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to typo in dev_dbg message
> 
> Signed-off-by: Colin Ian King 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hwrng: core - zeroize buffers with random data

2016-10-31 Thread Herbert Xu 
On Sat, Oct 22, 2016 at 03:57:05PM +0200, Stephan Mueller wrote:
> 
> The HWRNG core allocates two buffers during initialization which are
> used to obtain random data. After that data is processed, it is now
> zeroized as it is possible that the HWRNG core will not be asked to
> produce more random data for a long time. This prevents leaving such
> sensitive data in memory.
> 
> Signed-off-by: Stephan Mueller 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] crypto: testmgr - Add missing tests for internal sha*-mb implementations

2016-10-31 Thread Herbert Xu 
On Wed, Oct 26, 2016 at 03:04:42PM -0200, Marcelo Cerri wrote:
> This series adds null tests for all sha*-mb internal algorithms so they can
> be used in FIPS mode without further problems.
> 
> Since they are 3 separated modules I decided to use a separated commit for
> each one.
> 
> Marcelo Cerri (3):
>   crypto: testmgr - Add missing tests for internal sha1-mb
> implementation
>   crypto: testmgr - Add missing tests for internal sha256-mb
> implementation
>   crypto: testmgr - Add missing tests for internal sha512-mb
> implementation
> 
>  crypto/testmgr.c | 24 
>  1 file changed, 24 insertions(+)

I have a patch pending that skips testing of all internal algorithms.

Thanks,
-- 
Email: Herbert Xu 
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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: fix AEAD tag memory handling

2016-10-31 Thread Mat Martineau 


Hi Stephan -

On Mon, 31 Oct 2016, Stephan Mueller wrote:


Am Freitag, 28. Oktober 2016, 15:21:19 CET schrieb Mat Martineau:

Hi Mat,


Please check the current patch (the one I sent to you as a pre-release did
not contain the fix for the decryption part -- the patch sent to the list
and what we discuss here contains the appropriate handling for the
decryption side).

With your example using 16 bytes AAD, 16 bytes CT and 16 bytes Tag, the
read will *only* show the return code of 16 (bytes) now, because only the
CT part is converted into PT.

Yet, you are completely correct that the first 16 bytes of the AAD are
skipped by the read call.

Thus, the read call returns exactly the amount of changed bytes, but it
deviates from the POSIX logic by seeking to the end of the AAD buffer to
find the offset it shall place the resulting data to.


I re-built my kernel using the patch from this email thread, and I still
see the total read() length including the "skipped" AAD byte count. Here's
an strace excerpt for a decrypt operation with AAD length of 32 and
plaintext length of 32:


That is absolutely correct as the patch' intention is to avoid the
superflowous Tag memory consideration for input data during encryption and for
output data for decryption. This prevents the kernel from reading/writing
memory that it does not need to touch.

The patch is not intended for coverting the AAD you reported. Though, I am not
yet sure about whether we need to cover that aspect. My interpretation is that
the kernel is responsible for the entire memory of AAD || PT/CT and
potentially the tag. If the kernel sees that it does not need to change the
AAD, it will not do that and simply change the parts of the buffer it needs
to. Then, the kernel reports the changed bytes.

Note, if we change the kernel to operate as you suggest, there is more memory
re-calculation operation in the kernel as well as in user space. To me, that
is an invtation to errors.


To be clear: my preference is to have read() copy only PT or CT||Tag bytes 
in to the provided buffer. sendmsg() is for input, read() is for output. 
AAD is input and was passed to the kernel in the sendmsg() call.


My second choice is to write the AAD bytes to the read() buffer in order 
to work better with existing userspace code.



I don't see that there is extra userspace manipulation of memory if the 
read() buffer does not include space for AAD. The userspace program just 
passes a pointer to the location for PT or CT||Tag as the buffer pointer 
for read() - the bytes for AAD may already be in the memory preceding that 
pointer.



Besides, I see that only as an interpretation of
how POSIX is to be applied.


We seem to be at an impasse here: I contend that this aspect of POSIX 
read() is not open to interpretation. When read() returns a positive 
number N, that means the *first* N bytes of the buffer were updated. 
Making the programmer second guess which N bytes of the buffer were 
changed depending on which filesystem / socket type / etc is in use for 
that file descriptor is a recipe for serious bugs and seriously breaks 
the "everything is a file" abstraction.



However, I can make another proposal: we change the read/recv handler such
that it returns the actually processed memory, regardless whether it really
touched it. I.e. read will return the following bytes in its return code:

- encryption: AAD + CT + Tag

- decryption: AAD + PT

Even though read would report that amount of memory, we all know that the AAD
part will not be actually written. Yet, the kernel returns the amount of bytes
it processed.


This is what I was attempting to document in my previous reply: strace is 
showing that this is already the behavior implemented by the current 
patch.



Regardless, all of this discussion revolves around a topic that is separate to
this patch. I.e. this patch was not intended to handle the AAD. This patch is
only provided to handle the tag.


My main concern is getting the semantics correct and consistent in a 
single patch series. It would be a big problem to explain that AF_ALG AEAD 
read and write works one way in 4.x, another way in 4.y, and some 
different way in 4.z.


--
Mat Martineau
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] ARM: wire up HWCAP2 feature bits to the CPU modalias

2016-10-31 Thread Russell King - ARM Linux 
On Sat, Oct 29, 2016 at 11:08:36AM +0100, Ard Biesheuvel wrote:
> On 18 October 2016 at 11:52, Ard Biesheuvel  wrote:
> > Wire up the generic support for exposing CPU feature bits via the
> > modalias in /sys/device/system/cpu. This allows udev to automatically
> > load modules for things like crypto algorithms that are implemented
> > using optional instructions.
> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  arch/arm/Kconfig  |  1 +
> >  arch/arm/include/asm/cpufeature.h | 32 
> >  2 files changed, 33 insertions(+)
> >
> 
> Russell,
> 
> do you have any concerns regarding this patch? If not, I will drop it
> into the patch system.

It's still something I need to look at... I've been offline last week,
and sort-of offline the previous week, so I'm catching up.

-- 
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: cryptd - Remove unused but set variable 'tfm'

2016-10-31 Thread Tobias Klauser 
Remove the unused but set variable tfm in cryptd_enqueue_request to fix
the following warning when building with 'W=1':

crypto/cryptd.c:125:21: warning: variable 'tfm' set but not used 
[-Wunused-but-set-variable]

Signed-off-by: Tobias Klauser 
---
 crypto/cryptd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index 0c654e59f215..3fd2a20a8145 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -122,7 +122,6 @@ static int cryptd_enqueue_request(struct cryptd_queue 
*queue,
 {
int cpu, err;
struct cryptd_cpu_queue *cpu_queue;
-   struct crypto_tfm *tfm;
atomic_t *refcnt;
bool may_backlog;
 
@@ -141,7 +140,6 @@ static int cryptd_enqueue_request(struct cryptd_queue 
*queue,
if (!atomic_read(refcnt))
goto out_put_cpu;
 
-   tfm = request->tfm;
atomic_inc(refcnt);
 
 out_put_cpu:
-- 
2.9.0


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

Re: [PATCH] crypto: fix AEAD tag memory handling

2016-10-31 Thread Stephan Mueller 
Am Freitag, 28. Oktober 2016, 15:21:19 CET schrieb Mat Martineau:

Hi Mat,
> > 
> > Please check the current patch (the one I sent to you as a pre-release did
> > not contain the fix for the decryption part -- the patch sent to the list
> > and what we discuss here contains the appropriate handling for the
> > decryption side).
> > 
> > With your example using 16 bytes AAD, 16 bytes CT and 16 bytes Tag, the
> > read will *only* show the return code of 16 (bytes) now, because only the
> > CT part is converted into PT.
> > 
> > Yet, you are completely correct that the first 16 bytes of the AAD are
> > skipped by the read call.
> > 
> > Thus, the read call returns exactly the amount of changed bytes, but it
> > deviates from the POSIX logic by seeking to the end of the AAD buffer to
> > find the offset it shall place the resulting data to.
> 
> I re-built my kernel using the patch from this email thread, and I still
> see the total read() length including the "skipped" AAD byte count. Here's
> an strace excerpt for a decrypt operation with AAD length of 32 and
> plaintext length of 32:

That is absolutely correct as the patch' intention is to avoid the 
superflowous Tag memory consideration for input data during encryption and for 
output data for decryption. This prevents the kernel from reading/writing 
memory that it does not need to touch.

The patch is not intended for coverting the AAD you reported. Though, I am not 
yet sure about whether we need to cover that aspect. My interpretation is that 
the kernel is responsible for the entire memory of AAD || PT/CT and 
potentially the tag. If the kernel sees that it does not need to change the 
AAD, it will not do that and simply change the parts of the buffer it needs 
to. Then, the kernel reports the changed bytes.

Note, if we change the kernel to operate as you suggest, there is more memory 
re-calculation operation in the kernel as well as in user space. To me, that 
is an invtation to errors. Besides, I see that only as an interpretation of 
how POSIX is to be applied.

However, I can make another proposal: we change the read/recv handler such 
that it returns the actually processed memory, regardless whether it really 
touched it. I.e. read will return the following bytes in its return code:

- encryption: AAD + CT + Tag

- decryption: AAD + PT

Even though read would report that amount of memory, we all know that the AAD 
part will not be actually written. Yet, the kernel returns the amount of bytes 
it processed.

Regardless, all of this discussion revolves around a topic that is separate to 
this patch. I.e. this patch was not intended to handle the AAD. This patch is 
only provided to handle the tag.

...


Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND][PATCH] crypto: caam: add support for iMX6UL

2016-10-31 Thread Russell King - ARM Linux 
On Mon, Oct 17, 2016 at 01:28:00PM +0200, Marcus Folkesson wrote:
> i.MX6UL does only require three clocks to enable CAAM module.
> 
> Signed-off-by: Marcus Folkesson 
> Acked-by: Rob Herring 
> Reviewed-by: Horia Geantă 
> ---
>  .../devicetree/bindings/crypto/fsl-sec4.txt| 20 +
>  drivers/crypto/caam/ctrl.c | 35 
> --
>  2 files changed, 40 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt 
> b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
> index adeca34..10a425f 100644
> --- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
> +++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
> @@ -123,6 +123,9 @@ PROPERTIES
>  
>  
>  EXAMPLE
> +
> +iMX6QDL/SX requires four clocks
> +
>   crypto@30 {
>   compatible = "fsl,sec-v4.0";
>   fsl,sec-era = <2>;
> @@ -139,6 +142,23 @@ EXAMPLE
>   clock-names = "mem", "aclk", "ipg", "emi_slow";
>   };
>  
> +
> +iMX6UL does only require three clocks
> +
> + crypto: caam@214 {
> + compatible = "fsl,sec-v4.0";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0x214 0x3c000>;
> + ranges = <0 0x214 0x3c000>;
> + interrupts = ;
> +
> + clocks = < IMX6UL_CLK_CAAM_MEM>,
> +  < IMX6UL_CLK_CAAM_ACLK>,
> +  < IMX6UL_CLK_CAAM_IPG>;
> + clock-names = "mem", "aclk", "ipg";
> + };
> +
>  =
>  Job Ring (JR) Node
>  
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index 0ec112e..5abaf37 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -329,8 +329,8 @@ static int caam_remove(struct platform_device *pdev)
>   clk_disable_unprepare(ctrlpriv->caam_ipg);
>   clk_disable_unprepare(ctrlpriv->caam_mem);
>   clk_disable_unprepare(ctrlpriv->caam_aclk);
> - clk_disable_unprepare(ctrlpriv->caam_emi_slow);
> -
> + if (!of_machine_is_compatible("fsl,imx6ul"))
> + clk_disable_unprepare(ctrlpriv->caam_emi_slow);

There is no need to re-lookup the platform here.  Can't you check the
validity of ctrlpriv->caam_emi_slow ?

>   return 0;
>  }
>  
> @@ -481,14 +481,16 @@ static int caam_probe(struct platform_device *pdev)
>   }
>   ctrlpriv->caam_aclk = clk;
>  
> - clk = caam_drv_identify_clk(>dev, "emi_slow");
> - if (IS_ERR(clk)) {
> - ret = PTR_ERR(clk);
> - dev_err(>dev,
> - "can't identify CAAM emi_slow clk: %d\n", ret);
> - return ret;
> + if (!of_machine_is_compatible("fsl,imx6ul")) {
> + clk = caam_drv_identify_clk(>dev, "emi_slow");
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + dev_err(>dev,
> + "can't identify CAAM emi_slow clk: %d\n", ret);
> + return ret;
> + }
> + ctrlpriv->caam_emi_slow = clk;
>   }
> - ctrlpriv->caam_emi_slow = clk;
>  
>   ret = clk_prepare_enable(ctrlpriv->caam_ipg);
>   if (ret < 0) {
> @@ -509,11 +511,13 @@ static int caam_probe(struct platform_device *pdev)
>   goto disable_caam_mem;
>   }
>  
> - ret = clk_prepare_enable(ctrlpriv->caam_emi_slow);
> - if (ret < 0) {
> - dev_err(>dev, "can't enable CAAM emi slow clock: %d\n",
> - ret);
> - goto disable_caam_aclk;
> + if (!of_machine_is_compatible("fsl,imx6ul")) {

Same here.

> + ret = clk_prepare_enable(ctrlpriv->caam_emi_slow);
> + if (ret < 0) {
> + dev_err(>dev, "can't enable CAAM emi slow clock: 
> %d\n",
> + ret);
> + goto disable_caam_aclk;
> + }
>   }
>  
>   /* Get configuration properties from device tree */
> @@ -829,7 +833,8 @@ caam_remove:
>  iounmap_ctrl:
>   iounmap(ctrl);
>  disable_caam_emi_slow:
> - clk_disable_unprepare(ctrlpriv->caam_emi_slow);
> + if (!of_machine_is_compatible("fsl,imx6ul"))
> + clk_disable_unprepare(ctrlpriv->caam_emi_slow);

and here.

>  disable_caam_aclk:
>   clk_disable_unprepare(ctrlpriv->caam_aclk);
>  disable_caam_mem:
> -- 
> 2.8.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html