Re: [PATCH, RFC] random: introduce getrandom(2) system call
Hi, I am wondering if we could improve the design of the system call a bit to prevent programming errors. Right now, EINVAL is returned in case of invalid flags (or in the older version of getrandom() also if buflen is too large), EFAULT if buf is an invalid address and EAGAIN if there is not enough entropy. However, of course no programmer is save against programming errors. Everybody *should* check the return value of syscalls, but sometimes it is forgotten, and theoretically you must be stoned to death for that. Still, we should think about how we could prevent these errors. Here is a list of possible modifications of getrandom() and pros and cons: 1. memset(buf, 0x0, buflen) in case of an error pros: - it is more obvious to the userspace programmer that the content of buffer does *not* contain random bytes cons: - in case even the zero-ed buf is not noticed by the programmer, she/he might end up using a 100% predictable string of random bytes. In contrast if zero-ing the buf is ommitted, you would at least end up using some (not-cryptographically) random bytes from somewhere in RAM. I am aware that this memset() call should theoretically be superfluous but it would only be executed in very rare cases where the programmer misuses getrandom(). 2. int getrandom(void **buf, size_t buflen, unsigned int flags) ^^ If flags, are fine, return a pointer to a buffer of random bytes, otherwise return a pointer to NULL. pros: - it would ensure that an error in a getrandom() call cannot be ignored. cons: - not sure if a syscall should allocate memory in the name of a userspace program - not a very unix-like syscall signature - anytime getrandom() is called, it will allocate a new buffer which might end up in decreased performance (however, getrandom() should not be called multiple times) 3. send a signal to the userland process that (by default) leads to an abnormal termination of the process Essentially an error in getrandom() could be seen as critical as a division by 0. pros: - the userspace programmer is forced to handle this error (otherwise the signal would terminate the program) cons: - adds more complexity to the userspace program that might lead to new programming errors These are three possibilities. Maybe one of you is more creative and can come up with a much better idea. At the moment, I like option 2 the best, because it forces the programmer to deal with these errors, but probably one of you has a good point why this is not a good idea. Handling the NULL pointer would be much easier than using signals (option 3). However, it lead to a syscall signature that is different from, let's say read(), because the syscall itself would allocate its buffer. Again, I am aware that you must always check return values, but programming errors happen. E.g. everybody knows that you cannot trust data that you received via network, yet heartbleed happened. Here we have the chance to eradicate a critical programming error by improving the syscall design and I think we should spend some time thinking about that. Best, Manuel -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] crypto: testmgr - avoid DMA mapping from text, rodata, stack
On 7/23/2014 1:37 AM, Kim Phillips wrote: On Fri, 11 Jul 2014 15:34:46 +0300 Horia Geanta horia.gea...@freescale.com wrote: +++ b/crypto/testmgr.c @@ -198,13 +198,20 @@ static int __test_hash(struct crypto_ahash *tfm, struct hash_testvec *template, const char *algo = crypto_tfm_alg_driver_name(crypto_ahash_tfm(tfm)); unsigned int i, j, k, temp; struct scatterlist sg[8]; -char result[64]; +char *result = NULL; +char *key = NULL; these needn't be initialized, here and elsewhere. I was under the impression that this is needed for kfree on some exit paths, but indeed it's not the case. struct ahash_request *req; struct tcrypt_result tresult; void *hash_buff; char *xbuf[XBUFSIZE]; int ret = -ENOMEM; +result = kmalloc(64, GFP_KERNEL); s/64/MAX_DIGEST_SIZE +++ b/crypto/testmgr.h @@ -32,7 +32,7 @@ #define MAX_DIGEST_SIZE 64 #define MAX_TAP 8 -#define MAX_KEYLEN 56 +#define MAX_KEYLEN 160 #define MAX_IVLEN 32 this change could use a blurb in the commit message. I'll send a v2 only for current patch, if Herbert is ok with this. Other than that, this series gets my: Acked-by: Kim Phillips kim.phill...@freescale.com Thanks! Thanks for reviewing, testing. Horia -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] crypto: caam - fix DECO RSR polling
Acked-by :- Ruchika Gupta ruchika.gu...@freescale.com
Tested on P4080DS.
Ported and tested on LS1 platform also (This platform has the virtualization
enabled).
Thanks,
Ruchika
-Original Message-
From: Horia Geanta [mailto:horia.gea...@freescale.com]
Sent: Monday, July 21, 2014 6:33 PM
To: Herbert Xu; linux-crypto@vger.kernel.org; Gupta Ruchika-R66431; Phillips
Kim-R1AAHA
Cc: David S. Miller
Subject: [PATCH] crypto: caam - fix DECO RSR polling
RSR (Request Source Register) is not used when virtualization is disabled,
thus don't poll for Valid bit.
Besides this, if used, timeout has to be reinitialized.
Signed-off-by: Horia Geanta horia.gea...@freescale.com
---
Only compile-tested.
Ruchika / Kim, please review / test.
drivers/crypto/caam/ctrl.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
c6e9d3b2d502..84d4b95c761e 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -89,12 +89,15 @@ static inline int run_descriptor_deco0(struct device
*ctrldev, u32 *desc,
/* Set the bit to request direct access to DECO0 */
topregs = (struct caam_full __iomem *)ctrlpriv-ctrl;
- if (ctrlpriv-virt_en == 1)
+ if (ctrlpriv-virt_en == 1) {
setbits32(topregs-ctrl.deco_rsr, DECORSR_JR0);
- while (!(rd_reg32(topregs-ctrl.deco_rsr) DECORSR_VALID)
---timeout)
- cpu_relax();
+ while (!(rd_reg32(topregs-ctrl.deco_rsr) DECORSR_VALID)
+--timeout)
+ cpu_relax();
+
+ timeout = 10;
+ }
setbits32(topregs-ctrl.deco_rq, DECORR_RQD0ENABLE);
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/10] crypto: testmgr - avoid DMA mapping from text, rodata, stack
With DMA_API_DEBUG set, following warnings are emitted
(tested on CAAM accelerator):
DMA-API: device driver maps memory from kernel text or rodata
DMA-API: device driver maps memory from stack
and the culprits are:
-key in __test_aead and __test_hash
-result in __test_hash
MAX_KEYLEN is changed to accommodate maximum key length from
existing test vectors in crypto/testmgr.h (131 bytes) and rounded.
Signed-off-by: Horia Geanta horia.gea...@freescale.com
---
v2: Addressed Kim's comments.
crypto/testmgr.c | 57
crypto/testmgr.h | 2 +-
2 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 0f90612a00b9..81818b9a1b83 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -198,13 +198,20 @@ static int __test_hash(struct crypto_ahash *tfm, struct
hash_testvec *template,
const char *algo = crypto_tfm_alg_driver_name(crypto_ahash_tfm(tfm));
unsigned int i, j, k, temp;
struct scatterlist sg[8];
- char result[64];
+ char *result;
+ char *key;
struct ahash_request *req;
struct tcrypt_result tresult;
void *hash_buff;
char *xbuf[XBUFSIZE];
int ret = -ENOMEM;
+ result = kmalloc(MAX_DIGEST_SIZE, GFP_KERNEL);
+ if (!result)
+ return ret;
+ key = kmalloc(MAX_KEYLEN, GFP_KERNEL);
+ if (!key)
+ goto out_nobuf;
if (testmgr_alloc_buf(xbuf))
goto out_nobuf;
@@ -229,7 +236,7 @@ static int __test_hash(struct crypto_ahash *tfm, struct
hash_testvec *template,
goto out;
j++;
- memset(result, 0, 64);
+ memset(result, 0, MAX_DIGEST_SIZE);
hash_buff = xbuf[0];
hash_buff += align_offset;
@@ -239,8 +246,14 @@ static int __test_hash(struct crypto_ahash *tfm, struct
hash_testvec *template,
if (template[i].ksize) {
crypto_ahash_clear_flags(tfm, ~0);
- ret = crypto_ahash_setkey(tfm, template[i].key,
- template[i].ksize);
+ if (template[i].ksize MAX_KEYLEN) {
+ pr_err(alg: hash: setkey failed on test %d for
%s: key size %d %d\n,
+ j, algo, template[i].ksize, MAX_KEYLEN);
+ ret = -EINVAL;
+ goto out;
+ }
+ memcpy(key, template[i].key, template[i].ksize);
+ ret = crypto_ahash_setkey(tfm, key, template[i].ksize);
if (ret) {
printk(KERN_ERR alg: hash: setkey failed on
test %d for %s: ret=%d\n, j, algo,
@@ -300,7 +313,7 @@ static int __test_hash(struct crypto_ahash *tfm, struct
hash_testvec *template,
if (template[i].np) {
j++;
- memset(result, 0, 64);
+ memset(result, 0, MAX_DIGEST_SIZE);
temp = 0;
sg_init_table(sg, template[i].np);
@@ -319,8 +332,16 @@ static int __test_hash(struct crypto_ahash *tfm, struct
hash_testvec *template,
}
if (template[i].ksize) {
+ if (template[i].ksize MAX_KEYLEN) {
+ pr_err(alg: hash: setkey failed on
test %d for %s: key size %d %d\n,
+ j, algo, template[i].ksize,
+ MAX_KEYLEN);
+ ret = -EINVAL;
+ goto out;
+ }
crypto_ahash_clear_flags(tfm, ~0);
- ret = crypto_ahash_setkey(tfm, template[i].key,
+ memcpy(key, template[i].key, template[i].ksize);
+ ret = crypto_ahash_setkey(tfm, key,
template[i].ksize);
if (ret) {
@@ -372,6 +393,8 @@ out:
out_noreq:
testmgr_free_buf(xbuf);
out_nobuf:
+ kfree(key);
+ kfree(result);
return ret;
}
@@ -429,6 +452,9 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
iv = kzalloc(MAX_IVLEN, GFP_KERNEL);
if (!iv)
return ret;
+ key = kmalloc(MAX_KEYLEN, GFP_KERNEL);
+ if (!key)
+ goto out_noxbuf;
if (testmgr_alloc_buf(xbuf))
goto out_noxbuf;
if (testmgr_alloc_buf(axbuf))
@@ -493,7 +519,14 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
crypto_aead_set_flags(
Re: [PATCH, RFC] random: introduce getrandom(2) system call
Hi, On Wed, Jul 23, 2014, at 00:59, Theodore Ts'o wrote: But why would you need to use GRND_RANDOM in your scenario, and accept your application potentially getting stalled and stuck in amber for perhaps hours? If you are going to accept your application stalling like that, you can do the pointer arithmatic. It's really not hard, and someone who can't do that, again, shouldn't be allowd anywhere near crypto code in the first place (and if they are, they'll probably be making lots of other, equally fatal if not more so, newbie mistakes). I favored the idea of having a non-failing non-partial-read getrandom syscall. But I am with you if it often causes long stalls that we should stick to the old semantics. Thanks, Hannes -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/crypto/Kconfig: Let 'DEV_QCE' depend on both HAS_DMA and HAS_IOMEM
Oh, sorry, this patch can still be alive, after a long discussion, we need keep current status no touch -- still let individual modules to depends on HAS_IOMEM and/or HAS_DMA, if they need them. So please help check the patch, when you have time. Thanks. On 07/14/2014 08:12 PM, Chen Gang wrote: Hello all: This patch is obsoleted according to the other discussion thread about IOMEM and COMPILE_TEST. I shall fix it, and maybe not need touch drivers/crypto again, so maybe not need send patch v2 for it. Thanks. On 07/13/2014 11:01 AM, Chen Gang wrote: 'DEV_QCE' needs both HAS_DMA and HAS_IOMEM, so let it depend on them. The related error (with allmodconfig under score): MODPOST 1365 modules ERROR: devm_ioremap_resource [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_map_sg [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_set_mask [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_supported [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_unmap_sg [drivers/crypto/qce/qcrypto.ko] undefined! Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- drivers/crypto/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 5ef9ec9..2fb0fdf 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -422,7 +422,7 @@ source drivers/crypto/qat/Kconfig config CRYPTO_DEV_QCE tristate Qualcomm crypto engine accelerator -depends on ARCH_QCOM || COMPILE_TEST +depends on (ARCH_QCOM || COMPILE_TEST) HAS_DMA HAS_IOMEM select CRYPTO_AES select CRYPTO_DES select CRYPTO_ECB -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC] random: introduce getrandom(2) system call
I keep wishing for a more general solution. For example, some way to have a spare extra fd that could be accessed with a special O_NOFAIL flag. That would allow any number of library functions to not fail, such as logging from nasty corner cases. But you'd have to provide one per thread, and block non-fatal signals while it was open, so you don't get reentrancy problems. Ick. This overly-specialized system call (and worse yet, a blocking system call that you can't put into a poll() loop) just feels ugly to me. Is it *absolutely* necessary? For example, how about simply making getentropy() a library function that aborts if it can't open /dev/urandom? If you're suffering fd exhaustion, you're being DoSed already. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC] random: introduce getrandom(2) system call
On Wed, Jul 23, 2014, at 13:52, George Spelvin wrote: I keep wishing for a more general solution. For example, some way to have a spare extra fd that could be accessed with a special O_NOFAIL flag. That would allow any number of library functions to not fail, such as logging from nasty corner cases. But you'd have to provide one per thread, and block non-fatal signals while it was open, so you don't get reentrancy problems. Ick. This overly-specialized system call (and worse yet, a blocking system call that you can't put into a poll() loop) just feels ugly to me. Is it *absolutely* necessary? One point that often came up besides fd exhaustion is missing /dev/u?random device nodes in chroot environments. I also thought about a more general interface, like e.g. an opennod(dev_t device, int flags) call but all those ideas ended up being very complex changes besides having design issues. getrandom is simple and solves a real problem. The only problem I see, that we allow access to /dev/random without checking any permission bits like we did on opening /dev/random before and we cannot restrict applications to deplete the whole entropy pool. For example, how about simply making getentropy() a library function that aborts if it can't open /dev/urandom? If you're suffering fd exhaustion, you're being DoSed already. Maybe applications want to mitigate fd exhaustion. Bye, Hannes -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: initialize entry len for null input in crypto hash sg list walk
On Thu, Jul 10, 2014 at 04:18:08PM -0700, Tim Chen wrote: For the special case when we have a null input string, we want to initialize the entry len to 0 for the hash/ahash walk, so cyrpto_hash_walk_last will return the correct result indicating that we have completed the scatter list walk. Otherwise we may keep walking the sg list and access bogus memory address. Signed-off-by: Tim Chen tim.c.c...@linux.intel.com Sorry but which driver is broken by this? Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC, prePATCH] crypto: talitos modified for powerpc 88x security engine
On Fri, Jul 11, 2014 at 02:09:39PM +0200, Christophe Leroy wrote: Here is a pre-patch for the support of the SEC ENGINE of MPC88x/MPC82xx I have tried to make use of defines in order to keep a single driver for the two TALITOS variants as suggested by Kim, but I'm not too happy about the quantity of #ifdef Yeah these ifdefs have got to go. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/3] crypto: Add Allwinner Security System crypto accelerator
On Sat, Jul 12, 2014 at 02:59:13PM +0200, LABBE Corentin wrote: Add support for the Security System included in Allwinner SoC A20. The Security System is a hardware cryptographic accelerator that support AES/MD5/SHA1/DES/3DES/PRNG algorithms. Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com This is essentially a synchronous driver, no? If so please switch to the blkcipher/shash interface. Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: ccp - Base AXI DMA cache settings on device tree
On Thu, Jul 10, 2014 at 10:58:35AM -0500, Tom Lendacky wrote: The default cache operations for ARM64 were changed during 3.15. To use coherent operations a dma-coherent device tree property is required. If that property is not present in the device tree node then the non-coherent operations are assigned for the device. Add support to the ccp driver to assign the AXI DMA cache settings based on whether the dma-coherent property is present in the device node. If present, use settings that work with the caches. If not present, use settings that do not look at the caches. Signed-off-by: Tom Lendacky thomas.lenda...@amd.com Patch applied. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] crypto: testmgr - avoid DMA mapping from text, rodata, stack
On Wed, Jul 23, 2014 at 11:59:38AM +0300, Horia Geanta wrote: With DMA_API_DEBUG set, following warnings are emitted (tested on CAAM accelerator): DMA-API: device driver maps memory from kernel text or rodata DMA-API: device driver maps memory from stack and the culprits are: -key in __test_aead and __test_hash -result in __test_hash MAX_KEYLEN is changed to accommodate maximum key length from existing test vectors in crypto/testmgr.h (131 bytes) and rounded. Signed-off-by: Horia Geanta horia.gea...@freescale.com --- v2: Addressed Kim's comments. All applied. Thanks. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/crypto/Kconfig: Let 'DEV_QCE' depend on both HAS_DMA and HAS_IOMEM
On Sun, Jul 13, 2014 at 11:01:38AM +0800, Chen Gang wrote: 'DEV_QCE' needs both HAS_DMA and HAS_IOMEM, so let it depend on them. The related error (with allmodconfig under score): MODPOST 1365 modules ERROR: devm_ioremap_resource [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_map_sg [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_set_mask [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_supported [drivers/crypto/qce/qcrypto.ko] undefined! ERROR: dma_unmap_sg [drivers/crypto/qce/qcrypto.ko] undefined! Signed-off-by: Chen Gang gang.chen.5...@gmail.com Patch applied. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: caam - set DK (Decrypt Key) bit only for AES accelerator
On Fri, Jul 11, 2014 at 03:46:58PM +0300, Horia Geanta wrote: AES currently shares descriptor creation functions with DES and 3DES. DK bit is set in all cases, however it is valid only for the AES accelerator. Signed-off-by: Horia Geanta horia.gea...@freescale.com Patch applied. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: caam - fix DECO RSR polling
On Tue, Jul 22, 2014 at 04:31:56PM -0500, Kim Phillips wrote: On Mon, 21 Jul 2014 16:03:21 +0300 Horia Geanta horia.gea...@freescale.com wrote: RSR (Request Source Register) is not used when virtualization is disabled, thus don't poll for Valid bit. Besides this, if used, timeout has to be reinitialized. Signed-off-by: Horia Geanta horia.gea...@freescale.com --- Only compile-tested. Ruchika / Kim, please review / test. Acked-by: Kim Phillips kim.phill...@freescale.com Patch applied. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/3] crypto: Add Allwinner Security System crypto accelerator
Hi, On Wed, Jul 23, 2014 at 09:16:20PM +0800, Herbert Xu wrote: On Sat, Jul 12, 2014 at 02:59:13PM +0200, LABBE Corentin wrote: Add support for the Security System included in Allwinner SoC A20. The Security System is a hardware cryptographic accelerator that support AES/MD5/SHA1/DES/3DES/PRNG algorithms. Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com This is essentially a synchronous driver, no? If so please switch to the blkcipher/shash interface. The exact opposite has been asked for during v1's review... Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v4 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wed, Jul 23, 2014 at 03:48:57PM +0200, Maxime Ripard wrote: The exact opposite has been asked for during v1's review... Indeed but unfortunately it was bogus advice. The async interface brings with it a lot of complexity which should be avoided unless you actually need it. Even if you use the sync interface your driver will still be available to all async users. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto: ccp - Remove select OF from Kconfig
The addition of the select OF if ARM64 has led to a Kconfig recursive dependency error when make ARCH=sh rsk7269_defconfig was run. Since OF is selected by ARM64 and the of_property_read_bool is defined no matter what, delete the Kconfig line that selects OF. Reported-by: kbuild test robot fengguang...@intel.com Signed-off-by: Tom Lendacky thomas.lenda...@amd.com --- drivers/crypto/ccp/Kconfig |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig index 474382d..7639ffc 100644 --- a/drivers/crypto/ccp/Kconfig +++ b/drivers/crypto/ccp/Kconfig @@ -3,7 +3,6 @@ config CRYPTO_DEV_CCP_DD depends on CRYPTO_DEV_CCP default m select HW_RANDOM - select OF if ARM64 help Provides the interface to use the AMD Cryptographic Coprocessor which can be used to accelerate or offload encryption operations -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: initialize entry len for null input in crypto hash sg list walk
On Wed, 2014-07-23 at 21:09 +0800, Herbert Xu wrote: On Thu, Jul 10, 2014 at 04:18:08PM -0700, Tim Chen wrote: For the special case when we have a null input string, we want to initialize the entry len to 0 for the hash/ahash walk, so cyrpto_hash_walk_last will return the correct result indicating that we have completed the scatter list walk. Otherwise we may keep walking the sg list and access bogus memory address. Signed-off-by: Tim Chen tim.c.c...@linux.intel.com Sorry but which driver is broken by this? I haven't tested other driver, but I see this problem when I was testing the new multi-buffer sha1 driver Tim -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
Le 23/07/2014 17:51, Marek Vasut a écrit : On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: +} +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 +err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut Yes, DMA is possible and will be implemented soon. So if I have well understood, I keep using async interface. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wednesday, July 23, 2014 at 08:52:12 PM, Corentin LABBE wrote: Le 23/07/2014 17:51, Marek Vasut a écrit : On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: + } +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 + err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut Yes, DMA is possible and will be implemented soon. So if I have well understood, I keep using async interface. Yeah, in this case, using DMA and async interface combo is the way to go. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator
On Wed, Jul 23, 2014 at 09:38:35PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 08:52:12 PM, Corentin LABBE wrote: Le 23/07/2014 17:51, Marek Vasut a écrit : On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote: On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote: On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote: On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote: +} +#endif + +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5 +err = crypto_register_shash(sunxi_md5_alg); Do not use shash for such device. This is clearly and ahash (and async in general) device. The rule of a thumb here is that you use sync algos only for devices which have dedicated instructions for computing the transformation. For devices which are attached to some kind of bus, you use async algos (ahash etc). I'm sorry that I didn't catch this earlier but there is no such rule. Unless you need the async interface you should stick to the sync interfaces for the sake of simplicity. We have a number of existing drivers that are synchronous but using the async interface. They should either be converted over to the sync interface or made interrupt-driven if possible. Sure, but this device is interrupt driven and uses DMA to feed the crypto engine, therefore async, right ? If it's interrupt-driven, then yes it would certainly make sense to be async. But all I see is polling in the latest posting, was the first version different? I stand corrected then, sorry. Is it possible to use DMA to feed the crypto accelerator, Corentin? Best regards, Marek Vasut Yes, DMA is possible and will be implemented soon. So if I have well understood, I keep using async interface. Yeah, in this case, using DMA and async interface combo is the way to go. OK fair enough. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
