Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [3.12-rc1] Dependency on module-init-tools >= 3.11 ?
From: Waiman Long @ 2013-09-12 14:26 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Tetsuo Handa, andr345, rusty, linux-kernel, arthur.marsh,
	tim.c.chen, linux-crypto
In-Reply-To: <20130912102953.GA18916@gondor.apana.org.au>

On 09/12/2013 06:29 AM, Herbert Xu wrote:
> On Thu, Sep 12, 2013 at 07:20:23PM +0900, Tetsuo Handa wrote:
>> Herbert Xu wrote:
>>> The trouble is not all distros will include the softdep modules in
>>> the initramfs.  So for now I think we will have to live with a fallback.
>> I see.
>>
>> Herbert Xu wrote:
>>> OK, can you please try this patch on top of the current tree?
>>>
>>> This way at least you'll have a working system until your initramfs
>>> tool is fixed to do the right thing.
>> I tested the patch and confirmed that the boot failure was solved.
>>
>> But arch/x86/crypto/crct10dif-pclmul.ko is not included into initramfs and
>> therefore we cannot benefit from PCLMULQDQ version.
> That is expected and is also the status quo.  So once the initrd
> generation tool is fixed to include softdeps it will work properly.
>
> Thanks!

I would like to report that I also have the same boot problem on a 
RHEL6.4 box with the crypto patch. My workaround is to force kernel 
build to have the crc_t10dif code built-in by changing the config file:

4889c4889
< CONFIG_CRYPTO_CRCT10DIF=m
---
 > CONFIG_CRYPTO_CRCT10DIF=y
5002c5002
< CONFIG_CRC_T10DIF=m
---
 > CONFIG_CRC_T10DIF=y

This solved the boot problem without any additional patch.  Do you think 
you should consider changing the configuration default to "y" instead of 
"m" or doesn't allow the "m" option at all?

Thanks!

^ permalink raw reply

* Re: [3.12-rc1] Dependency on module-init-tools >= 3.11 ?
From: Herbert Xu @ 2013-09-12 10:29 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: andr345, rusty, linux-kernel, arthur.marsh, tim.c.chen,
	linux-crypto
In-Reply-To: <201309121920.AHJ58678.OMVJFFSHOtOFQL@I-love.SAKURA.ne.jp>

On Thu, Sep 12, 2013 at 07:20:23PM +0900, Tetsuo Handa wrote:
> Herbert Xu wrote:
> > The trouble is not all distros will include the softdep modules in
> > the initramfs.  So for now I think we will have to live with a fallback.
> 
> I see.
> 
> Herbert Xu wrote:
> > OK, can you please try this patch on top of the current tree?
> > 
> > This way at least you'll have a working system until your initramfs
> > tool is fixed to do the right thing.
> 
> I tested the patch and confirmed that the boot failure was solved.
> 
> But arch/x86/crypto/crct10dif-pclmul.ko is not included into initramfs and
> therefore we cannot benefit from PCLMULQDQ version.

That is expected and is also the status quo.  So once the initrd
generation tool is fixed to include softdeps it will work properly.

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: [3.12-rc1] Dependency on module-init-tools >= 3.11 ?
From: Tetsuo Handa @ 2013-09-12 10:20 UTC (permalink / raw)
  To: herbert
  Cc: andr345, rusty, linux-kernel, arthur.marsh, tim.c.chen,
	linux-crypto
In-Reply-To: <20130912052806.GA15737@gondor.apana.org.au>

Herbert Xu wrote:
> The trouble is not all distros will include the softdep modules in
> the initramfs.  So for now I think we will have to live with a fallback.

I see.

Herbert Xu wrote:
> OK, can you please try this patch on top of the current tree?
> 
> This way at least you'll have a working system until your initramfs
> tool is fixed to do the right thing.

I tested the patch and confirmed that the boot failure was solved.

But arch/x86/crypto/crct10dif-pclmul.ko is not included into initramfs and
therefore we cannot benefit from PCLMULQDQ version.

---------- before applying patch ----------
kernel/arch/x86/crypto/crct10dif-pclmul.ko: kernel/crypto/crct10dif.ko
kernel/crypto/crct10dif.ko:
kernel/drivers/scsi/lpfc/lpfc.ko: kernel/drivers/scsi/scsi_transport_fc.ko kernel/drivers/scsi/scsi_tgt.ko kernel/lib/crc-t10dif.ko
kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko
kernel/drivers/scsi/scsi_debug.ko: kernel/lib/crc-t10dif.ko
kernel/lib/crc-t10dif.ko:
---------- before applying patch ----------

---------- after applying patch ----------
kernel/arch/x86/crypto/crct10dif-pclmul.ko: kernel/crypto/crct10dif_common.ko
kernel/crypto/crct10dif_common.ko:
kernel/crypto/crct10dif_generic.ko: kernel/crypto/crct10dif_common.ko
kernel/drivers/scsi/lpfc/lpfc.ko: kernel/drivers/scsi/scsi_transport_fc.ko kernel/drivers/scsi/scsi_tgt.ko kernel/lib/crc-t10dif.ko kernel/crypto/crct10dif_common.ko
kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko kernel/crypto/crct10dif_common.ko
kernel/drivers/scsi/scsi_debug.ko: kernel/lib/crc-t10dif.ko kernel/crypto/crct10dif_common.ko
kernel/lib/crc-t10dif.ko: kernel/crypto/crct10dif_common.ko
---------- after applying patch ----------

^ permalink raw reply

* Re: [3.12-rc1] Dependency on module-init-tools >= 3.11 ?
From: Herbert Xu @ 2013-09-12  5:28 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: andr345, rusty, linux-kernel, arthur.marsh, tim.c.chen,
	Linux Crypto Mailing List
In-Reply-To: <201309121403.FIH09821.JtLOVOSFOFHFQM@I-love.SAKURA.ne.jp>

On Thu, Sep 12, 2013 at 02:03:41PM +0900, Tetsuo Handa wrote:
> Herbert Xu wrote:
> > This way at least you'll have a working system until your initramfs
> > tool is fixed to do the right thing.
> 
> Thank you. But it is module-init-tools-3.9-21.el6_4 in RHEL 6.4.
> We can't wait until Red Hat backports module-init-tools >= 3.11 to RHEL 6.x.
> 
> Since most people are already using module-init-tools >= 3.11 and
> there is workaround for my case (i.e. choose built-in), just updating
> 
>   module-init-tools      0.9.10                  # depmod -V
> 
> line at "Current Minimal Requirements" in Documentation/Changes will be OK.

The trouble is not all distros will include the softdep modules in
the initramfs.  So for now I think we will have to live with a fallback.

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

* question about rfc404 support.
From: Hsieh, Che-Min @ 2013-09-11 19:16 UTC (permalink / raw)
  To: Herbert Xu, linux-crypto@vger.kernel.org; +Cc: Bronstein, Dan

Herbert:

  Can you confirm the following. Thanks.

  Rfc2404 - The Use of HMAC-SHA-1-96 within ESP and AH
 
 For the support, I can't find any algorithm to be specified in the .craname of ahash_alg for Rfc2404.
 
  From 
http://www.freebsd.org/cgi/man.cgi?query=setkey&sektion=8
 it says the following :

  ALGORITHMS
     The following list shows the supported algorithms.  The protocol and algorithm are almost completely orthogonal.  The following list of authentication algorithms can be used as aalgo in the -A aalgo of the protocol parameter:

	   algorithm	   keylen (bits)       comment
	   
	   hmac-sha1	   160		   ah: rfc2404
 

That leads me to believe, from crypto driver stand point, there is no need to have a new algorithm for hmac-sha-1-96 support.  Instead, the agent SW(such as ipsec) should use  "hmac(sha1)", and do the truncation of digested data from 160 bits to 96 bits.

I run a quick test. The input file to setkey command is defined as such ----

flush;
spdflush;
add 10.2.243.75 10.2.243.29 ah 0x604 -A hmac-sha1 0x8D967D88F6CAA9D714800AB3D48051D63F73A312;
add 10.2.243.29 10.2.243.75 ah 0x605 -A hmac-sha1 0x8D967D88F6CAA9D714800AB3D48051D63F73A314;
spdadd 10.2.243.75 10.2.243.29 any -P in ipsec ah/transport//use;
spdadd 10.2.243.29 10.2.243.75 any -P out ipsec ah/transport//use;


I use tcpdump. Clearly I can see 24 bytes of AH header of the ipsec packets.  The header has 12 bytes of fixed information  - Next (1), Ah len (1) ,Reserved (2), Spi (4)  ,Sequence                      (4),  and Auth result.  (12 bytes)


Can you confirm the above? Thanks.

Regards,
Chemin

^ permalink raw reply

* Re: [PATCH] crypto_memcmp: add constant-time memcmp
From: James Yonan @ 2013-09-11 17:20 UTC (permalink / raw)
  To: Marcelo Cerri; +Cc: Daniel Borkmann, linux-crypto, herbert, Florian Weimer
In-Reply-To: <20130911121956.GA16462@oc8526070481.ibm.com>

On 10/09/2013 12:57, Daniel Borkmann wrote:
> There was a similar patch posted some time ago [1] on lkml, where
> Florian (CC) made a good point in [2] that future compiler optimizations
> could short circuit on this. This issue should probably be addressed in
> such a patch here as well.
>
>  [1] https://lkml.org/lkml/2013/2/10/131
>  [2] https://lkml.org/lkml/2013/2/11/381

On 11/09/2013 06:19, Marcelo Cerri wrote:
> The discussion that Daniel pointed out has another interesting point
> regarding the function name. I don't think it's a good idea to name it
> crypto_memcpy since it doesn't have behavior the same way as strcmp.
>
> Florian suggested in the thread names such crypto_mem_equal, which I
> think fits better here.

Ok, here's another stab at this:

* Changed the name to crypto_mem_not_equal.  The "not_equal" seems to
make more sense because the function returns a nonzero "true" value if
the memory regions are not equal.

* Good point that a smart optimizer might add instructions to
short-circuit the loop if all bits in ret have been set.  One way to
deal with this is to disable optimizations that might increase code
size, since a short-circuit optimization in this case would require
adding instructions.

    #pragma GCC optimize ("Os")

The nice thing about using #pragma is that older versions of gcc that
don't recognize it will simply ignore it, and we can probably presume
that older versions of gcc do not support a short-circuit optimization
if the latest one does not.  I did a quick test using gcc 3.4.6 at -O2,
and did not see any evidence of a short-circuit optimization.

* Improved performance when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is
enabled.  This makes the performance roughly on-par with memcmp.

----------------

#pragma GCC optimize ("Os")

noinline unsigned long crypto_mem_not_equal(const void *a, const void *b, size_t size)
{
	unsigned long ret = 0;

#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
#if BITS_PER_LONG == 64
	while (size >= 8) {
		ret |= *(unsigned long *)a ^ *(unsigned long *)b;
		a += 8;
		b += 8;
		size -= 8;
	}
	if (!size)
		return ret;
#endif /* BITS_PER_LONG == 64 */
	if (sizeof(unsigned int) == 4) {
		while (size >= 4) {
			ret |= *(unsigned int *)a ^ *(unsigned int *)b;
			a += 4;
			b += 4;
			size -= 4;
		}
		if (!size)
			return ret;
	}
#endif /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
	while (size > 0) {
		ret |= *(unsigned char *)a ^ *(unsigned char *)b;
		a += 1;
		b += 1;
		size -= 1;
	}
	return ret;
}

James

^ permalink raw reply

* Re: sd_mod - uknown symbol crc_t10_diff [was Re: unable to finish booting when "crypto: crct10dif - Wrap crc_t10dif function all to use crypto transform framework" applied]
From: Arthur Marsh @ 2013-09-11 14:57 UTC (permalink / raw)
  To: Tim Chen; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <523083F5.8050104@internode.on.net>


Oops, that should be crc_t10_dif with one "f".

Arthur.

^ permalink raw reply

* [PATCH v2] crypto: caam - map src buffer before access
From: Yashpal Dutta @ 2013-09-11 19:24 UTC (permalink / raw)
  To: linux-crypto; +Cc: Yashpal Dutta

KMap the buffers before copying trailing bytes during hmac into a session
temporary buffer. This is required if pinned buffer from user-space is send
during hmac and is safe even if hmac request is generated from within kernel.

Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
---
Patch covers review comments on first version of patch.
./scripts/checkpatch.pl --strict 0001-crypto-caam-map-src-buffer-before-access.patch 
total: 0 errors, 0 warnings, 0 checks, 62 lines checked

 drivers/crypto/caam/sg_sw_sec4.h |   34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h
index e0037c8..6d21a12 100644
--- a/drivers/crypto/caam/sg_sw_sec4.h
+++ b/drivers/crypto/caam/sg_sw_sec4.h
@@ -117,6 +117,21 @@ static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg,
 	return nents;
 }
 
+/* Map SG page in kernel virtual address space and copy */
+static inline void sg_map_copy(u8 *dest, struct scatterlist *sg,
+			       int len, int offset)
+{
+	u8 *mapped_addr;
+
+	/*
+	 * Page here can be user-space pinned using get_user_pages
+	 * Same must be kmapped before use and kunmapped subsequently
+	 */
+	mapped_addr = kmap_atomic(sg_page(sg));
+	memcpy(dest, mapped_addr + offset, len);
+	kunmap_atomic(mapped_addr);
+}
+
 /* Copy from len bytes of sg to dest, starting from beginning */
 static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
 {
@@ -124,15 +139,15 @@ static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
 	int cpy_index = 0, next_cpy_index = current_sg->length;
 
 	while (next_cpy_index < len) {
-		memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
-		       current_sg->length);
+		sg_map_copy(dest + cpy_index, current_sg, current_sg->length,
+			    current_sg->offset);
 		current_sg = scatterwalk_sg_next(current_sg);
 		cpy_index = next_cpy_index;
 		next_cpy_index += current_sg->length;
 	}
 	if (cpy_index < len)
-		memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
-		       len - cpy_index);
+		sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
+			    current_sg->offset);
 }
 
 /* Copy sg data, from to_skip to end, to dest */
@@ -140,7 +155,7 @@ static inline void sg_copy_part(u8 *dest, struct scatterlist *sg,
 				      int to_skip, unsigned int end)
 {
 	struct scatterlist *current_sg = sg;
-	int sg_index, cpy_index;
+	int sg_index, cpy_index, offset;
 
 	sg_index = current_sg->length;
 	while (sg_index <= to_skip) {
@@ -148,9 +163,10 @@ static inline void sg_copy_part(u8 *dest, struct scatterlist *sg,
 		sg_index += current_sg->length;
 	}
 	cpy_index = sg_index - to_skip;
-	memcpy(dest, (u8 *) sg_virt(current_sg) +
-	       current_sg->length - cpy_index, cpy_index);
-	current_sg = scatterwalk_sg_next(current_sg);
-	if (end - sg_index)
+	offset = current_sg->offset + current_sg->length - cpy_index;
+	sg_map_copy(dest, current_sg, cpy_index, offset);
+	if (end - sg_index) {
+		current_sg = scatterwalk_sg_next(current_sg);
 		sg_copy(dest + cpy_index, current_sg, end - sg_index);
+	}
 }
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] drivers/crypto:caam:Map src buffer before access in CAAM driver
From: Horia Geantă @ 2013-09-11 12:54 UTC (permalink / raw)
  To: Yashpal.Dutta; +Cc: linux-crypto
In-Reply-To: <1378879346-24500-1-git-send-email-Yashpal.Dutta@freescale.com>

On 9/11/2013 9:02 AM, Yashpal.Dutta@freescale.com wrote:
> From: Yashpal Dutta <yashpal.dutta@freescale.com>
>
> KMap the buffers before copying trailing bytes during hmac in CAAM driver into a
> session temporary buffer. This is required if pinned buffer from user-space
> is send to CAAM driver during hmac and is safe even if hmac request is generated
> from within kernel.
>
> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
> ---

Subject not consistent with previous caam driver commits. Prefix should 
be "crypto: caam - ".
I would drop the "in CAAM driver" since it's redundant - the prefix 
already mentions this.

> @@ -124,15 +139,15 @@ static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
>   	int cpy_index = 0, next_cpy_index = current_sg->length;
>   
>   	while (next_cpy_index < len) {
> -		memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
> -		       current_sg->length);
> +		sg_map_copy(dest + cpy_index, current_sg, current_sg->length,
> +				current_sg->offset);

CHECK: Alignment should match open parenthesis
#49: FILE: drivers/crypto/caam/sg_sw_sec4.h:143:
+               sg_map_copy(dest + cpy_index, current_sg, 
current_sg->length,
+                               current_sg->offset);

>   		current_sg = scatterwalk_sg_next(current_sg);
>   		cpy_index = next_cpy_index;
>   		next_cpy_index += current_sg->length;
>   	}
>   	if (cpy_index < len)
> -		memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
> -		       len - cpy_index);
> +		sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
> +				current_sg->offset);

CHECK: Alignment should match open parenthesis
#58: FILE: drivers/crypto/caam/sg_sw_sec4.h:150:
+               sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
+                               current_sg->offset);

^ permalink raw reply

* Re: [PATCH] crypto_memcmp: add constant-time memcmp
From: Marcelo Cerri @ 2013-09-11 12:19 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: James Yonan, linux-crypto, herbert, Florian Weimer
In-Reply-To: <522F6BB3.2050308@redhat.com>

The discussion that Daniel pointed out has another interesting point
regarding the function name. I don't think it's a good idea to name it
crypto_memcpy since it doesn't have behavior the same way as strcmp.

Florian suggested in the thread names such crypto_mem_equal, which I
think fits better here.

Regards,
Marcelo

On Tue, Sep 10, 2013 at 08:57:55PM +0200, Daniel Borkmann wrote:
> On 09/10/2013 08:38 PM, James Yonan wrote:
> >When comparing MAC hashes, AEAD authentication tags, or other hash
> >values in the context of authentication or integrity checking, it
> >is important not to leak timing information to a potential attacker.
> >
> >Bytewise memory comparisons (such as memcmp) are usually optimized so
> >that they return a nonzero value as soon as a mismatch is found.
> >This early-return behavior can leak timing information, allowing an
> >attacker to iteratively guess the correct result.
> >
> >This patch adds a new method crypto_memcmp that has the same prototype
> >as standard memcmp, but that compares strings of the same length in
> >roughly constant time (cache misses could change the timing, but
> >since they don't reveal information about the content of the strings
> >being compared, they are effectively benign).  Note that crypto_memcmp
> >(unlike memcmp) can only be used to test for equality or inequality,
> >NOT greater-than or less-than.  This is not an issue for its use-cases
> >within the Crypto API.
> >
> >I tried to locate all of the places in the Crypto API where memcmp was
> >being used for authentication or integrity checking, and convert them
> >over to crypto_memcmp.
> >
> >crypto_memcmp is declared noinline and placed in its own source file
> >because a very smart compiler (or LTO) might notice that the return
> >value is always compared against zero/nonzero, and might then
> >reintroduce the same early-return optimization that we are trying to
> >avoid.
> 
> There was a similar patch posted some time ago [1] on lkml, where
> Florian (CC) made a good point in [2] that future compiler optimizations
> could short circuit on this. This issue should probably be addressed in
> such a patch here as well.
> 
>  [1] https://lkml.org/lkml/2013/2/10/131
>  [2] https://lkml.org/lkml/2013/2/11/381
> 
> >Signed-off-by: James Yonan <james@openvpn.net>
> >---
> >  crypto/Makefile                         |  2 +-
> >  crypto/asymmetric_keys/rsa.c            |  5 +++--
> >  crypto/authenc.c                        |  7 ++++---
> >  crypto/authencesn.c                     |  9 +++++----
> >  crypto/ccm.c                            |  5 +++--
> >  crypto/crypto_memcmp.c                  | 31 +++++++++++++++++++++++++++++++
> >  crypto/gcm.c                            |  3 ++-
> >  include/crypto/internal/crypto_memcmp.h | 20 ++++++++++++++++++++
> >  8 files changed, 69 insertions(+), 13 deletions(-)
> >  create mode 100644 crypto/crypto_memcmp.c
> >  create mode 100644 include/crypto/internal/crypto_memcmp.h
> >
> >diff --git a/crypto/Makefile b/crypto/Makefile
> >index 2ba0df2..39a574d 100644
> >--- a/crypto/Makefile
> >+++ b/crypto/Makefile
> >@@ -3,7 +3,7 @@
> >  #
> >
> >  obj-$(CONFIG_CRYPTO) += crypto.o
> >-crypto-y := api.o cipher.o compress.o
> >+crypto-y := api.o cipher.o compress.o crypto_memcmp.o
> >
> >  obj-$(CONFIG_CRYPTO_WORKQUEUE) += crypto_wq.o
> >
> >diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> >index 4a6a069..4f9a250 100644
> >--- a/crypto/asymmetric_keys/rsa.c
> >+++ b/crypto/asymmetric_keys/rsa.c
> >@@ -13,6 +13,7 @@
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> >+#include <crypto/internal/crypto_memcmp.h>
> >  #include "public_key.h"
> >
> >  MODULE_LICENSE("GPL");
> >@@ -189,12 +190,12 @@ static int RSA_verify(const u8 *H, const u8 *EM, size_t k, size_t hash_size,
> >  		}
> >  	}
> >
> >-	if (memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
> >+	if (crypto_memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
> >  		kleave(" = -EBADMSG [EM[T] ASN.1 mismatch]");
> >  		return -EBADMSG;
> >  	}
> >
> >-	if (memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
> >+	if (crypto_memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
> >  		kleave(" = -EKEYREJECTED [EM[T] hash mismatch]");
> >  		return -EKEYREJECTED;
> >  	}
> >diff --git a/crypto/authenc.c b/crypto/authenc.c
> >index ffce19d..82ca98f 100644
> >--- a/crypto/authenc.c
> >+++ b/crypto/authenc.c
> >@@ -13,6 +13,7 @@
> >  #include <crypto/aead.h>
> >  #include <crypto/internal/hash.h>
> >  #include <crypto/internal/skcipher.h>
> >+#include <crypto/internal/crypto_memcmp.h>
> >  #include <crypto/authenc.h>
> >  #include <crypto/scatterwalk.h>
> >  #include <linux/err.h>
> >@@ -188,7 +189,7 @@ static void authenc_verify_ahash_update_done(struct crypto_async_request *areq,
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >
> >-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >  	if (err)
> >  		goto out;
> >
> >@@ -227,7 +228,7 @@ static void authenc_verify_ahash_done(struct crypto_async_request *areq,
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >
> >-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >  	if (err)
> >  		goto out;
> >
> >@@ -462,7 +463,7 @@ static int crypto_authenc_verify(struct aead_request *req,
> >  	ihash = ohash + authsize;
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >-	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> >+	return crypto_memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> >  }
> >
> >  static int crypto_authenc_iverify(struct aead_request *req, u8 *iv,
> >diff --git a/crypto/authencesn.c b/crypto/authencesn.c
> >index ab53762..ec3bef9 100644
> >--- a/crypto/authencesn.c
> >+++ b/crypto/authencesn.c
> >@@ -15,6 +15,7 @@
> >  #include <crypto/aead.h>
> >  #include <crypto/internal/hash.h>
> >  #include <crypto/internal/skcipher.h>
> >+#include <crypto/internal/crypto_memcmp.h>
> >  #include <crypto/authenc.h>
> >  #include <crypto/scatterwalk.h>
> >  #include <linux/err.h>
> >@@ -247,7 +248,7 @@ static void authenc_esn_verify_ahash_update_done(struct crypto_async_request *ar
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >
> >-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >  	if (err)
> >  		goto out;
> >
> >@@ -296,7 +297,7 @@ static void authenc_esn_verify_ahash_update_done2(struct crypto_async_request *a
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >
> >-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >  	if (err)
> >  		goto out;
> >
> >@@ -336,7 +337,7 @@ static void authenc_esn_verify_ahash_done(struct crypto_async_request *areq,
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >
> >-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> >  	if (err)
> >  		goto out;
> >
> >@@ -568,7 +569,7 @@ static int crypto_authenc_esn_verify(struct aead_request *req)
> >  	ihash = ohash + authsize;
> >  	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
> >  				 authsize, 0);
> >-	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> >+	return crypto_memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> >  }
> >
> >  static int crypto_authenc_esn_iverify(struct aead_request *req, u8 *iv,
> >diff --git a/crypto/ccm.c b/crypto/ccm.c
> >index 499c917..57349d3 100644
> >--- a/crypto/ccm.c
> >+++ b/crypto/ccm.c
> >@@ -12,6 +12,7 @@
> >
> >  #include <crypto/internal/aead.h>
> >  #include <crypto/internal/skcipher.h>
> >+#include <crypto/internal/crypto_memcmp.h>
> >  #include <crypto/scatterwalk.h>
> >  #include <linux/err.h>
> >  #include <linux/init.h>
> >@@ -363,7 +364,7 @@ static void crypto_ccm_decrypt_done(struct crypto_async_request *areq,
> >
> >  	if (!err) {
> >  		err = crypto_ccm_auth(req, req->dst, cryptlen);
> >-		if (!err && memcmp(pctx->auth_tag, pctx->odata, authsize))
> >+		if (!err && crypto_memcmp(pctx->auth_tag, pctx->odata, authsize))
> >  			err = -EBADMSG;
> >  	}
> >  	aead_request_complete(req, err);
> >@@ -422,7 +423,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
> >  		return err;
> >
> >  	/* verify */
> >-	if (memcmp(authtag, odata, authsize))
> >+	if (crypto_memcmp(authtag, odata, authsize))
> >  		return -EBADMSG;
> >
> >  	return err;
> >diff --git a/crypto/crypto_memcmp.c b/crypto/crypto_memcmp.c
> >new file mode 100644
> >index 0000000..ef4101c
> >--- /dev/null
> >+++ b/crypto/crypto_memcmp.c
> >@@ -0,0 +1,31 @@
> >+/*
> >+ * Constant-time memcmp.
> >+ *
> >+ * Copyright (C) 2013 OpenVPN Technologies Inc.
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify it
> >+ * under the terms of the GNU General Public License as published by the Free
> >+ * Software Foundation; either version 2 of the License, or (at your option)
> >+ * any later version.
> >+ */
> >+
> >+#include <linux/export.h>
> >+#include <crypto/internal/crypto_memcmp.h>
> >+
> >+/**
> >+ * Like memcmp(), but constant-time to prevent leakage of timing info.
> >+ * Returns 0 when data is equal, non-zero otherwise.
> >+ */
> >+noinline int crypto_memcmp(const void *a, const void *b, size_t size)
> >+{
> >+	const u8 *a1 = a;
> >+	const u8 *b1 = b;
> >+	int ret = 0;
> >+	size_t i;
> >+
> >+	for (i = 0; i < size; i++) {
> >+		ret |= *a1++ ^ *b1++;
> >+	}
> >+	return ret;
> >+}
> >+EXPORT_SYMBOL_GPL(crypto_memcmp);
> >diff --git a/crypto/gcm.c b/crypto/gcm.c
> >index 43e1fb0..6be5bca 100644
> >--- a/crypto/gcm.c
> >+++ b/crypto/gcm.c
> >@@ -12,6 +12,7 @@
> >  #include <crypto/internal/aead.h>
> >  #include <crypto/internal/skcipher.h>
> >  #include <crypto/internal/hash.h>
> >+#include <crypto/internal/crypto_memcmp.h>
> >  #include <crypto/scatterwalk.h>
> >  #include <crypto/hash.h>
> >  #include "internal.h"
> >@@ -582,7 +583,7 @@ static int crypto_gcm_verify(struct aead_request *req,
> >
> >  	crypto_xor(auth_tag, iauth_tag, 16);
> >  	scatterwalk_map_and_copy(iauth_tag, req->src, cryptlen, authsize, 0);
> >-	return memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
> >+	return crypto_memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
> >  }
> >
> >  static void gcm_decrypt_done(struct crypto_async_request *areq, int err)
> >diff --git a/include/crypto/internal/crypto_memcmp.h b/include/crypto/internal/crypto_memcmp.h
> >new file mode 100644
> >index 0000000..db23cc0
> >--- /dev/null
> >+++ b/include/crypto/internal/crypto_memcmp.h
> >@@ -0,0 +1,20 @@
> >+/*
> >+ * Constant-time memcmp.
> >+ *
> >+ * Copyright (C) 2013 OpenVPN Technologies Inc.
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify it
> >+ * under the terms of the GNU General Public License as published by the Free
> >+ * Software Foundation; either version 2 of the License, or (at your option)
> >+ * any later version.
> >+ */
> >+
> >+#ifndef _CRYPTO_INTERNAL_CRYPTO_MEMCMP_H
> >+#define _CRYPTO_INTERNAL_CRYPTO_MEMCMP_H
> >+
> >+#include <linux/types.h>
> >+#include <linux/compiler.h>
> >+
> >+noinline int crypto_memcmp(const void *a, const void *b, size_t size);
> >+
> >+#endif
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [PATCH] drivers/crypto:caam:Map src buffer before access in CAAM driver
From: Yashpal.Dutta @ 2013-09-11  6:02 UTC (permalink / raw)
  To: linux-crypto; +Cc: Yashpal Dutta

From: Yashpal Dutta <yashpal.dutta@freescale.com>

KMap the buffers before copying trailing bytes during hmac in CAAM driver into a
session temporary buffer. This is required if pinned buffer from user-space
is send to CAAM driver during hmac and is safe even if hmac request is generated
from within kernel.

Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
---
 drivers/crypto/caam/sg_sw_sec4.h |   34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h
index e0037c8..9575985 100644
--- a/drivers/crypto/caam/sg_sw_sec4.h
+++ b/drivers/crypto/caam/sg_sw_sec4.h
@@ -117,6 +117,21 @@ static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg,
 	return nents;
 }
 
+/* Map SG page in kernel virtual address space and copy */
+static inline void sg_map_copy(u8 *dest, struct scatterlist *sg,
+					int len, int offset)
+{
+	u8 *mapped_addr;
+
+	/*
+	 * Page here can be user-space pinned using get_user_pages
+	 * Same must be kmapped before use and kunmapped subsequently
+	 */
+	mapped_addr = kmap_atomic(sg_page(sg));
+	memcpy(dest, mapped_addr + offset, len);
+	kunmap_atomic(mapped_addr);
+}
+
 /* Copy from len bytes of sg to dest, starting from beginning */
 static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
 {
@@ -124,15 +139,15 @@ static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
 	int cpy_index = 0, next_cpy_index = current_sg->length;
 
 	while (next_cpy_index < len) {
-		memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
-		       current_sg->length);
+		sg_map_copy(dest + cpy_index, current_sg, current_sg->length,
+				current_sg->offset);
 		current_sg = scatterwalk_sg_next(current_sg);
 		cpy_index = next_cpy_index;
 		next_cpy_index += current_sg->length;
 	}
 	if (cpy_index < len)
-		memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
-		       len - cpy_index);
+		sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
+				current_sg->offset);
 }
 
 /* Copy sg data, from to_skip to end, to dest */
@@ -140,7 +155,7 @@ static inline void sg_copy_part(u8 *dest, struct scatterlist *sg,
 				      int to_skip, unsigned int end)
 {
 	struct scatterlist *current_sg = sg;
-	int sg_index, cpy_index;
+	int sg_index, cpy_index, offset;
 
 	sg_index = current_sg->length;
 	while (sg_index <= to_skip) {
@@ -148,9 +163,10 @@ static inline void sg_copy_part(u8 *dest, struct scatterlist *sg,
 		sg_index += current_sg->length;
 	}
 	cpy_index = sg_index - to_skip;
-	memcpy(dest, (u8 *) sg_virt(current_sg) +
-	       current_sg->length - cpy_index, cpy_index);
-	current_sg = scatterwalk_sg_next(current_sg);
-	if (end - sg_index)
+	offset = current_sg->offset + current_sg->length - cpy_index;
+	sg_map_copy(dest, current_sg, cpy_index, offset);
+	if (end - sg_index) {
+		current_sg = scatterwalk_sg_next(current_sg);
 		sg_copy(dest + cpy_index, current_sg, end - sg_index);
+	}
 }
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] modules: add support for soft module dependencies
From: Herbert Xu @ 2013-09-11  4:40 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Rusty Russell, Andreas Robinson, Linux Kernel Mailing List,
	Zhao Hongjiang, David Miller, tim.c.chen, Andrew Morton,
	gregkh@linuxfoundation.org, linux-crypto, linux-modules,
	Tom Gundersen
In-Reply-To: <CAKi4VA+=1vOOt0BqdQHQaeF1hP3DrRtCL_ObXY0cPeO7S_MtYw@mail.gmail.com>

On Tue, Sep 10, 2013 at 11:01:38AM -0300, Lucas De Marchi wrote:
>
> However looking at the only user right now, crct10dif: couldn't we
> detect at runtime if this module can be used instead of just trying to
> load it as a pre softdep and possibly failing?

No as sd_mod requires crct10dif to be present.  The use of softdep
as opposed to just a symbolic dependency is due to there being two
modules for crct10dif, a generic version and an arch-specific one.

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: race condition in crypto larval handling
From: Herbert Xu @ 2013-09-10 23:13 UTC (permalink / raw)
  To: James Yonan; +Cc: linux-crypto
In-Reply-To: <522D2743.9060802@openvpn.net>

Please don't trim cc lists!

James Yonan <james@openvpn.net> wrote:
> 
> I tried this patch, but I still see an apparent module lookup/load race 
> if code on several CPUs calls crypto_alloc_aead at the same time, and an 
> external module such as aes needs to be loaded.
> 
> Seeing this in the log: "request_module: runaway loop modprobe gcm(aes)"
> 
> Shouldn't module lookup/load be bracketed by some sort of lock to 
> prevent this?

This sounds like a different problem.  Larvals are only used to
control the instantiation of templates.  The loading of modules
occur prior to that so you can certainly have multiple loads occur
if multiple threads are trying to allocate the same algorithm.

This should be harmless since gcm(aes) doesn't exist anyway so
regardless of whether we load the module the lookup will fail and
we will go through the larval code path.

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] crypto_memcmp: add constant-time memcmp
From: Daniel Borkmann @ 2013-09-10 18:57 UTC (permalink / raw)
  To: James Yonan; +Cc: linux-crypto, herbert, Florian Weimer
In-Reply-To: <1378838291-7036-1-git-send-email-james@openvpn.net>

On 09/10/2013 08:38 PM, James Yonan wrote:
> When comparing MAC hashes, AEAD authentication tags, or other hash
> values in the context of authentication or integrity checking, it
> is important not to leak timing information to a potential attacker.
>
> Bytewise memory comparisons (such as memcmp) are usually optimized so
> that they return a nonzero value as soon as a mismatch is found.
> This early-return behavior can leak timing information, allowing an
> attacker to iteratively guess the correct result.
>
> This patch adds a new method crypto_memcmp that has the same prototype
> as standard memcmp, but that compares strings of the same length in
> roughly constant time (cache misses could change the timing, but
> since they don't reveal information about the content of the strings
> being compared, they are effectively benign).  Note that crypto_memcmp
> (unlike memcmp) can only be used to test for equality or inequality,
> NOT greater-than or less-than.  This is not an issue for its use-cases
> within the Crypto API.
>
> I tried to locate all of the places in the Crypto API where memcmp was
> being used for authentication or integrity checking, and convert them
> over to crypto_memcmp.
>
> crypto_memcmp is declared noinline and placed in its own source file
> because a very smart compiler (or LTO) might notice that the return
> value is always compared against zero/nonzero, and might then
> reintroduce the same early-return optimization that we are trying to
> avoid.

There was a similar patch posted some time ago [1] on lkml, where
Florian (CC) made a good point in [2] that future compiler optimizations
could short circuit on this. This issue should probably be addressed in
such a patch here as well.

  [1] https://lkml.org/lkml/2013/2/10/131
  [2] https://lkml.org/lkml/2013/2/11/381

> Signed-off-by: James Yonan <james@openvpn.net>
> ---
>   crypto/Makefile                         |  2 +-
>   crypto/asymmetric_keys/rsa.c            |  5 +++--
>   crypto/authenc.c                        |  7 ++++---
>   crypto/authencesn.c                     |  9 +++++----
>   crypto/ccm.c                            |  5 +++--
>   crypto/crypto_memcmp.c                  | 31 +++++++++++++++++++++++++++++++
>   crypto/gcm.c                            |  3 ++-
>   include/crypto/internal/crypto_memcmp.h | 20 ++++++++++++++++++++
>   8 files changed, 69 insertions(+), 13 deletions(-)
>   create mode 100644 crypto/crypto_memcmp.c
>   create mode 100644 include/crypto/internal/crypto_memcmp.h
>
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 2ba0df2..39a574d 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -3,7 +3,7 @@
>   #
>
>   obj-$(CONFIG_CRYPTO) += crypto.o
> -crypto-y := api.o cipher.o compress.o
> +crypto-y := api.o cipher.o compress.o crypto_memcmp.o
>
>   obj-$(CONFIG_CRYPTO_WORKQUEUE) += crypto_wq.o
>
> diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> index 4a6a069..4f9a250 100644
> --- a/crypto/asymmetric_keys/rsa.c
> +++ b/crypto/asymmetric_keys/rsa.c
> @@ -13,6 +13,7 @@
>   #include <linux/module.h>
>   #include <linux/kernel.h>
>   #include <linux/slab.h>
> +#include <crypto/internal/crypto_memcmp.h>
>   #include "public_key.h"
>
>   MODULE_LICENSE("GPL");
> @@ -189,12 +190,12 @@ static int RSA_verify(const u8 *H, const u8 *EM, size_t k, size_t hash_size,
>   		}
>   	}
>
> -	if (memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
> +	if (crypto_memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
>   		kleave(" = -EBADMSG [EM[T] ASN.1 mismatch]");
>   		return -EBADMSG;
>   	}
>
> -	if (memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
> +	if (crypto_memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
>   		kleave(" = -EKEYREJECTED [EM[T] hash mismatch]");
>   		return -EKEYREJECTED;
>   	}
> diff --git a/crypto/authenc.c b/crypto/authenc.c
> index ffce19d..82ca98f 100644
> --- a/crypto/authenc.c
> +++ b/crypto/authenc.c
> @@ -13,6 +13,7 @@
>   #include <crypto/aead.h>
>   #include <crypto/internal/hash.h>
>   #include <crypto/internal/skcipher.h>
> +#include <crypto/internal/crypto_memcmp.h>
>   #include <crypto/authenc.h>
>   #include <crypto/scatterwalk.h>
>   #include <linux/err.h>
> @@ -188,7 +189,7 @@ static void authenc_verify_ahash_update_done(struct crypto_async_request *areq,
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -227,7 +228,7 @@ static void authenc_verify_ahash_done(struct crypto_async_request *areq,
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -462,7 +463,7 @@ static int crypto_authenc_verify(struct aead_request *req,
>   	ihash = ohash + authsize;
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
> -	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> +	return crypto_memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
>   }
>
>   static int crypto_authenc_iverify(struct aead_request *req, u8 *iv,
> diff --git a/crypto/authencesn.c b/crypto/authencesn.c
> index ab53762..ec3bef9 100644
> --- a/crypto/authencesn.c
> +++ b/crypto/authencesn.c
> @@ -15,6 +15,7 @@
>   #include <crypto/aead.h>
>   #include <crypto/internal/hash.h>
>   #include <crypto/internal/skcipher.h>
> +#include <crypto/internal/crypto_memcmp.h>
>   #include <crypto/authenc.h>
>   #include <crypto/scatterwalk.h>
>   #include <linux/err.h>
> @@ -247,7 +248,7 @@ static void authenc_esn_verify_ahash_update_done(struct crypto_async_request *ar
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -296,7 +297,7 @@ static void authenc_esn_verify_ahash_update_done2(struct crypto_async_request *a
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -336,7 +337,7 @@ static void authenc_esn_verify_ahash_done(struct crypto_async_request *areq,
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -568,7 +569,7 @@ static int crypto_authenc_esn_verify(struct aead_request *req)
>   	ihash = ohash + authsize;
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
> -	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> +	return crypto_memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
>   }
>
>   static int crypto_authenc_esn_iverify(struct aead_request *req, u8 *iv,
> diff --git a/crypto/ccm.c b/crypto/ccm.c
> index 499c917..57349d3 100644
> --- a/crypto/ccm.c
> +++ b/crypto/ccm.c
> @@ -12,6 +12,7 @@
>
>   #include <crypto/internal/aead.h>
>   #include <crypto/internal/skcipher.h>
> +#include <crypto/internal/crypto_memcmp.h>
>   #include <crypto/scatterwalk.h>
>   #include <linux/err.h>
>   #include <linux/init.h>
> @@ -363,7 +364,7 @@ static void crypto_ccm_decrypt_done(struct crypto_async_request *areq,
>
>   	if (!err) {
>   		err = crypto_ccm_auth(req, req->dst, cryptlen);
> -		if (!err && memcmp(pctx->auth_tag, pctx->odata, authsize))
> +		if (!err && crypto_memcmp(pctx->auth_tag, pctx->odata, authsize))
>   			err = -EBADMSG;
>   	}
>   	aead_request_complete(req, err);
> @@ -422,7 +423,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
>   		return err;
>
>   	/* verify */
> -	if (memcmp(authtag, odata, authsize))
> +	if (crypto_memcmp(authtag, odata, authsize))
>   		return -EBADMSG;
>
>   	return err;
> diff --git a/crypto/crypto_memcmp.c b/crypto/crypto_memcmp.c
> new file mode 100644
> index 0000000..ef4101c
> --- /dev/null
> +++ b/crypto/crypto_memcmp.c
> @@ -0,0 +1,31 @@
> +/*
> + * Constant-time memcmp.
> + *
> + * Copyright (C) 2013 OpenVPN Technologies Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/export.h>
> +#include <crypto/internal/crypto_memcmp.h>
> +
> +/**
> + * Like memcmp(), but constant-time to prevent leakage of timing info.
> + * Returns 0 when data is equal, non-zero otherwise.
> + */
> +noinline int crypto_memcmp(const void *a, const void *b, size_t size)
> +{
> +	const u8 *a1 = a;
> +	const u8 *b1 = b;
> +	int ret = 0;
> +	size_t i;
> +
> +	for (i = 0; i < size; i++) {
> +		ret |= *a1++ ^ *b1++;
> +	}
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(crypto_memcmp);
> diff --git a/crypto/gcm.c b/crypto/gcm.c
> index 43e1fb0..6be5bca 100644
> --- a/crypto/gcm.c
> +++ b/crypto/gcm.c
> @@ -12,6 +12,7 @@
>   #include <crypto/internal/aead.h>
>   #include <crypto/internal/skcipher.h>
>   #include <crypto/internal/hash.h>
> +#include <crypto/internal/crypto_memcmp.h>
>   #include <crypto/scatterwalk.h>
>   #include <crypto/hash.h>
>   #include "internal.h"
> @@ -582,7 +583,7 @@ static int crypto_gcm_verify(struct aead_request *req,
>
>   	crypto_xor(auth_tag, iauth_tag, 16);
>   	scatterwalk_map_and_copy(iauth_tag, req->src, cryptlen, authsize, 0);
> -	return memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
> +	return crypto_memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
>   }
>
>   static void gcm_decrypt_done(struct crypto_async_request *areq, int err)
> diff --git a/include/crypto/internal/crypto_memcmp.h b/include/crypto/internal/crypto_memcmp.h
> new file mode 100644
> index 0000000..db23cc0
> --- /dev/null
> +++ b/include/crypto/internal/crypto_memcmp.h
> @@ -0,0 +1,20 @@
> +/*
> + * Constant-time memcmp.
> + *
> + * Copyright (C) 2013 OpenVPN Technologies Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#ifndef _CRYPTO_INTERNAL_CRYPTO_MEMCMP_H
> +#define _CRYPTO_INTERNAL_CRYPTO_MEMCMP_H
> +
> +#include <linux/types.h>
> +#include <linux/compiler.h>
> +
> +noinline int crypto_memcmp(const void *a, const void *b, size_t size);
> +
> +#endif
>

^ permalink raw reply

* [PATCH] crypto_memcmp: add constant-time memcmp
From: James Yonan @ 2013-09-10 18:38 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, James Yonan

When comparing MAC hashes, AEAD authentication tags, or other hash
values in the context of authentication or integrity checking, it
is important not to leak timing information to a potential attacker.

Bytewise memory comparisons (such as memcmp) are usually optimized so
that they return a nonzero value as soon as a mismatch is found.
This early-return behavior can leak timing information, allowing an
attacker to iteratively guess the correct result.

This patch adds a new method crypto_memcmp that has the same prototype
as standard memcmp, but that compares strings of the same length in
roughly constant time (cache misses could change the timing, but
since they don't reveal information about the content of the strings
being compared, they are effectively benign).  Note that crypto_memcmp
(unlike memcmp) can only be used to test for equality or inequality,
NOT greater-than or less-than.  This is not an issue for its use-cases
within the Crypto API.

I tried to locate all of the places in the Crypto API where memcmp was
being used for authentication or integrity checking, and convert them
over to crypto_memcmp.

crypto_memcmp is declared noinline and placed in its own source file
because a very smart compiler (or LTO) might notice that the return
value is always compared against zero/nonzero, and might then
reintroduce the same early-return optimization that we are trying to
avoid.

Signed-off-by: James Yonan <james@openvpn.net>
---
 crypto/Makefile                         |  2 +-
 crypto/asymmetric_keys/rsa.c            |  5 +++--
 crypto/authenc.c                        |  7 ++++---
 crypto/authencesn.c                     |  9 +++++----
 crypto/ccm.c                            |  5 +++--
 crypto/crypto_memcmp.c                  | 31 +++++++++++++++++++++++++++++++
 crypto/gcm.c                            |  3 ++-
 include/crypto/internal/crypto_memcmp.h | 20 ++++++++++++++++++++
 8 files changed, 69 insertions(+), 13 deletions(-)
 create mode 100644 crypto/crypto_memcmp.c
 create mode 100644 include/crypto/internal/crypto_memcmp.h

diff --git a/crypto/Makefile b/crypto/Makefile
index 2ba0df2..39a574d 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -3,7 +3,7 @@
 #
 
 obj-$(CONFIG_CRYPTO) += crypto.o
-crypto-y := api.o cipher.o compress.o
+crypto-y := api.o cipher.o compress.o crypto_memcmp.o
 
 obj-$(CONFIG_CRYPTO_WORKQUEUE) += crypto_wq.o
 
diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 4a6a069..4f9a250 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
+#include <crypto/internal/crypto_memcmp.h>
 #include "public_key.h"
 
 MODULE_LICENSE("GPL");
@@ -189,12 +190,12 @@ static int RSA_verify(const u8 *H, const u8 *EM, size_t k, size_t hash_size,
 		}
 	}
 
-	if (memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
+	if (crypto_memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
 		kleave(" = -EBADMSG [EM[T] ASN.1 mismatch]");
 		return -EBADMSG;
 	}
 
-	if (memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
+	if (crypto_memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
 		kleave(" = -EKEYREJECTED [EM[T] hash mismatch]");
 		return -EKEYREJECTED;
 	}
diff --git a/crypto/authenc.c b/crypto/authenc.c
index ffce19d..82ca98f 100644
--- a/crypto/authenc.c
+++ b/crypto/authenc.c
@@ -13,6 +13,7 @@
 #include <crypto/aead.h>
 #include <crypto/internal/hash.h>
 #include <crypto/internal/skcipher.h>
+#include <crypto/internal/crypto_memcmp.h>
 #include <crypto/authenc.h>
 #include <crypto/scatterwalk.h>
 #include <linux/err.h>
@@ -188,7 +189,7 @@ static void authenc_verify_ahash_update_done(struct crypto_async_request *areq,
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
 
-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
 	if (err)
 		goto out;
 
@@ -227,7 +228,7 @@ static void authenc_verify_ahash_done(struct crypto_async_request *areq,
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
 
-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
 	if (err)
 		goto out;
 
@@ -462,7 +463,7 @@ static int crypto_authenc_verify(struct aead_request *req,
 	ihash = ohash + authsize;
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
-	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
+	return crypto_memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
 }
 
 static int crypto_authenc_iverify(struct aead_request *req, u8 *iv,
diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index ab53762..ec3bef9 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -15,6 +15,7 @@
 #include <crypto/aead.h>
 #include <crypto/internal/hash.h>
 #include <crypto/internal/skcipher.h>
+#include <crypto/internal/crypto_memcmp.h>
 #include <crypto/authenc.h>
 #include <crypto/scatterwalk.h>
 #include <linux/err.h>
@@ -247,7 +248,7 @@ static void authenc_esn_verify_ahash_update_done(struct crypto_async_request *ar
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
 
-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
 	if (err)
 		goto out;
 
@@ -296,7 +297,7 @@ static void authenc_esn_verify_ahash_update_done2(struct crypto_async_request *a
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
 
-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
 	if (err)
 		goto out;
 
@@ -336,7 +337,7 @@ static void authenc_esn_verify_ahash_done(struct crypto_async_request *areq,
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
 
-	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
+	err = crypto_memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
 	if (err)
 		goto out;
 
@@ -568,7 +569,7 @@ static int crypto_authenc_esn_verify(struct aead_request *req)
 	ihash = ohash + authsize;
 	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
 				 authsize, 0);
-	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
+	return crypto_memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
 }
 
 static int crypto_authenc_esn_iverify(struct aead_request *req, u8 *iv,
diff --git a/crypto/ccm.c b/crypto/ccm.c
index 499c917..57349d3 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -12,6 +12,7 @@
 
 #include <crypto/internal/aead.h>
 #include <crypto/internal/skcipher.h>
+#include <crypto/internal/crypto_memcmp.h>
 #include <crypto/scatterwalk.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -363,7 +364,7 @@ static void crypto_ccm_decrypt_done(struct crypto_async_request *areq,
 
 	if (!err) {
 		err = crypto_ccm_auth(req, req->dst, cryptlen);
-		if (!err && memcmp(pctx->auth_tag, pctx->odata, authsize))
+		if (!err && crypto_memcmp(pctx->auth_tag, pctx->odata, authsize))
 			err = -EBADMSG;
 	}
 	aead_request_complete(req, err);
@@ -422,7 +423,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
 		return err;
 
 	/* verify */
-	if (memcmp(authtag, odata, authsize))
+	if (crypto_memcmp(authtag, odata, authsize))
 		return -EBADMSG;
 
 	return err;
diff --git a/crypto/crypto_memcmp.c b/crypto/crypto_memcmp.c
new file mode 100644
index 0000000..ef4101c
--- /dev/null
+++ b/crypto/crypto_memcmp.c
@@ -0,0 +1,31 @@
+/*
+ * Constant-time memcmp.
+ *
+ * Copyright (C) 2013 OpenVPN Technologies Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/export.h>
+#include <crypto/internal/crypto_memcmp.h>
+
+/**
+ * Like memcmp(), but constant-time to prevent leakage of timing info.
+ * Returns 0 when data is equal, non-zero otherwise.
+ */
+noinline int crypto_memcmp(const void *a, const void *b, size_t size)
+{
+	const u8 *a1 = a;
+	const u8 *b1 = b;
+	int ret = 0;
+	size_t i;
+
+	for (i = 0; i < size; i++) {
+		ret |= *a1++ ^ *b1++;
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(crypto_memcmp);
diff --git a/crypto/gcm.c b/crypto/gcm.c
index 43e1fb0..6be5bca 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -12,6 +12,7 @@
 #include <crypto/internal/aead.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/internal/hash.h>
+#include <crypto/internal/crypto_memcmp.h>
 #include <crypto/scatterwalk.h>
 #include <crypto/hash.h>
 #include "internal.h"
@@ -582,7 +583,7 @@ static int crypto_gcm_verify(struct aead_request *req,
 
 	crypto_xor(auth_tag, iauth_tag, 16);
 	scatterwalk_map_and_copy(iauth_tag, req->src, cryptlen, authsize, 0);
-	return memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
+	return crypto_memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
 }
 
 static void gcm_decrypt_done(struct crypto_async_request *areq, int err)
diff --git a/include/crypto/internal/crypto_memcmp.h b/include/crypto/internal/crypto_memcmp.h
new file mode 100644
index 0000000..db23cc0
--- /dev/null
+++ b/include/crypto/internal/crypto_memcmp.h
@@ -0,0 +1,20 @@
+/*
+ * Constant-time memcmp.
+ *
+ * Copyright (C) 2013 OpenVPN Technologies Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#ifndef _CRYPTO_INTERNAL_CRYPTO_MEMCMP_H
+#define _CRYPTO_INTERNAL_CRYPTO_MEMCMP_H
+
+#include <linux/types.h>
+#include <linux/compiler.h>
+
+noinline int crypto_memcmp(const void *a, const void *b, size_t size);
+
+#endif
-- 
1.8.1.2

^ permalink raw reply related

* Re: [PATCH 1/3] crypto: omap-des: Add omap-des driver for OMAP4/AM43xx
From: Joel Fernandes @ 2013-09-10 16:16 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: Tony Lindgren, Mark Greer, Linux Kernel Mailing List, Herbert Xu,
	Linux ARM Kernel List, Linux OMAP List, David S. Miller,
	Lokesh Vutla, Linux Crypto Mailing List
In-Reply-To: <522063AB.2030907@ti.com>

On 08/30/2013 04:19 AM, Rajendra Nayak wrote:
> []..
> 
>> +
>> +#define pr_fmt(fmt) "%s: " fmt, __func__
>> +
>> +#ifdef DEBUG
>> +#define prn(num) printk(#num "=%d\n", num)
>> +#define prx(num) printk(#num "=%x\n", num)
>> +#else
>> +#define prn(num) do { } while (0)
>> +#define prx(num)  do { } while (0)
>> +#endif
>> +
>> +#include <linux/err.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/scatterlist.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/omap-dma.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_address.h>
>> +#include <linux/io.h>
>> +#include <linux/crypto.h>
>> +#include <linux/interrupt.h>
>> +#include <crypto/scatterwalk.h>
>> +#include <crypto/des.h>
>> +
>> +#define DST_MAXBURST			2
>> +
>> +#define DES_BLOCK_WORDS		(DES_BLOCK_SIZE >> 2)
>> +
>> +#define _calc_walked(inout) (dd->inout##_walk.offset - dd->inout##_sg->offset)
>> +
>> +#define DES_REG_KEY(dd, x)		((dd)->pdata->key_ofs - \
>> +						((x ^ 0x01) * 0x04))
>> +
>> +#define DES_REG_IV(dd, x)		((dd)->pdata->iv_ofs + ((x) * 0x04))
>> +
>> +#define DES_REG_CTRL(dd)		((dd)->pdata->ctrl_ofs)
>> +#define DES_REG_CTRL_CBC		(1 << 4)
>> +#define DES_REG_CTRL_TDES		(1 << 3)
>> +#define DES_REG_CTRL_DIRECTION		(1 << 2)
>> +#define DES_REG_CTRL_INPUT_READY	(1 << 1)
>> +#define DES_REG_CTRL_OUTPUT_READY	(1 << 0)
> 
> Why not use bitops like you have done below.

Ok, will do that.

> 
>> +
>> +#define DES_REG_DATA_N(dd, x)		((dd)->pdata->data_ofs + ((x) * 0x04))
>> +
>> +#define DES_REG_REV(dd)			((dd)->pdata->rev_ofs)
>> +
>> +#define DES_REG_MASK(dd)		((dd)->pdata->mask_ofs)
>> +
>> +#define DES_REG_LENGTH_N(x)		(0x24 + ((x) * 0x04))
>> +
>> +#define DES_REG_IRQ_STATUS(dd)         ((dd)->pdata->irq_status_ofs)
>> +#define DES_REG_IRQ_ENABLE(dd)         ((dd)->pdata->irq_enable_ofs)
>> +#define DES_REG_IRQ_DATA_IN            BIT(1)
>> +#define DES_REG_IRQ_DATA_OUT           BIT(2)
>> +
>> +#define FLAGS_MODE_MASK		0x000f
>> +#define FLAGS_ENCRYPT		BIT(0)
>> +#define FLAGS_CBC		BIT(1)
>> +#define FLAGS_INIT		BIT(4)
>> +#define FLAGS_BUSY		BIT(6)
>> +
> 
> []..
> 
>> +struct omap_des_pdata {
>> +	struct omap_des_algs_info	*algs_info;
>> +	unsigned int	algs_info_size;
>> +
>> +	void		(*trigger)(struct omap_des_dev *dd, int length);
> 
> Is this really used? How does a DT platform pass function pointers?

This is hard coded in the pdata structures for each platform and provided once
the DT matches. Different platforms may have different pdata.

>> +
>> +	u32		key_ofs;
>> +	u32		iv_ofs;
>> +	u32		ctrl_ofs;
>> +	u32		data_ofs;
>> +	u32		rev_ofs;
>> +	u32		mask_ofs;
>> +	u32             irq_enable_ofs;
>> +	u32             irq_status_ofs;
>> +
>> +	u32		dma_enable_in;
>> +	u32		dma_enable_out;
>> +	u32		dma_start;
>> +
>> +	u32		major_mask;
>> +	u32		major_shift;
>> +	u32		minor_mask;
>> +	u32		minor_shift;
>> +};
>> +
>> +struct omap_des_dev {
>> +	struct list_head	list;
>> +	unsigned long		phys_base;
>> +	void __iomem		*io_base;
>> +	struct omap_des_ctx	*ctx;
>> +	struct device		*dev;
>> +	unsigned long		flags;
>> +	int			err;
>> +
>> +	/* spinlock used for queues */
>> +	spinlock_t		lock;
>> +	struct crypto_queue	queue;
>> +
>> +	struct tasklet_struct	done_task;
>> +	struct tasklet_struct	queue_task;
>> +
>> +	struct ablkcipher_request	*req;
>> +	/*
>> +	 * total is used by PIO mode for book keeping so introduce
>> +	 * variable total_save as need it to calc page_order
>> +	 */
>> +	size_t                          total;
>> +	size_t                          total_save;
>> +
>> +	struct scatterlist		*in_sg;
>> +	struct scatterlist		*out_sg;
>> +
>> +	/* Buffers for copying for unaligned cases */
>> +	struct scatterlist		in_sgl;
>> +	struct scatterlist		out_sgl;
>> +	struct scatterlist		*orig_out;
>> +	int				sgs_copied;
>> +
>> +	struct scatter_walk		in_walk;
>> +	struct scatter_walk		out_walk;
>> +	int			dma_in;
>> +	struct dma_chan		*dma_lch_in;
>> +	int			dma_out;
>> +	struct dma_chan		*dma_lch_out;
>> +	int			in_sg_len;
>> +	int			out_sg_len;
>> +	int			pio_only;
>> +	const struct omap_des_pdata	*pdata;
>> +};
>> +
>> +/* keep registered devices data here */
>> +static LIST_HEAD(dev_list);
>> +static DEFINE_SPINLOCK(list_lock);
>> +
> 
> []..
> 
>> +
>> +static int omap_des_crypt_dma_start(struct omap_des_dev *dd)
>> +{
>> +	struct crypto_tfm *tfm = crypto_ablkcipher_tfm(
>> +					crypto_ablkcipher_reqtfm(dd->req));
>> +	int err;
>> +
>> +	pr_debug("total: %d\n", dd->total);
>> +
>> +	if (!dd->pio_only) {
>> +		err = dma_map_sg(dd->dev, dd->in_sg, dd->in_sg_len,
>> +				 DMA_TO_DEVICE);
>> +		if (!err) {
>> +			dev_err(dd->dev, "dma_map_sg() error\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		err = dma_map_sg(dd->dev, dd->out_sg, dd->out_sg_len,
>> +				 DMA_FROM_DEVICE);
>> +		if (!err) {
>> +			dev_err(dd->dev, "dma_map_sg() error\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	err = omap_des_crypt_dma(tfm, dd->in_sg, dd->out_sg, dd->in_sg_len,
>> +				 dd->out_sg_len);
>> +	if (err && !dd->pio_only) {
>> +		dma_unmap_sg(dd->dev, dd->in_sg, dd->in_sg_len, DMA_TO_DEVICE);
>> +		dma_unmap_sg(dd->dev, dd->out_sg, dd->out_sg_len,
>> +			     DMA_FROM_DEVICE);
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> +static void omap_des_finish_req(struct omap_des_dev *dd, int err)
>> +{
>> +	struct ablkcipher_request *req = dd->req;
>> +
>> +	pr_debug("err: %d\n", err);
>> +
>> +	pm_runtime_put(dd->dev);
> 
> You seem to do a pm_runtime_get_sync() in omap_des_write_ctrl() and a
> pm_runtime_put() here and not a pm_runtime_put_sync()?

Yes, that's on purpose :) I had submitted a fix earlier to do that.
http://www.spinics.net/lists/linux-crypto/msg08482.html

> 
>> +
>> +#ifdef CONFIG_OF
>> +static const struct omap_des_pdata omap_des_pdata_omap4 = {
>> +	.algs_info	= omap_des_algs_info_ecb_cbc,
>> +	.algs_info_size	= ARRAY_SIZE(omap_des_algs_info_ecb_cbc),
>> +	.trigger	= omap_des_dma_trigger_omap4,
>> +	.key_ofs	= 0x14,
>> +	.iv_ofs		= 0x18,
>> +	.ctrl_ofs	= 0x20,
>> +	.data_ofs	= 0x28,
>> +	.rev_ofs	= 0x30,
>> +	.mask_ofs	= 0x34,
>> +	.irq_status_ofs = 0x3c,
>> +	.irq_enable_ofs = 0x40,
>> +	.dma_enable_in	= BIT(5),
>> +	.dma_enable_out	= BIT(6),
>> +	.major_mask	= 0x0700,
>> +	.major_shift	= 8,
>> +	.minor_mask	= 0x003f,
>> +	.minor_shift	= 0,
>> +};
>> +
>> +static irqreturn_t omap_des_irq(int irq, void *dev_id)
>> +{
>> +	struct omap_des_dev *dd = dev_id;
>> +	u32 status, i;
>> +	u32 *src, *dst;
>> +
>> +	status = omap_des_read(dd, DES_REG_IRQ_STATUS(dd));
>> +	if (status & DES_REG_IRQ_DATA_IN) {
>> +		omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x0);
>> +
>> +		BUG_ON(!dd->in_sg);
>> +
>> +		BUG_ON(_calc_walked(in) > dd->in_sg->length);
>> +
>> +		src = sg_virt(dd->in_sg) + _calc_walked(in);
>> +
>> +		for (i = 0; i < DES_BLOCK_WORDS; i++) {
>> +			omap_des_write(dd, DES_REG_DATA_N(dd, i), *src);
>> +
>> +			scatterwalk_advance(&dd->in_walk, 4);
>> +			if (dd->in_sg->length == _calc_walked(in)) {
>> +				dd->in_sg = scatterwalk_sg_next(dd->in_sg);
>> +				if (dd->in_sg) {
>> +					scatterwalk_start(&dd->in_walk,
>> +							  dd->in_sg);
>> +					src = sg_virt(dd->in_sg) +
>> +					      _calc_walked(in);
>> +				}
>> +			} else {
>> +				src++;
>> +			}
>> +		}
>> +
>> +		/* Clear IRQ status */
>> +		status &= ~DES_REG_IRQ_DATA_IN;
>> +		omap_des_write(dd, DES_REG_IRQ_STATUS(dd), status);
>> +
>> +		/* Enable DATA_OUT interrupt */
>> +		omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x4);
>> +
>> +	} else if (status & DES_REG_IRQ_DATA_OUT) {
>> +		omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x0);
>> +
>> +		BUG_ON(!dd->out_sg);
>> +
>> +		BUG_ON(_calc_walked(out) > dd->out_sg->length);
>> +
>> +		dst = sg_virt(dd->out_sg) + _calc_walked(out);
>> +
>> +		for (i = 0; i < DES_BLOCK_WORDS; i++) {
>> +			*dst = omap_des_read(dd, DES_REG_DATA_N(dd, i));
>> +			scatterwalk_advance(&dd->out_walk, 4);
>> +			if (dd->out_sg->length == _calc_walked(out)) {
>> +				dd->out_sg = scatterwalk_sg_next(dd->out_sg);
>> +				if (dd->out_sg) {
>> +					scatterwalk_start(&dd->out_walk,
>> +							  dd->out_sg);
>> +					dst = sg_virt(dd->out_sg) +
>> +					      _calc_walked(out);
>> +				}
>> +			} else {
>> +				dst++;
>> +			}
>> +		}
>> +
>> +		dd->total -= DES_BLOCK_SIZE;
>> +
>> +		BUG_ON(dd->total < 0);
>> +
>> +		/* Clear IRQ status */
>> +		status &= ~DES_REG_IRQ_DATA_OUT;
>> +		omap_des_write(dd, DES_REG_IRQ_STATUS(dd), status);
>> +
>> +		if (!dd->total)
>> +			/* All bytes read! */
>> +			tasklet_schedule(&dd->done_task);
>> +		else
>> +			/* Enable DATA_IN interrupt for next block */
>> +			omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x2);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static const struct of_device_id omap_des_of_match[] = {
>> +	{
>> +		.compatible	= "ti,omap4-des",
>> +		.data		= &omap_des_pdata_omap4,
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, omap_des_of_match);
>> +
>> +static int omap_des_get_res_of(struct omap_des_dev *dd,
>> +		struct device *dev, struct resource *res)
>> +{
>> +	struct device_node *node = dev->of_node;
>> +	const struct of_device_id *match;
>> +	int err = 0;
>> +
>> +	match = of_match_device(of_match_ptr(omap_des_of_match), dev);
>> +	if (!match) {
>> +		dev_err(dev, "no compatible OF match\n");
>> +		err = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	err = of_address_to_resource(node, 0, res);
> 
> Do you really need to do this? DT should have already populated
> a resource for you based on the 'reg' property.

Ok, thanks. Now will do:
        *res = platform_get_resource(pdev, IORESOURCE_MEM);

>> +	if (err < 0) {
>> +		dev_err(dev, "can't translate OF node address\n");
>> +		err = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	dd->dma_out = -1; /* Dummy value that's unused */
>> +	dd->dma_in = -1; /* Dummy value that's unused */
>> +
>> +	dd->pdata = match->data;
>> +
>> +err:
>> +	return err;
>> +}
>> +#else
>> +static const struct of_device_id omap_des_of_match[] = {
>> +	{},
>> +};
> 
> you don't need this if you use of_match_ptr()

Ok I changed it to:
+               .of_match_table = of_match_ptr(omap_des_of_match),

and removed empty definition for !CONFIG_OF.

>> +
>> +static int omap_des_get_res_of(struct omap_des_dev *dd,
>> +		struct device *dev, struct resource *res)
>> +{
>> +	return -EINVAL;
>> +}
>> +#endif
>> +
>> +static int omap_des_get_res_pdev(struct omap_des_dev *dd,
>> +		struct platform_device *pdev, struct resource *res)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct resource *r;
>> +	int err = 0;
>> +
>> +	/* Get the base address */
>> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!r) {
>> +		dev_err(dev, "no MEM resource info\n");
>> +		err = -ENODEV;
>> +		goto err;
>> +	}
>> +	memcpy(res, r, sizeof(*res));
> 
> I don't think you need any of this. Regardless of a DT or a
> non-DT platform, you should be able to do a platform_get_resource()
> for mem resources.

Yes ok, I removed all this and am directly using the res pointer now instead of
pre-filling a structure. Also both DT/non-DT paths now use platform_get_resource.

[..]
>> +	tasklet_kill(&dd->done_task);
>> +	tasklet_kill(&dd->queue_task);
>> +	omap_des_dma_cleanup(dd);
>> +	pm_runtime_disable(dd->dev);
>> +	dd = NULL;
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int omap_des_suspend(struct device *dev)
>> +{
>> +	pm_runtime_put_sync(dev);
> 
> I know you seemed to have taken this from the omap-aes driver, but this
> does not seem correct. Your system suspend callbacks shouldn't
> really be using pm_runtime apis.
> On OMAPs this is handled by the pm_domain level callbacks, in case
> your device is not runtime suspended during system suspend.

Can you suggest how else to handle this?

>> +	return 0;
>> +}
>> +
>> +static int omap_des_resume(struct device *dev)
>> +{
>> +	pm_runtime_get_sync(dev);
> 
> Same here.
> Btw, has the omap-aes or this driver been tested with system
> suspend on any platfoms?

Yes, omap-aes has been successfully tested with Suspend/Resume on AM335x platforms.

>> +	return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops omap_des_pm_ops = {
>> +	SET_SYSTEM_SLEEP_PM_OPS(omap_des_suspend, omap_des_resume)
>> +};
>> +
>> +static struct platform_driver omap_des_driver = {
>> +	.probe	= omap_des_probe,
>> +	.remove	= omap_des_remove,
>> +	.driver	= {
>> +		.name	= "omap-des",
>> +		.owner	= THIS_MODULE,
>> +		.pm	= &omap_des_pm_ops,
>> +		.of_match_table	= omap_des_of_match,
> 
> You could use of_match_ptr() here and avoid having the empty
> omap_des_of_match defined.

Done, thanks.

Regards,

-Joel

>> +	},
>> +};
>> +
>> +module_platform_driver(omap_des_driver);
>> +
>> +MODULE_DESCRIPTION("OMAP DES hw acceleration support.");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Joel Fernandes <joelf@ti.com>");
>>
> 

^ permalink raw reply

* Re: [PATCH] modules: add support for soft module dependencies
From: Tom Gundersen @ 2013-09-10 14:18 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Herbert Xu, Rusty Russell, Andreas Robinson,
	Linux Kernel Mailing List, Zhao Hongjiang, David Miller,
	tim.c.chen-VuQAYsv1563Yd54FQh9/CA, Andrew Morton,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, linux-modules
In-Reply-To: <CAKi4VA+=1vOOt0BqdQHQaeF1hP3DrRtCL_ObXY0cPeO7S_MtYw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Sep 10, 2013 at 4:01 PM, Lucas De Marchi
<lucas.de.marchi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
> <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org> wrote:
>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>> Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org> writes:
>>> > Hi Rusty:
>>> >
>>> > I don't know why this patch never went into the kernel, even
>>> > though the corresponding features have been added to modprobe
>>> > in most if not all distros.
>>>
>>> Because Andreas never sent me the patch?  This is the first I've *heard*
>>> of this feature.  Looks like it didn't hit lkml either.  And what was
>>> 2/2?
>>
>> 2/2 was the patch to actually use this in crc32c.
>>
>>> It's not how I would have done this: post-deps are more flexibly done at
>>> runtime, because the module may have to do work to figure out what to
>>> pull in.  But since it already exists, I'll apply this patch: it doesn't
>>> cost the kernel anything.
>
> But it did cause boot failures. The file modules.softdep file was
> supposed to be informational until now. That's why depmod put a
> comment saying to "copy on user's discretion to /etc/modules.d"
> instead of parsing it directly.
>
> If dependencies expressed with softdeps can be required dependencies
> now, depmod needs to be updated to work it out otherwise we will get
> missing dependencies as happened with the crypto stuff after this
> patch. I'm CC'ing Tom who had the boot failure; it looks like the same
> boot failure that caused this feature to get reverted the first time
> (the deps being required by sd_mod).

For the record: what happened was that I didn't get the "softdep'ed"
modules  in my initrd and when trying to modprobe sd_mod in the initrd
I got an error message about missing symbols (I forget the precise
wording). We cold certainly fix the initrd generation to include the
softdep modules, but it seems unexpected that missing softdeps should
cause failures like this.

> Even if it is an optional module, it would be great to have depmod
> updated so it parses the file directly now that we are exporting this
> info. I can do that once we agree what we want to do with softdeps
> coming directly from kernel itself.

I just submitted a patch to do this, I think it is much more useful
than expecting the packager/admin to copy modules.softdep to the
correct location.

> However looking at the only user right now, crct10dif: couldn't we
> detect at runtime if this module can be used instead of just trying to
> load it as a pre softdep and possibly failing?

Having softdeps statically specified is useful for inird generation.
Even if it turns out that the modules are truly optional, having the
possibility of automatically including all optional modules would
surely be useful.

Cheers,

Tom

^ permalink raw reply

* Re: [PATCH] modules: add support for soft module dependencies
From: Lucas De Marchi @ 2013-09-10 14:01 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Rusty Russell, Andreas Robinson, Linux Kernel Mailing List,
	Zhao Hongjiang, David Miller, tim.c.chen-VuQAYsv1563Yd54FQh9/CA,
	Andrew Morton,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, linux-modules, Tom Gundersen
In-Reply-To: <20130725020329.GA14625-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>

On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
<herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org> wrote:
> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>> Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org> writes:
>> > Hi Rusty:
>> >
>> > I don't know why this patch never went into the kernel, even
>> > though the corresponding features have been added to modprobe
>> > in most if not all distros.
>>
>> Because Andreas never sent me the patch?  This is the first I've *heard*
>> of this feature.  Looks like it didn't hit lkml either.  And what was
>> 2/2?
>
> 2/2 was the patch to actually use this in crc32c.
>
>> It's not how I would have done this: post-deps are more flexibly done at
>> runtime, because the module may have to do work to figure out what to
>> pull in.  But since it already exists, I'll apply this patch: it doesn't
>> cost the kernel anything.

But it did cause boot failures. The file modules.softdep file was
supposed to be informational until now. That's why depmod put a
comment saying to "copy on user's discretion to /etc/modules.d"
instead of parsing it directly.

If dependencies expressed with softdeps can be required dependencies
now, depmod needs to be updated to work it out otherwise we will get
missing dependencies as happened with the crypto stuff after this
patch. I'm CC'ing Tom who had the boot failure; it looks like the same
boot failure that caused this feature to get reverted the first time
(the deps being required by sd_mod).

Even if it is an optional module, it would be great to have depmod
updated so it parses the file directly now that we are exporting this
info. I can do that once we agree what we want to do with softdeps
coming directly from kernel itself.

However looking at the only user right now, crct10dif: couldn't we
detect at runtime if this module can be used instead of just trying to
load it as a pre softdep and possibly failing?


Lucas De Marchi

^ permalink raw reply

* Re: [PATCH] crypto: tegra: use kernel entropy instead of ad-hoc
From: Stephen Warren @ 2013-09-09 16:02 UTC (permalink / raw)
  To: Linus Walleij, Varun Wadekar
  Cc: linux-crypto-u79uwXL29TY76Z2rM5mHXA, Herbert Xu,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Neil Horman,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1378712154-30602-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 09/09/2013 01:35 AM, Linus Walleij wrote:
> The way I read the Tegra AES RNG is that it has a homebrew
> algorithm for initializing the 128bit RNG using timespec and
> the unique chip ID. This looks like reinventing the (square)
> wheel, instead just grab 128bits from the kernel entropy pool
> where the time and (after another patch) chip unique ID is
> already mixed in.
> 
> Incidentally this also gets rid of a rather ugly
> cross-dependence on the machine using an extern declaration.

This sounds reasonable to me, although I know little about the driver.
Varun, can you please comment?

Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply

* [PATCH 7/7] crypto: caam - enable instantiation of all RNG4 state handles
From: Alex Porosanu @ 2013-09-09 15:56 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, horia.geanta, vakul
In-Reply-To: <1378742194-32194-1-git-send-email-alexandru.porosanu@freescale.com>

RNG4 block contains multiple (i.e. 2) state handles that can be
initialized. This patch adds the necessary code for detecting
which of the two state handles has been instantiated by another
piece of software e.g. u-boot and instantiate the other one (or
both if none was instantiated). Only the state handle(s)
instantiated by this driver will be deinstantiated when removing
the module.

Signed-off-by: Alex Porosanu <alexandru.porosanu@freescale.com>
---
 drivers/crypto/caam/ctrl.c   |  206 ++++++++++++++++++++++++++++++++----------
 drivers/crypto/caam/intern.h |    8 +-
 drivers/crypto/caam/regs.h   |    7 ++-
 3 files changed, 169 insertions(+), 52 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 29cbec1..26438cd 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -17,41 +17,49 @@
  * Descriptor to instantiate RNG State Handle 0 in normal mode and
  * load the JDKEK, TDKEK and TDSK registers
  */
-static void build_instantiation_desc(u32 *desc)
+static void build_instantiation_desc(u32 *desc, int handle, int do_sk)
 {
-	u32 *jump_cmd;
+	u32 *jump_cmd, op_flags;
 
 	init_job_desc(desc, 0);
 
+	op_flags = OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
+			(handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT;
+
 	/* INIT RNG in non-test mode */
-	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
-			 OP_ALG_AS_INIT);
+	append_operation(desc, op_flags);
 
-	/* wait for done */
-	jump_cmd = append_jump(desc, JUMP_CLASS_CLASS1);
-	set_jump_tgt_here(desc, jump_cmd);
+	if (!handle && do_sk) {
+		/*
+		 * For SH0, Secure Keys must be generated as well
+		 */
 
-	/*
-	 * load 1 to clear written reg:
-	 * resets the done interrupt and returns the RNG to idle.
-	 */
-	append_load_imm_u32(desc, 1, LDST_SRCDST_WORD_CLRW);
+		/* wait for done */
+		jump_cmd = append_jump(desc, JUMP_CLASS_CLASS1);
+		set_jump_tgt_here(desc, jump_cmd);
 
-	/* generate secure keys (non-test) */
-	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
-			 OP_ALG_AAI_RNG4_SK);
+		/*
+		 * load 1 to clear written reg:
+		 * resets the done interrrupt and returns the RNG to idle.
+		 */
+		append_load_imm_u32(desc, 1, LDST_SRCDST_WORD_CLRW);
+
+		/* Initialize State Handle  */
+		append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
+				 OP_ALG_AAI_RNG4_SK);
+	}
 
 	append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
 }
 
 /* Descriptor for deinstantiation of State Handle 0 of the RNG block. */
-static void build_deinstantiation_desc(u32 *desc)
+static void build_deinstantiation_desc(u32 *desc, int handle)
 {
 	init_job_desc(desc, 0);
 
 	/* Uninstantiate State Handle 0 */
 	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
-			 OP_ALG_AS_INITFINAL);
+			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INITFINAL);
 
 	append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
 }
@@ -60,11 +68,14 @@ static void build_deinstantiation_desc(u32 *desc)
  * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
  *			  the software (no JR/QI used).
  * @ctrldev - pointer to device
+ * @status - descriptor status, after being run
+ *
  * Return: - 0 if no error occurred
  *	   - -ENODEV if the DECO couldn't be acquired
  *	   - -EAGAIN if an error occurred while executing the descriptor
  */
-static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
+static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
+					u32 *status)
 {
 	struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev);
 	struct caam_full __iomem *topregs;
@@ -113,6 +124,9 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
 		cpu_relax();
 	} while ((deco_dbg_reg & DESC_DBG_DECO_STAT_VALID) && --timeout);
 
+	*status = rd_reg32(&topregs->deco.op_status_hi) &
+		  DECO_OP_STATUS_HI_ERR_MASK;
+
 	/* Mark the DECO as free */
 	clrbits32(&topregs->ctrl.deco_rq, DECORR_RQD0ENABLE);
 
@@ -126,6 +140,14 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
  * instantiate_rng - builds and executes a descriptor on DECO0,
  *		     which initializes the RNG block.
  * @ctrldev - pointer to device
+ * @state_handle_mask - bitmask containing the instantiation status
+ *			for the RNG4 state handles which exist in
+ *			the RNG4 block: 1 if it's been instantiated
+ *			by an external entry, 0 otherwise.
+ * @gen_sk  - generate data to be loaded into the JDKEK, TDKEK and TDSK;
+ *	      Caution: this can be done only once; if the keys need to be
+ *	      regenerated, a POR is required
+ *
  * Return: - 0 if no error occurred
  *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
  *	   - -ENODEV if DECO0 couldn't be acquired
@@ -133,19 +155,56 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
  *	      f.i. there was a RNG hardware error due to not "good enough"
  *	      entropy being aquired.
  */
-static int instantiate_rng(struct device *ctrldev)
+static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
+			   int gen_sk)
 {
-	u32 *desc;
-	int ret = 0;
+	struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev);
+	struct caam_full __iomem *topregs;
+	struct rng4tst __iomem *r4tst;
+	u32 *desc, status, rdsta_val;
+	int ret = 0, sh_idx;
+
+	topregs = (struct caam_full __iomem *)ctrlpriv->ctrl;
+	r4tst = &topregs->ctrl.r4tst[0];
 
 	desc = kmalloc(CAAM_CMD_SZ * 7, GFP_KERNEL);
 	if (!desc)
 		return -ENOMEM;
-	/* Create the descriptor for instantiating RNG State Handle 0 */
-	build_instantiation_desc(desc);
 
-	/* Try to run it through DECO0 */
-	ret = run_descriptor_deco0(ctrldev, desc);
+	for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
+		/*
+		 * If the corresponding bit is set, this state handle
+		 * was initialized by somebody else, so it's left alone.
+		 */
+		if ((1 << sh_idx) & state_handle_mask)
+			continue;
+
+		/* Create the descriptor for instantiating RNG State Handle */
+		build_instantiation_desc(desc, sh_idx, gen_sk);
+
+		/* Try to run it through DECO0 */
+		ret = run_descriptor_deco0(ctrldev, desc, &status);
+
+		/*
+		 * If ret is not 0, or descriptor status is not 0, then
+		 * something went wrong. No need to try the next state
+		 * handle (if available), bail out here.
+		 * Also, if for some reason, the State Handle didn't get
+		 * instantiated although the descriptor has finished
+		 * without any error (HW optimizations for later
+		 * CAAM eras), then try again.
+		 */
+		rdsta_val =
+			rd_reg32(&topregs->ctrl.r4tst[0].rdsta) & RDSTA_IFMASK;
+		if (status || !(rdsta_val & (1 << sh_idx)))
+			ret = -EAGAIN;
+		if (ret)
+			break;
+
+		dev_info(ctrldev, "Instantiated RNG4 SH%d\n", sh_idx);
+		/* Clear the contents before recreating the descriptor */
+		memset(desc, 0x00, CAAM_CMD_SZ * 7);
+	}
 
 	kfree(desc);
 
@@ -156,29 +215,49 @@ static int instantiate_rng(struct device *ctrldev)
  * deinstantiate_rng - builds and executes a descriptor on DECO0,
  *		       which deinitializes the RNG block.
  * @ctrldev - pointer to device
+ * @state_handle_mask - bitmask containing the instantiation status
+ *			for the RNG4 state handles which exist in
+ *			the RNG4 block: 1 if it's been instantiated
  *
  * Return: - 0 if no error occurred
  *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
  *	   - -ENODEV if DECO0 couldn't be acquired
  *	   - -EAGAIN if an error occurred when executing the descriptor
  */
-static int deinstantiate_rng(struct device *ctrldev)
+static int deinstantiate_rng(struct device *ctrldev, int state_handle_mask)
 {
-	u32 *desc;
-	int i, ret = 0;
+	u32 *desc, status;
+	int sh_idx, ret = 0;
 
 	desc = kmalloc(CAAM_CMD_SZ * 3, GFP_KERNEL);
 	if (!desc)
 		return -ENOMEM;
 
-	/* Create the descriptor for deinstantating RNG State Handle 0 */
-	build_deinstantiation_desc(desc);
-
-	/* Try to run it through DECO0 */
-	ret = run_descriptor_deco0(ctrldev, desc);
-
-	if (ret)
-		dev_err(ctrldev, "failed to deinstantiate RNG\n");
+	for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
+		/*
+		 * If the corresponding bit is set, then it means the state
+		 * handle was initialized by us, and thus it needs to be
+		 * deintialized as well
+		 */
+		if ((1 << sh_idx) & state_handle_mask) {
+			/*
+			 * Create the descriptor for deinstantating this state
+			 * handle
+			 */
+			build_deinstantiation_desc(desc, sh_idx);
+
+			/* Try to run it through DECO0 */
+			ret = run_descriptor_deco0(ctrldev, desc, &status);
+
+			if (ret || status) {
+				dev_err(ctrldev,
+					"Failed to deinstantiate RNG4 SH%d\n",
+					sh_idx);
+				break;
+			}
+			dev_info(ctrldev, "Deinstantiated RNG4 SH%d\n", sh_idx);
+		}
+	}
 
 	kfree(desc);
 
@@ -204,9 +283,9 @@ static int caam_remove(struct platform_device *pdev)
 		irq_dispose_mapping(jrpriv->irq);
 	}
 
-	/* De-initialize RNG if it was initialized by this driver. */
-	if (ctrlpriv->rng4_init)
-		deinstantiate_rng(ctrldev);
+	/* De-initialize RNG state handles initialized by this driver. */
+	if (ctrlpriv->rng4_sh_init)
+		deinstantiate_rng(ctrldev, ctrlpriv->rng4_sh_init);
 
 	/* Shut down debug views */
 #ifdef CONFIG_DEBUG_FS
@@ -306,7 +385,7 @@ EXPORT_SYMBOL(caam_get_era);
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
-	int ret, ring, rspec, ent_delay = RTSDCTL_ENT_DLY_MIN;
+	int ret, ring, rspec, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
 	u64 caam_id;
 	struct device *dev;
 	struct device_node *nprop, *np;
@@ -414,20 +493,53 @@ static int caam_probe(struct platform_device *pdev)
 	 * If SEC has RNG version >= 4 and RNG state handle has not been
 	 * already instantiated, do RNG instantiation
 	 */
-	if ((cha_vid & CHA_ID_RNG_MASK) >> CHA_ID_RNG_SHIFT >= 4 &&
-	    !(rd_reg32(&topregs->ctrl.r4tst[0].rdsta) & RDSTA_IF0)) {
+	if ((cha_vid & CHA_ID_RNG_MASK) >> CHA_ID_RNG_SHIFT >= 4) {
+		ctrlpriv->rng4_sh_init =
+			rd_reg32(&topregs->ctrl.r4tst[0].rdsta);
+		/*
+		 * If the secure keys (TDKEK, JDKEK, TDSK), were already
+		 * generated, signal this to the function that is instantiating
+		 * the state handles. An error would occur if RNG4 attempts
+		 * to regenerate these keys before the next POR.
+		 */
+		gen_sk = ctrlpriv->rng4_sh_init & RDSTA_SKVN ? 0 : 1;
+		ctrlpriv->rng4_sh_init &= RDSTA_IFMASK;
 		do {
-			kick_trng(pdev, ent_delay);
-			ret = instantiate_rng(dev);
-			ent_delay += 400;
+			int inst_handles =
+				rd_reg32(&topregs->ctrl.r4tst[0].rdsta) &
+								RDSTA_IFMASK;
+			/*
+			 * If either SH were instantiated by somebody else
+			 * (e.g. u-boot) then it is assumed that the entropy
+			 * parameters are properly set and thus the function
+			 * setting these (kick_trng(...)) is skipped.
+			 * Also, if a handle was instantiated, do not change
+			 * the TRNG parameters.
+			 */
+			if (!(ctrlpriv->rng4_sh_init || inst_handles)) {
+				kick_trng(pdev, ent_delay);
+				ent_delay += 400;
+			}
+			/*
+			 * if instantiate_rng(...) fails, the loop will rerun
+			 * and the kick_trng(...) function will modfiy the
+			 * upper and lower limits of the entropy sampling
+			 * interval, leading to a sucessful initialization of
+			 * the RNG.
+			 */
+			ret = instantiate_rng(dev, inst_handles,
+					      gen_sk);
 		} while ((ret == -EAGAIN) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
 		if (ret) {
 			dev_err(dev, "failed to instantiate RNG");
 			caam_remove(pdev);
 			return ret;
 		}
-
-		ctrlpriv->rng4_init = 1;
+		/*
+		 * Set handles init'ed by this module as the complement of the
+		 * already initialized ones
+		 */
+		ctrlpriv->rng4_sh_init = ~ctrlpriv->rng4_sh_init & RDSTA_IFMASK;
 
 		/* Enable RDB bit so that RNG works faster */
 		setbits32(&topregs->ctrl.scfgr, SCFGR_RDBENABLE);
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index ada2429..bbc1ac9 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -87,11 +87,11 @@ struct caam_drv_private {
 	/* list of registered hash algorithms (mk generic context handle?) */
 	struct list_head hash_list;
 
+#define	RNG4_MAX_HANDLES 2
 	/* RNG4 block */
-	bool rng4_init;		/* If RNG4 block is initialized by this driver,
-				   then this will be set; if it was initialized
-				   by another entity (e.g. u-boot), it will be
-				   cleared. */
+	u32 rng4_sh_init;	/* This bitmap shows which of the State
+				   Handles of the RNG4 block are initialized
+				   by this driver */
 
 	/*
 	 * debugfs entries for developer view into driver/device
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 9aa9f71..d50174f 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -245,7 +245,7 @@ struct rngtst {
 
 /* RNG4 TRNG test registers */
 struct rng4tst {
-#define RTMCTL_PRGM 0x00010000	/* 1 -> program mode, 0 -> run mode */
+#define RTMCTL_PRGM	0x00010000	/* 1 -> program mode, 0 -> run mode */
 	u32 rtmctl;		/* misc. control register */
 	u32 rtscmisc;		/* statistical check misc. register */
 	u32 rtpkrrng;		/* poker range register */
@@ -268,7 +268,11 @@ struct rng4tst {
 		u32 rtfrqcnt;	/* PRGM=0: freq. count register */
 	};
 	u32 rsvd1[40];
+#define RDSTA_SKVT 0x80000000
+#define RDSTA_SKVN 0x40000000
 #define RDSTA_IF0 0x00000001
+#define RDSTA_IF1 0x00000002
+#define RDSTA_IFMASK (RDSTA_IF1 | RDSTA_IF0)
 	u32 rdsta;
 	u32 rsvd2[15];
 };
@@ -694,6 +698,7 @@ struct caam_deco {
 	u32 jr_ctl_hi;	/* CxJRR - JobR Control Register      @800 */
 	u32 jr_ctl_lo;
 	u64 jr_descaddr;	/* CxDADR - JobR Descriptor Address */
+#define DECO_OP_STATUS_HI_ERR_MASK 0xF00000FF
 	u32 op_status_hi;	/* DxOPSTA - DECO Operation Status */
 	u32 op_status_lo;
 	u32 rsvd24[2];
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH 6/7] crypto: caam - fix RNG4 AAI defines
From: Alex Porosanu @ 2013-09-09 15:56 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, horia.geanta, vakul
In-Reply-To: <1378742194-32194-1-git-send-email-alexandru.porosanu@freescale.com>

RNG4 defines in desc.h were incomplete (bits AI & PS were missing),
while SK was set as an ALG related bit. This patchs adds the
missing bits and corrects the SK bit.

Signed-off-by: Alex Porosanu <alexandru.porosanu@freescale.com>
---
 drivers/crypto/caam/ctrl.c |    2 +-
 drivers/crypto/caam/desc.h |   17 +++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 9e9215a..29cbec1 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -39,7 +39,7 @@ static void build_instantiation_desc(u32 *desc)
 
 	/* generate secure keys (non-test) */
 	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
-			 OP_ALG_RNG4_SK);
+			 OP_ALG_AAI_RNG4_SK);
 
 	append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
 }
diff --git a/drivers/crypto/caam/desc.h b/drivers/crypto/caam/desc.h
index 53b296f..7e4500f 100644
--- a/drivers/crypto/caam/desc.h
+++ b/drivers/crypto/caam/desc.h
@@ -1155,8 +1155,15 @@ struct sec4_sg_entry {
 
 /* randomizer AAI set */
 #define OP_ALG_AAI_RNG		(0x00 << OP_ALG_AAI_SHIFT)
-#define OP_ALG_AAI_RNG_NOZERO	(0x10 << OP_ALG_AAI_SHIFT)
-#define OP_ALG_AAI_RNG_ODD	(0x20 << OP_ALG_AAI_SHIFT)
+#define OP_ALG_AAI_RNG_NZB	(0x10 << OP_ALG_AAI_SHIFT)
+#define OP_ALG_AAI_RNG_OBP	(0x20 << OP_ALG_AAI_SHIFT)
+
+/* RNG4 AAI set */
+#define OP_ALG_AAI_RNG4_SH_0	(0x00 << OP_ALG_AAI_SHIFT)
+#define OP_ALG_AAI_RNG4_SH_1	(0x01 << OP_ALG_AAI_SHIFT)
+#define OP_ALG_AAI_RNG4_PS	(0x40 << OP_ALG_AAI_SHIFT)
+#define OP_ALG_AAI_RNG4_AI	(0x80 << OP_ALG_AAI_SHIFT)
+#define OP_ALG_AAI_RNG4_SK	(0x100 << OP_ALG_AAI_SHIFT)
 
 /* hmac/smac AAI set */
 #define OP_ALG_AAI_HASH		(0x00 << OP_ALG_AAI_SHIFT)
@@ -1178,12 +1185,6 @@ struct sec4_sg_entry {
 #define OP_ALG_AAI_GSM		(0x10 << OP_ALG_AAI_SHIFT)
 #define OP_ALG_AAI_EDGE		(0x20 << OP_ALG_AAI_SHIFT)
 
-/* RNG4 set */
-#define OP_ALG_RNG4_SHIFT	4
-#define OP_ALG_RNG4_MASK	(0x1f3 << OP_ALG_RNG4_SHIFT)
-
-#define OP_ALG_RNG4_SK		(0x100 << OP_ALG_RNG4_SHIFT)
-
 #define OP_ALG_AS_SHIFT		2
 #define OP_ALG_AS_MASK		(0x3 << OP_ALG_AS_SHIFT)
 #define OP_ALG_AS_UPDATE	(0 << OP_ALG_AS_SHIFT)
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH 5/7] crypto: caam - uninstantiate RNG state handle 0 if instantiated by caam driver
From: Alex Porosanu @ 2013-09-09 15:56 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, horia.geanta, vakul
In-Reply-To: <1378742194-32194-1-git-send-email-alexandru.porosanu@freescale.com>

If the caam driver module instantiates the RNG state handle 0, then
upon the removal of the module, the RNG state handle is left
initialized. This patch takes care of reverting the state of the
handle back to its previous uninstantatied state.

Signed-off-by: Alex Porosanu <alexandru.porosanu@freescale.com>
---
 drivers/crypto/caam/ctrl.c   |   57 +++++++++++++++++++++++++++++++++++++----
 drivers/crypto/caam/intern.h |    6 ++++
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 3ad032f..9e9215a 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -44,6 +44,17 @@ static void build_instantiation_desc(u32 *desc)
 	append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
 }
 
+/* Descriptor for deinstantiation of State Handle 0 of the RNG block. */
+static void build_deinstantiation_desc(u32 *desc)
+{
+	init_job_desc(desc, 0);
+
+	/* Uninstantiate State Handle 0 */
+	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
+			 OP_ALG_AS_INITFINAL);
+
+	append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
+}
 
 /*
  * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
@@ -59,7 +70,7 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
 	struct caam_full __iomem *topregs;
 	unsigned int timeout = 100000;
 	u32 deco_dbg_reg, flags;
-	int i, ret = 0;
+	int i;
 
 	/* Set the bit to request direct access to DECO0 */
 	topregs = (struct caam_full __iomem *)ctrlpriv->ctrl;
@@ -102,11 +113,6 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
 		cpu_relax();
 	} while ((deco_dbg_reg & DESC_DBG_DECO_STAT_VALID) && --timeout);
 
-	if (!timeout) {
-		dev_err(ctrldev, "failed to instantiate RNG\n");
-		ret = -EIO;
-	}
-
 	/* Mark the DECO as free */
 	clrbits32(&topregs->ctrl.deco_rq, DECORR_RQD0ENABLE);
 
@@ -146,6 +152,39 @@ static int instantiate_rng(struct device *ctrldev)
 	return ret;
 }
 
+/*
+ * deinstantiate_rng - builds and executes a descriptor on DECO0,
+ *		       which deinitializes the RNG block.
+ * @ctrldev - pointer to device
+ *
+ * Return: - 0 if no error occurred
+ *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
+ *	   - -ENODEV if DECO0 couldn't be acquired
+ *	   - -EAGAIN if an error occurred when executing the descriptor
+ */
+static int deinstantiate_rng(struct device *ctrldev)
+{
+	u32 *desc;
+	int i, ret = 0;
+
+	desc = kmalloc(CAAM_CMD_SZ * 3, GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+
+	/* Create the descriptor for deinstantating RNG State Handle 0 */
+	build_deinstantiation_desc(desc);
+
+	/* Try to run it through DECO0 */
+	ret = run_descriptor_deco0(ctrldev, desc);
+
+	if (ret)
+		dev_err(ctrldev, "failed to deinstantiate RNG\n");
+
+	kfree(desc);
+
+	return ret;
+}
+
 static int caam_remove(struct platform_device *pdev)
 {
 	struct device *ctrldev;
@@ -165,6 +204,10 @@ static int caam_remove(struct platform_device *pdev)
 		irq_dispose_mapping(jrpriv->irq);
 	}
 
+	/* De-initialize RNG if it was initialized by this driver. */
+	if (ctrlpriv->rng4_init)
+		deinstantiate_rng(ctrldev);
+
 	/* Shut down debug views */
 #ifdef CONFIG_DEBUG_FS
 	debugfs_remove_recursive(ctrlpriv->dfs_root);
@@ -384,6 +427,8 @@ static int caam_probe(struct platform_device *pdev)
 			return ret;
 		}
 
+		ctrlpriv->rng4_init = 1;
+
 		/* Enable RDB bit so that RNG works faster */
 		setbits32(&topregs->ctrl.scfgr, SCFGR_RDBENABLE);
 	}
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 34c4b9f..ada2429 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -87,6 +87,12 @@ struct caam_drv_private {
 	/* list of registered hash algorithms (mk generic context handle?) */
 	struct list_head hash_list;
 
+	/* RNG4 block */
+	bool rng4_init;		/* If RNG4 block is initialized by this driver,
+				   then this will be set; if it was initialized
+				   by another entity (e.g. u-boot), it will be
+				   cleared. */
+
 	/*
 	 * debugfs entries for developer view into driver/device
 	 * variables at runtime.
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH 3/7] crypto: caam - fix RNG4 instantiation
From: Alex Porosanu @ 2013-09-09 15:56 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, horia.geanta, vakul
In-Reply-To: <1378742194-32194-1-git-send-email-alexandru.porosanu@freescale.com>

The RNG4 block in CAAM needs to be 'seeded' first before being used
for generating pseudo-random data. The 'seeding' is done by getting
entropy from the TRNG ring oscillator. The RTFRQMAX register controls
the maximum allowable number of samples that can be aquired during
an entropy sample. Depending on the clock at which the RNG4 block
(and for that matter the SEC block) runs, it's possible that a
hard-coded value for the maximum frequency is inadequate, i.e. more
samples than needed are taken. This is an error, and thus the RNG4
block doesn't get initialized.  The patch attempts to alleviate
this issue by trying with progressivly larger frequencies, until
the number of samples is adequate.
This patch also fixes how a descriptor is deemed as being finished:
instead of checking the VALID field in the DECO debug register,
it makes sure that the DECO is idle, by checking the DECO state field
of the said register.

Signed-off-by: Alex Porosanu <alexandru.porosanu@freescale.com>
---
 drivers/crypto/caam/ctrl.c |   63 ++++++++++++++++++++++++++++++++-----------
 drivers/crypto/caam/regs.h |    7 +++-
 2 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 11c7f29..d5fe5f5 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -82,7 +82,7 @@ static int instantiate_rng(struct device *ctrldev)
 	struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev);
 	struct caam_full __iomem *topregs;
 	unsigned int timeout = 100000;
-	u32 *desc;
+	u32 *desc, deco_dbg_reg;
 	int i, ret = 0;
 
 	desc = kmalloc(CAAM_CMD_SZ * 7, GFP_KERNEL | GFP_DMA);
@@ -112,9 +112,17 @@ static int instantiate_rng(struct device *ctrldev)
 	wr_reg32(&topregs->deco.jr_ctl_hi, DECO_JQCR_WHL | DECO_JQCR_FOUR);
 
 	timeout = 10000000;
-	while ((rd_reg32(&topregs->deco.desc_dbg) & DECO_DBG_VALID) &&
-								 --timeout)
+	do {
+		deco_dbg_reg = rd_reg32(&topregs->deco.desc_dbg);
+		/*
+		 * If an error occured in the descriptor, then
+		 * the DECO status field will be set to 0x0D
+		 */
+		if ((deco_dbg_reg & DESC_DBG_DECO_STAT_MASK) ==
+		    DESC_DBG_DECO_STAT_HOST_ERR)
+			break;
 		cpu_relax();
+	} while ((deco_dbg_reg & DESC_DBG_DECO_STAT_VALID) && --timeout);
 
 	if (!timeout) {
 		dev_err(ctrldev, "failed to instantiate RNG\n");
@@ -128,10 +136,12 @@ out:
 }
 
 /*
- * By default, the TRNG runs for 200 clocks per sample;
- * 1600 clocks per sample generates better entropy.
+ * kick_trng - sets the various parameters for enabling the initialization
+ *	       of the RNG4 block in CAAM
+ * @pdev - pointer to the platform device
+ * @ent_delay - Defines the length (in system clocks) of each entropy sample.
  */
-static void kick_trng(struct platform_device *pdev)
+static void kick_trng(struct platform_device *pdev, int ent_delay)
 {
 	struct device *ctrldev = &pdev->dev;
 	struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev);
@@ -144,14 +154,31 @@ static void kick_trng(struct platform_device *pdev)
 
 	/* put RNG4 into program mode */
 	setbits32(&r4tst->rtmctl, RTMCTL_PRGM);
-	/* 1600 clocks per sample */
+
+	/*
+	 * Performance-wise, it does not make sense to
+	 * set the delay to a value that is lower
+	 * than the last one that worked (i.e. the state handles
+	 * were instantiated properly. Thus, instead of wasting
+	 * time trying to set the values controlling the sample
+	 * frequency, the function simply returns.
+	 */
+	val = (rd_reg32(&r4tst->rtsdctl) & RTSDCTL_ENT_DLY_MASK)
+	      >> RTSDCTL_ENT_DLY_SHIFT;
+	if (ent_delay <= val) {
+		/* put RNG4 into run mode */
+		clrbits32(&r4tst->rtmctl, RTMCTL_PRGM);
+		return;
+	}
+
 	val = rd_reg32(&r4tst->rtsdctl);
-	val = (val & ~RTSDCTL_ENT_DLY_MASK) | (1600 << RTSDCTL_ENT_DLY_SHIFT);
+	val = (val & ~RTSDCTL_ENT_DLY_MASK) |
+	      (ent_delay << RTSDCTL_ENT_DLY_SHIFT);
 	wr_reg32(&r4tst->rtsdctl, val);
-	/* min. freq. count */
-	wr_reg32(&r4tst->rtfrqmin, 400);
-	/* max. freq. count */
-	wr_reg32(&r4tst->rtfrqmax, 6400);
+	/* min. freq. count, equal to 1/4 of the entropy sample length */
+	wr_reg32(&r4tst->rtfrqmin, ent_delay >> 2);
+	/* max. freq. count, equal to 8 times the entropy sample length */
+	wr_reg32(&r4tst->rtfrqmax, ent_delay << 3);
 	/* put RNG4 into run mode */
 	clrbits32(&r4tst->rtmctl, RTMCTL_PRGM);
 }
@@ -192,7 +219,7 @@ EXPORT_SYMBOL(caam_get_era);
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
-	int ret, ring, rspec;
+	int ret, ring, rspec, ent_delay = RTSDCTL_ENT_DLY_MIN;
 	u64 caam_id;
 	struct device *dev;
 	struct device_node *nprop, *np;
@@ -298,13 +325,17 @@ static int caam_probe(struct platform_device *pdev)
 
 	/*
 	 * If SEC has RNG version >= 4 and RNG state handle has not been
-	 * already instantiated ,do RNG instantiation
+	 * already instantiated, do RNG instantiation
 	 */
 	if ((cha_vid & CHA_ID_RNG_MASK) >> CHA_ID_RNG_SHIFT >= 4 &&
 	    !(rd_reg32(&topregs->ctrl.r4tst[0].rdsta) & RDSTA_IF0)) {
-		kick_trng(pdev);
-		ret = instantiate_rng(dev);
+		do {
+			kick_trng(pdev, ent_delay);
+			ret = instantiate_rng(dev);
+			ent_delay += 400;
+		} while ((ret == -EIO) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
 		if (ret) {
+			dev_err(dev, "failed to instantiate RNG");
 			caam_remove(pdev);
 			return ret;
 		}
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 4455396..9aa9f71 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -255,6 +255,8 @@ struct rng4tst {
 	};
 #define RTSDCTL_ENT_DLY_SHIFT 16
 #define RTSDCTL_ENT_DLY_MASK (0xffff << RTSDCTL_ENT_DLY_SHIFT)
+#define RTSDCTL_ENT_DLY_MIN 1200
+#define RTSDCTL_ENT_DLY_MAX 12800
 	u32 rtsdctl;		/* seed control register */
 	union {
 		u32 rtsblim;	/* PRGM=1: sparse bit limit register */
@@ -706,12 +708,13 @@ struct caam_deco {
 	u32 rsvd29[48];
 	u32 descbuf[64];	/* DxDESB - Descriptor buffer */
 	u32 rscvd30[193];
+#define DESC_DBG_DECO_STAT_HOST_ERR	0x00D00000
+#define DESC_DBG_DECO_STAT_VALID	0x80000000
+#define DESC_DBG_DECO_STAT_MASK		0x00F00000
 	u32 desc_dbg;		/* DxDDR - DECO Debug Register */
 	u32 rsvd31[126];
 };
 
-/* DECO DBG Register Valid Bit*/
-#define DECO_DBG_VALID		0x80000000
 #define DECO_JQCR_WHL		0x20000000
 #define DECO_JQCR_FOUR		0x10000000
 
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH 4/7] crypto: caam - split RNG4 instantiation function
From: Alex Porosanu @ 2013-09-09 15:56 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, horia.geanta, vakul
In-Reply-To: <1378742194-32194-1-git-send-email-alexandru.porosanu@freescale.com>

This patch splits the RNG4 state handle instantiation
function into two parts: one that handles the creation
of the descriptor which instructs the CAAM to perform
the instantiation of the state handle and another
function that performs the running of the said descriptor
using the DECO debug mechanism.

Signed-off-by: Alex Porosanu <alexandru.porosanu@freescale.com>
---
 drivers/crypto/caam/ctrl.c |  140 +++++++++++++++++++++++++++++---------------
 1 files changed, 92 insertions(+), 48 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index d5fe5f5..3ad032f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -13,39 +13,6 @@
 #include "error.h"
 #include "ctrl.h"
 
-static int caam_remove(struct platform_device *pdev)
-{
-	struct device *ctrldev;
-	struct caam_drv_private *ctrlpriv;
-	struct caam_drv_private_jr *jrpriv;
-	struct caam_full __iomem *topregs;
-	int ring, ret = 0;
-
-	ctrldev = &pdev->dev;
-	ctrlpriv = dev_get_drvdata(ctrldev);
-	topregs = (struct caam_full __iomem *)ctrlpriv->ctrl;
-
-	/* shut down JobRs */
-	for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
-		ret |= caam_jr_shutdown(ctrlpriv->jrdev[ring]);
-		jrpriv = dev_get_drvdata(ctrlpriv->jrdev[ring]);
-		irq_dispose_mapping(jrpriv->irq);
-	}
-
-	/* Shut down debug views */
-#ifdef CONFIG_DEBUG_FS
-	debugfs_remove_recursive(ctrlpriv->dfs_root);
-#endif
-
-	/* Unmap controller region */
-	iounmap(&topregs->ctrl);
-
-	kfree(ctrlpriv->jrdev);
-	kfree(ctrlpriv);
-
-	return ret;
-}
-
 /*
  * Descriptor to instantiate RNG State Handle 0 in normal mode and
  * load the JDKEK, TDKEK and TDSK registers
@@ -77,21 +44,23 @@ static void build_instantiation_desc(u32 *desc)
 	append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
 }
 
-static int instantiate_rng(struct device *ctrldev)
+
+/*
+ * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
+ *			  the software (no JR/QI used).
+ * @ctrldev - pointer to device
+ * Return: - 0 if no error occurred
+ *	   - -ENODEV if the DECO couldn't be acquired
+ *	   - -EAGAIN if an error occurred while executing the descriptor
+ */
+static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc)
 {
 	struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev);
 	struct caam_full __iomem *topregs;
 	unsigned int timeout = 100000;
-	u32 *desc, deco_dbg_reg;
+	u32 deco_dbg_reg, flags;
 	int i, ret = 0;
 
-	desc = kmalloc(CAAM_CMD_SZ * 7, GFP_KERNEL | GFP_DMA);
-	if (!desc) {
-		dev_err(ctrldev, "can't allocate RNG init descriptor memory\n");
-		return -ENOMEM;
-	}
-	build_instantiation_desc(desc);
-
 	/* Set the bit to request direct access to DECO0 */
 	topregs = (struct caam_full __iomem *)ctrlpriv->ctrl;
 	setbits32(&topregs->ctrl.deco_rq, DECORR_RQD0ENABLE);
@@ -102,14 +71,23 @@ static int instantiate_rng(struct device *ctrldev)
 
 	if (!timeout) {
 		dev_err(ctrldev, "failed to acquire DECO 0\n");
-		ret = -EIO;
-		goto out;
+		clrbits32(&topregs->ctrl.deco_rq, DECORR_RQD0ENABLE);
+		return -ENODEV;
 	}
 
 	for (i = 0; i < desc_len(desc); i++)
-		topregs->deco.descbuf[i] = *(desc + i);
+		wr_reg32(&topregs->deco.descbuf[i], *(desc + i));
 
-	wr_reg32(&topregs->deco.jr_ctl_hi, DECO_JQCR_WHL | DECO_JQCR_FOUR);
+	flags = DECO_JQCR_WHL;
+	/*
+	 * If the descriptor length is longer than 4 words, then the
+	 * FOUR bit in JRCTRL register must be set.
+	 */
+	if (desc_len(desc) >= 4)
+		flags |= DECO_JQCR_FOUR;
+
+	/* Instruct the DECO to execute it */
+	wr_reg32(&topregs->deco.jr_ctl_hi, flags);
 
 	timeout = 10000000;
 	do {
@@ -129,9 +107,75 @@ static int instantiate_rng(struct device *ctrldev)
 		ret = -EIO;
 	}
 
+	/* Mark the DECO as free */
 	clrbits32(&topregs->ctrl.deco_rq, DECORR_RQD0ENABLE);
-out:
+
+	if (!timeout)
+		return -EAGAIN;
+
+	return 0;
+}
+
+/*
+ * instantiate_rng - builds and executes a descriptor on DECO0,
+ *		     which initializes the RNG block.
+ * @ctrldev - pointer to device
+ * Return: - 0 if no error occurred
+ *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
+ *	   - -ENODEV if DECO0 couldn't be acquired
+ *	   - -EAGAIN if an error occurred when executing the descriptor
+ *	      f.i. there was a RNG hardware error due to not "good enough"
+ *	      entropy being aquired.
+ */
+static int instantiate_rng(struct device *ctrldev)
+{
+	u32 *desc;
+	int ret = 0;
+
+	desc = kmalloc(CAAM_CMD_SZ * 7, GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	/* Create the descriptor for instantiating RNG State Handle 0 */
+	build_instantiation_desc(desc);
+
+	/* Try to run it through DECO0 */
+	ret = run_descriptor_deco0(ctrldev, desc);
+
 	kfree(desc);
+
+	return ret;
+}
+
+static int caam_remove(struct platform_device *pdev)
+{
+	struct device *ctrldev;
+	struct caam_drv_private *ctrlpriv;
+	struct caam_drv_private_jr *jrpriv;
+	struct caam_full __iomem *topregs;
+	int ring, ret = 0;
+
+	ctrldev = &pdev->dev;
+	ctrlpriv = dev_get_drvdata(ctrldev);
+	topregs = (struct caam_full __iomem *)ctrlpriv->ctrl;
+
+	/* shut down JobRs */
+	for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
+		ret |= caam_jr_shutdown(ctrlpriv->jrdev[ring]);
+		jrpriv = dev_get_drvdata(ctrlpriv->jrdev[ring]);
+		irq_dispose_mapping(jrpriv->irq);
+	}
+
+	/* Shut down debug views */
+#ifdef CONFIG_DEBUG_FS
+	debugfs_remove_recursive(ctrlpriv->dfs_root);
+#endif
+
+	/* Unmap controller region */
+	iounmap(&topregs->ctrl);
+
+	kfree(ctrlpriv->jrdev);
+	kfree(ctrlpriv);
+
 	return ret;
 }
 
@@ -333,7 +377,7 @@ static int caam_probe(struct platform_device *pdev)
 			kick_trng(pdev, ent_delay);
 			ret = instantiate_rng(dev);
 			ent_delay += 400;
-		} while ((ret == -EIO) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
+		} while ((ret == -EAGAIN) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
 		if (ret) {
 			dev_err(dev, "failed to instantiate RNG");
 			caam_remove(pdev);
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH 2/7] crypto: caam - fix hash, alg and rng registration if CAAM driver not initialized
From: Alex Porosanu @ 2013-09-09 15:56 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, horia.geanta, vakul
In-Reply-To: <1378742194-32194-1-git-send-email-alexandru.porosanu@freescale.com>

If the CAAM driver initialization failed (due to various reasons, e.g. RNG4
initialization failed), then the registration of hash/algorithms/rng shouldn't
take place. This patch adds the necessary code to prevent this registration.

Signed-off-by: Alex Porosanu <alexandru.porosanu@freescale.com>
---
 drivers/crypto/caam/caamalg.c  |    7 +++++++
 drivers/crypto/caam/caamhash.c |    7 +++++++
 drivers/crypto/caam/caamrng.c  |    7 +++++++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 7c63b72..86a0d41 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -2209,6 +2209,13 @@ static int __init caam_algapi_init(void)
 	priv = dev_get_drvdata(ctrldev);
 	of_node_put(dev_node);
 
+	/*
+	 * If priv is NULL, it's probably because the caam driver wasn't
+	 * properly initialized (e.g. RNG4 init failed). Thus, bail out here.
+	 */
+	if (!priv)
+		return -ENODEV;
+
 	INIT_LIST_HEAD(&priv->alg_list);
 
 	atomic_set(&priv->tfm_count, -1);
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index e732bd9..ca6218e 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1833,6 +1833,13 @@ static int __init caam_algapi_hash_init(void)
 	priv = dev_get_drvdata(ctrldev);
 	of_node_put(dev_node);
 
+	/*
+	 * If priv is NULL, it's probably because the caam driver wasn't
+	 * properly initialized (e.g. RNG4 init failed). Thus, bail out here.
+	 */
+	if (!priv)
+		return -ENODEV;
+
 	INIT_LIST_HEAD(&priv->hash_list);
 
 	atomic_set(&priv->tfm_count, -1);
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index d1939a9..588ad22 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -298,6 +298,13 @@ static int __init caam_rng_init(void)
 	priv = dev_get_drvdata(ctrldev);
 	of_node_put(dev_node);
 
+	/*
+	 * If priv is NULL, it's probably because the caam driver wasn't
+	 * properly initialized (e.g. RNG4 init failed). Thus, bail out here.
+	 */
+	if (!priv)
+		return -ENODEV;
+
 	caam_init_rng(&rng_ctx, priv->jrdev[0]);
 
 	dev_info(priv->jrdev[0], "registering rng-caam\n");
-- 
1.7.7.6

^ permalink raw reply related


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