Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode
On Mon, Mar 02, 2015 at 03:37:28PM +0100, Milan Broz wrote: If crypto API allows to encrypt more sectors in one run (handling IV internally) dmcrypt can be modified of course. But do not forget we can use another IV (not only sequential number) e.g. ESSIV with XTS as well (even if it doesn't make much sense, some people are using it). Interesting, I'd not considered using XTS with an IV other than plain/64. The talitos hardware would not support aes/xts in any mode other than plain/plain64 I don't think...Although perhaps you could push in an 8-byte IV and the hardware would interpret it as the sector #. Maybe the following question would be if the dmcrypt sector IV algorithms should moved into crypto API as well. (But because I misused dmcrypt IVs hooks for some additional operations for loopAES and old Truecrypt CBC mode, it is not so simple...) Speaking again with talitos in mind, there would be no advantage for this hardware. Although larger requests are possible only a single IV can be provided per request, so for algorithms like AES-CBC and dm-crypt 512byte IOs are the only option (short of switching to 4kB block size). mh -- Martin Hicks P.Eng.| m...@bork.org Bork Consulting Inc. | +1 (613) 266-2296 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode
On Mon, Mar 02, 2015 at 03:25:56PM +0200, Horia Geantă wrote: On 2/20/2015 7:00 PM, Martin Hicks wrote: This adds the AES-XTS mode, supported by the Freescale SEC 3.3.2. One of the nice things about this hardware is that it knows how to deal with encrypt/decrypt requests that are larger than sector size, but that also requires that that the sector size be passed into the crypto engine as an XTS cipher context parameter. When a request is larger than the sector size the sector number is incremented by the talitos engine and the tweak key is re-calculated for the new sector. I've tested this with 256bit and 512bit keys (tweak and data keys of 128bit and 256bit) to ensure interoperability with the software AES-XTS implementation. All testing was done using dm-crypt/LUKS with aes-xts-plain64. Is there a better solution that just hard coding the sector size to (1SECTOR_SHIFT)? Maybe dm-crypt should be modified to pass the sector size along with the plain/plain64 IV to an XTS algorithm? AFAICT, SW implementation of xts mode in kernel (crypto/xts.c) is not aware of a sector size (data unit size in IEEE P1619 terminology): There's a hidden assumption that all the data send to xts in one request belongs to a single sector. Even more, it's supposed that the first 16-byte block in the request is block 0 in the sector. These can be seen from the way the tweak (T) value is computed. (Side note: there's no support of ciphertext stealing in crypto/xts.c - i.e. sector sizes must be a multiple of underlying block cipher size - that is 16B.) If dm-crypt would be modified to pass sector size somehow, all in-kernel xts implementations would have to be made aware of the change. I have nothing against this, but let's see what crypto maintainers have to say... Right. Additionally, there may be some requirement for the encryption implementation to broadcast the maximum size that can be handled in a single request. For example Talitos could handle XTS encrypt/decrypt requests of up to 64kB (regardless of the block device's sector size). BTW, there were some discussions back in 2013 wrt. being able to configure / increase sector size, smth. crypto engines would benefit from: http://www.saout.de/pipermail/dm-crypt/2013-January/003125.html (experimental patch) http://www.saout.de/pipermail/dm-crypt/2013-March/003202.html The experimental patch sends sector size as the req-nbytes - hidden assumption: data size sent in an xts crypto request equals a sector. I found this last week, and used it as a starting point for some testing. I modified it to keep the underlying sector size of the dm-crypt mapping as 512byte, but allowed the code to combine requests in IOs up to 4kB. Doing greater request sizes would require allocating additional pages...I plan to implement that to see how much extra performance can be squeezed out. patch below... With regards to performance, with my low-powered Freescale P1022 board, I see performance numbers like this on ext4, as measured by bonnie++. Write (MB/s)Read (MB/s) Unencrypted 140 176 aes-xts-plain64 512b113 115 aes-xts-plain64 4kB 71 56 The more detailed bonnie++ output is here: http://www.bork.org/~mort/dm-crypt-enc-blksize.html The larger IO sizes is a huge win for this board. The patch I'm using to send IOs up to 4kB to talitos follows. Thanks, mh diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 08981be..88e95b5 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -42,6 +42,7 @@ struct convert_context { struct bvec_iter iter_out; sector_t cc_sector; atomic_t cc_pending; + unsigned int block_size; struct ablkcipher_request *req; }; @@ -142,6 +143,8 @@ struct crypt_config { sector_t iv_offset; unsigned int iv_size; + unsigned int block_size; + /* ESSIV: struct crypto_cipher *essiv_tfm */ void *iv_private; struct crypto_ablkcipher **tfms; @@ -801,10 +804,17 @@ static void crypt_convert_init(struct crypt_config *cc, { ctx-bio_in = bio_in; ctx-bio_out = bio_out; - if (bio_in) + ctx-block_size = 0; + if (bio_in) { ctx-iter_in = bio_in-bi_iter; - if (bio_out) + ctx-block_size = max(ctx-block_size, bio_cur_bytes(bio_in)); + } + if (bio_out) { ctx-iter_out = bio_out-bi_iter; + ctx-block_size = max(ctx-block_size, bio_cur_bytes(bio_out)); + } + if (ctx-block_size cc-block_size) + ctx-block_size = cc-block_size; ctx-cc_sector = sector + cc-iv_offset; init_completion(ctx-restart); } @@ -844,15 +854,15 @@ static int crypt_convert_block(struct crypt_config *cc, dmreq-iv_sector = ctx-cc_sector; dmreq-ctx = ctx; sg_init_table(dmreq-sg_in, 1); -
Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode
On Mon, Mar 02, 2015 at 04:44:19PM -0500, Martin Hicks wrote: Write (MB/s)Read (MB/s) Unencrypted 140 176 aes-xts-plain64 512b 113 115 aes-xts-plain64 4kB 71 56 I got the two AES lines backwards. Sorry about that. mh -- Martin Hicks P.Eng.| m...@bork.org Bork Consulting Inc. | +1 (613) 266-2296 -- 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 0/3] fix some CAAM warnings.
From: Yanjiang Jin yanjiang@windriver.com Hi, This patch series fix some CAAM compile and runtime warnings. The patches 0001 and 0002 are same as V1. I have tested this on fsl-p5020ds board using upstream 4.0.0-rc1+ with the below configs: CONFIG_DMA_API_DEBUG=y # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set Change log: v2: Abandon the v1 patch 0003-crypto-caamhash-add-two-missed-dma_mapping_error.patch. Replace the v1 patch 0004-crypto-caamhash-replace-kmalloc-with-kzalloc.patch with the new patch 0003-crypto-caamhash-fix-uninitialized-edesc-sec4_sg_byte.patch. Yanjiang Jin (3): crypto: caam: fix some compile warnings crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem crypto: caamhash: - fix uninitialized edesc-sec4_sg_bytes field drivers/crypto/caam/caamhash.c | 1 + drivers/crypto/caam/caamrng.c| 4 ++-- drivers/crypto/caam/sg_sw_sec4.h | 8 3 files changed, 7 insertions(+), 6 deletions(-) -- 1.9.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
Re: [PATCH 4/4] crypto: caamhash: replace kmalloc with kzalloc
On 2015年03月02日 19:03, Horia Geantă wrote: On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote: From: Yanjiang Jin yanjiang@windriver.com This can make sure we get a clean memory, else system would report the below warning: I'd avoid using kzalloc, it's an overhead on the hot path. kmalloc can be used with a bit of attention to detail, i.e. what params to explicitly initialize. Got it. Just zeroing edesc-sec4_sg_bytes in V2. Thanks! Yanjiang I see that the stack trace reports using WR Linux and a modified caam driver - it uses NAPI (net_rx_action) instead of tasklet. Have you actually reproduced the problem on upstream linux? There are some fixes that already address similar (if not exact) problem: 76b99080ccc9 crypto: caam - fix uninitialized edesc-dst_dma fiel 45e9af78b1ab crypto: caam - fix uninitialized S/G table size in ahash_digest Thanks, Horia caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 bytes] [ cut here ] WARNING: at lib/dma-debug.c:877 Modules linked in: CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #175 task: c000f74bc400 ti: c000fffd task.ti: c000f775c000 NIP: c04f5ed8 LR: c04f5ed4 CTR: c055a160 REGS: c000fffd3650 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard) MSR: 80029000 CE,EE,ME CR: 24a48e84 XER: SOFTE: 1 004f5ed4 c000fffd38d0 c12af348 00a0 24a48e84 c125f1c8 01eb 0060 0001 10187373 0020 01eb c1fff780 c11ac928 c0007f003028 0097 0098 0098 c000f7758800 f7098c00 0001 0001 003f f7098c00 0014 c0007f003000 c11b0e98 c1565b80 c000fffd39e0 c000f72f2410 NIP [c04f5ed8] .check_unmap+0x848/0x9c0 LR [c04f5ed4] .check_unmap+0x844/0x9c0 Call Trace: [c000fffd38d0] [c04f5ed4] .check_unmap+0x844/0x9c0 (unreliable) [c000fffd3970] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0 [c000fffd3aa0] [c08295cc] .ahash_done+0x1dc/0x360 [c000fffd3ca0] [c081b7ec] .caam_jr_dequeue+0x26c/0x3a0 [c000fffd3da0] [c08be50c] .net_rx_action+0x1cc/0x330 [c000fffd3e80] [c007276c] .__do_softirq+0x19c/0x3d0 [c000fffd3f90] [c0017054] .call_do_softirq+0x14/0x24 [c000f775ef10] [c0005fe8] .do_softirq+0x118/0x150 sda: sda1 sda2 sda3 [c000f775efa0] [c0072c54] .irq_exit+0x124/0x140 [c000f775f020] [c0005ac4] .do_IRQ+0x184/0x370 [c000f775f0d0] [c001b93c] exc_0x500_common+0xfc/0x100 --- Exception: 501 at .rcu_note_context_switch+0x0/0x370 edule+0xbc/0x7f0 [c000f775f3c0] [c0a29944] .__schedule+0xa4/0x7f0 (unreliable) [c000f775f620] [c0a277f4] .schedule_timeout+0x1b4/0x2e0 [c000f775f700] [c0a29428] .wait_for_common+0xf8/0x1d0 [c000f775f7c0] [c0a295ac] .wait_for_completion_interruptible+0x2c/0x50 [c000f775f840] [c0494b64] .do_one_async_hash_op.isra.1.part.2+0x24/0x50 [c000f775f8c0] [c04951a8] .test_hash+0x618/0x7d0 [c000f775fb30] [c0495424] .alg_test_hash+0xc4/0xf0 [c000f775fbc0] [c0494928] .alg_test+0xa8/0x2c0 [c000f775fcb0] [c0491164] .cryptomgr_test+0x64/0x80 [c000f775fd30] [c009a8d0] .kthread+0xf0/0x100 [c000f775fe30] [ca08] .ret_from_kernel_thread+0x5c/0xd4 Instruction dump: 7c641b78 419e0160 e8a90050 2fa5 409e0008 e8a90010 e8de0028 e8fe0030 3c62ff90 38638320 48546b69 6000 0fe0 4b34 e87e0010 2fa3 ---[ end trace 52825d316d569f00 ]--- Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- -- 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 3/3] crypto: caamhash: - fix uninitialized edesc-sec4_sg_bytes field
From: Yanjiang Jin yanjiang@windriver.com sec4_sg_bytes not being properly initialized causes ahash_done to try to free unallocated DMA memory: caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0xdeadbeefdeadbeef] [size=3735928559 bytes] [ cut here ] WARNING: at lib/dma-debug.c:1093 Modules linked in: CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.0.0-rc1-WR6.0.0.0_standard+ #6 task: e9598c00 ti: effca000 task.ti: e95a2000 NIP: c04ef24c LR: c04ef24c CTR: c0549730 REGS: effcbd40 TRAP: 0700 Not tainted (4.0.0-rc1-WR6.0.0.0_standard+) MSR: 00029002 CE,EE,ME CR: 22008084 XER: 2000 GPR00: c04ef24c effcbdf0 e9598c00 0096 c08f7424 c00ab2b0 0001 GPR08: c0fe7510 effca000 01c3 22008082 c1048e77 c105 GPR16: c0c36700 493c0040 002c e690e4a0 c1054fb4 c18bac40 00029002 c18b0788 GPR24: 0014 e690e480 effcbe48 c0fde128 e6ffac10 deadbeef deadbeef NIP [c04ef24c] check_unmap+0x93c/0xb40 LR [c04ef24c] check_unmap+0x93c/0xb40 Call Trace: [effcbdf0] [c04ef24c] check_unmap+0x93c/0xb40 (unreliable) [effcbe40] [c04ef4f4] debug_dma_unmap_page+0xa4/0xc0 [effcbec0] [c070cda8] ahash_done+0x128/0x1a0 [effcbef0] [c0700070] caam_jr_dequeue+0x1d0/0x290 [effcbf40] [c0045f40] tasklet_action+0x110/0x1f0 [effcbf80] [c0044bc8] __do_softirq+0x188/0x700 [effcbfe0] [c00455d8] irq_exit+0x108/0x120 [effcbff0] [c000f520] call_do_irq+0x24/0x3c [e95a3e20] [c00059b8] do_IRQ+0xc8/0x170 [e95a3e50] [c0011bc8] ret_from_except+0x0/0x18 --- interrupt: 501 at arch_cpu_idle+0x30/0xa0 LR = arch_cpu_idle+0x30/0xa0 [e95a3f10] [c009a5c8] trace_hardirqs_off_caller+0xf8/0x1d0 (unreliable) [e95a3f20] [c0094810] cpu_startup_entry+0x220/0x4b0 [e95a3f90] [c00140f8] start_secondary+0x3a8/0x4e0 [e95a3ff0] [c0001c88] __secondary_start+0x7c/0xc8 Instruction dump: 41de01c8 80a9002c 2f85 40fe0008 80a90008 80fa0018 3c60c0ae 811a001c 38637b8c 813a0020 815a0024 4840ef15 0fe0 8134004c 2f89 40feff48 ---[ end trace 52fb050c6682b55a ]--- Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- Change log: v2: Abandon the v1 patch 0004-crypto-caamhash-replace-kmalloc-with-kzalloc.patch. Just zeroing edesc-sec4_sg_bytes. drivers/crypto/caam/caamhash.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c index f347ab7..ba0532e 100644 --- a/drivers/crypto/caam/caamhash.c +++ b/drivers/crypto/caam/caamhash.c @@ -1172,6 +1172,7 @@ static int ahash_final_no_ctx(struct ahash_request *req) return -ENOMEM; } + edesc-sec4_sg_bytes = 0; sh_len = desc_len(sh_desc); desc = edesc-hw_desc; init_job_desc_shared(desc, ptr, sh_len, HDR_SHARE_DEFER | HDR_REVERSE); -- 1.9.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 1/3] crypto: caam: fix some compile warnings
From: Yanjiang Jin yanjiang@windriver.com This commit is to avoid the below warnings: drivers/crypto/caam/sg_sw_sec4.h:88:12: warning: 'dma_map_sg_chained' defined but not used [-Wunused-function] static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, ^ drivers/crypto/caam/sg_sw_sec4.h:104:12: warning: 'dma_unmap_sg_chained' defined but not used [-Wunused-function] static int dma_unmap_sg_chained(struct device *dev, ^ Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- V2: no change. drivers/crypto/caam/sg_sw_sec4.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h index 3b91821..a6276eb 100644 --- a/drivers/crypto/caam/sg_sw_sec4.h +++ b/drivers/crypto/caam/sg_sw_sec4.h @@ -85,7 +85,7 @@ static inline int sg_count(struct scatterlist *sg_list, int nbytes, return sg_nents; } -static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, +static inline int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, unsigned int nents, enum dma_data_direction dir, bool chained) { @@ -101,9 +101,9 @@ static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, return nents; } -static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg, - unsigned int nents, enum dma_data_direction dir, - bool chained) +static inline int dma_unmap_sg_chained(struct device *dev, + struct scatterlist *sg, unsigned int nents, + enum dma_data_direction dir, bool chained) { if (unlikely(chained)) { int i; -- 1.9.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 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem
From: Yanjiang Jin yanjiang@windriver.com Fix rng_unmap_ctx's DMA_UNMAP size problem for caam_rng, else system would report the below calltrace during kexec boot: caam_jr ffe301000.jr: DMA-API: device driver frees DMA memory with different size [device address=0x7f080010] [map size=16 bytes] [unmap size=40 bytes] [ cut here ] WARNING: at lib/dma-debug.c:887 Modules linked in: CPU: 1 PID: 730 Comm: kexec Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #188 task: c000f7cdaa80 ti: c000e534 task.ti: c000e534 NIP: c04f5bc8 LR: c04f5bc4 CTR: c05f69b0 REGS: c000e53433c0 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard) MSR: 80029000 CE,EE,ME CR: 24088482 XER: SOFTE: 0 GPR00: c04f5bc4 c000e5343640 c12af360 009f GPR04: 00a0 c0d02070 c00015980660 GPR08: c0cff360 c12da018 GPR12: 01e3 c1fff780 100f 0001 GPR16: 0002 GPR20: 0001 GPR24: 0001 0001 0001 GPR28: c1556b90 c1565b80 c000e5343750 c000f9427480 NIP [c04f5bc8] .check_unmap+0x538/0x9c0 LR [c04f5bc4] .check_unmap+0x534/0x9c0 Call Trace: [c000e5343640] [c04f5bc4] .check_unmap+0x534/0x9c0 (unreliable) [c000e53436e0] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0 [c000e5343810] [c082f9d4] .caam_cleanup+0x1d4/0x240 [c000e53438a0] [c056cc88] .hwrng_unregister+0xd8/0x1c0 [c000e5343930] [c082fa74] .caam_rng_shutdown+0x34/0x60 [c000e53439a0] [c0817354] .caam_remove+0x54/0x420 [c000e5343a70] [c05791ac] .platform_drv_shutdown+0x3c/0x60 [c000e5343af0] [c0573728] .device_shutdown+0x128/0x240 [c000e5343b90] [c00880b4] .kernel_restart_prepare+0x54/0x70 [c000e5343c10] [c00e5cac] .kernel_kexec+0x9c/0xd0 [c000e5343c90] [c0088404] .SyS_reboot+0x244/0x2d0 [c000e5343e30] [c718] syscall_exit+0x0/0x8c Instruction dump: 7c641b78 41de0410 e8a90050 2fa5 419e0484 e8de0028 e8ff0030 3c62ff90 e91e0030 38638388 48546ed9 6000 0fe0 3c62ff8f 38637fc8 48546ec5 ---[ end trace e43fd1734d6600df ]--- Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- V2: no change. drivers/crypto/caam/caamrng.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c index ae31e55..314f73d 100644 --- a/drivers/crypto/caam/caamrng.c +++ b/drivers/crypto/caam/caamrng.c @@ -90,8 +90,8 @@ static inline void rng_unmap_ctx(struct caam_rng_ctx *ctx) struct device *jrdev = ctx-jrdev; if (ctx-sh_desc_dma) - dma_unmap_single(jrdev, ctx-sh_desc_dma, DESC_RNG_LEN, -DMA_TO_DEVICE); + dma_unmap_single(jrdev, ctx-sh_desc_dma, + desc_bytes(ctx-sh_desc), DMA_TO_DEVICE); rng_unmap_buf(jrdev, ctx-bufs[0]); rng_unmap_buf(jrdev, ctx-bufs[1]); } -- 1.9.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
Re: [PATCH 3/4] crypto: caamhash: add two missed dma_mapping_error
On 2015年03月02日 19:53, Horia Geantă wrote: On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote: From: Yanjiang Jin yanjiang@windriver.com Add two missed dma_mapping_error() after dma_map_single(). Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- drivers/crypto/caam/caamhash.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c index f347ab7..f6ad322 100644 --- a/drivers/crypto/caam/caamhash.c +++ b/drivers/crypto/caam/caamhash.c @@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, struct device *jrdev, dma_addr_t dst_dma; dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE); + if (dma_mapping_error(jrdev, dst_dma)) { + dev_err(jrdev, unable to map dst dma\n); + return -ENOMEM; + } append_seq_out_ptr(desc, dst_dma, digestsize, 0); return dst_dma; Value returned by map_seq_out_ptr_result() - dst_dma - is always fed to dma_mapping_error(). Note that using an invalid dst_dma in append_seq_out_ptr() doesn't break anything, so it's ok to check dst_dma later. @@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device *jrdev, dma_addr_t buf_dma; buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE); + if (dma_mapping_error(jrdev, buf_dma)) { + dev_err(jrdev, unable to map buf dma\n); + return 0; + } dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0); return buf_dma; These functions are expected to return dma_addr_t, not an error code. If dma_mapping_error() is needed within their scope, the return type will have to change. And return value will need to be checked by their callers. System run well without the above changes, so abandoned this patch in V2. Will do more test in the future. Thanks! Yanjiang -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode
On 03/02/2015 02:25 PM, Horia Geantă wrote: On 2/20/2015 7:00 PM, Martin Hicks wrote: This adds the AES-XTS mode, supported by the Freescale SEC 3.3.2. One of the nice things about this hardware is that it knows how to deal with encrypt/decrypt requests that are larger than sector size, but that also requires that that the sector size be passed into the crypto engine as an XTS cipher context parameter. When a request is larger than the sector size the sector number is incremented by the talitos engine and the tweak key is re-calculated for the new sector. I've tested this with 256bit and 512bit keys (tweak and data keys of 128bit and 256bit) to ensure interoperability with the software AES-XTS implementation. All testing was done using dm-crypt/LUKS with aes-xts-plain64. Is there a better solution that just hard coding the sector size to (1SECTOR_SHIFT)? Maybe dm-crypt should be modified to pass the sector size along with the plain/plain64 IV to an XTS algorithm? AFAICT, SW implementation of xts mode in kernel (crypto/xts.c) is not aware of a sector size (data unit size in IEEE P1619 terminology): There's a hidden assumption that all the data send to xts in one request belongs to a single sector. Even more, it's supposed that the first 16-byte block in the request is block 0 in the sector. These can be seen from the way the tweak (T) value is computed. (Side note: there's no support of ciphertext stealing in crypto/xts.c - i.e. sector sizes must be a multiple of underlying block cipher size - that is 16B.) If dm-crypt would be modified to pass sector size somehow, all in-kernel xts implementations would have to be made aware of the change. I have nothing against this, but let's see what crypto maintainers have to say... BTW, there were some discussions back in 2013 wrt. being able to configure / increase sector size, smth. crypto engines would benefit from: http://www.saout.de/pipermail/dm-crypt/2013-January/003125.html (experimental patch) http://www.saout.de/pipermail/dm-crypt/2013-March/003202.html The experimental patch sends sector size as the req-nbytes - hidden assumption: data size sent in an xts crypto request equals a sector. There was no follow-up but the idea is not yet abandoned :-) Dmcrypt will always use sector as a minimal unit (and I believe sectors will by always multiple of block size so no need for ciphertext stealing in XTS.) For now, dmcrypt always use 512 bytes sector size. If crypto API allows to encrypt more sectors in one run (handling IV internally) dmcrypt can be modified of course. But do not forget we can use another IV (not only sequential number) e.g. ESSIV with XTS as well (even if it doesn't make much sense, some people are using it). Maybe the following question would be if the dmcrypt sector IV algorithms should moved into crypto API as well. (But because I misused dmcrypt IVs hooks for some additional operations for loopAES and old Truecrypt CBC mode, it is not so simple...) Milan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] crypto: caam: fix some compile warnings
On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote: From: Yanjiang Jin yanjiang@windriver.com This commit is to avoid the below warnings: drivers/crypto/caam/sg_sw_sec4.h:88:12: warning: 'dma_map_sg_chained' defined but not used [-Wunused-function] static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, ^ drivers/crypto/caam/sg_sw_sec4.h:104:12: warning: 'dma_unmap_sg_chained' defined but not used [-Wunused-function] static int dma_unmap_sg_chained(struct device *dev, ^ Signed-off-by: Yanjiang Jin yanjiang@windriver.com Reviewed-by: Horia Geanta horia.gea...@freescale.com --- drivers/crypto/caam/sg_sw_sec4.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h index 3b91821..a6276eb 100644 --- a/drivers/crypto/caam/sg_sw_sec4.h +++ b/drivers/crypto/caam/sg_sw_sec4.h @@ -85,7 +85,7 @@ static inline int sg_count(struct scatterlist *sg_list, int nbytes, return sg_nents; } -static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, +static inline int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, unsigned int nents, enum dma_data_direction dir, bool chained) { @@ -101,9 +101,9 @@ static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, return nents; } -static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg, - unsigned int nents, enum dma_data_direction dir, - bool chained) +static inline int dma_unmap_sg_chained(struct device *dev, + struct scatterlist *sg, unsigned int nents, + enum dma_data_direction dir, bool chained) { if (unlikely(chained)) { int i; -- 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 2/4] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem
On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote: From: Yanjiang Jin yanjiang@windriver.com Fix rng_unmap_ctx's DMA_UNMAP size problem for caam_rng, else system would report the below calltrace during kexec boot: caam_jr ffe301000.jr: DMA-API: device driver frees DMA memory with different size [device address=0x7f080010] [map size=16 bytes] [unmap size=40 bytes] [ cut here ] WARNING: at lib/dma-debug.c:887 Modules linked in: CPU: 1 PID: 730 Comm: kexec Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #188 task: c000f7cdaa80 ti: c000e534 task.ti: c000e534 NIP: c04f5bc8 LR: c04f5bc4 CTR: c05f69b0 REGS: c000e53433c0 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard) MSR: 80029000 CE,EE,ME CR: 24088482 XER: SOFTE: 0 GPR00: c04f5bc4 c000e5343640 c12af360 009f GPR04: 00a0 c0d02070 c00015980660 GPR08: c0cff360 c12da018 GPR12: 01e3 c1fff780 100f 0001 GPR16: 0002 GPR20: 0001 GPR24: 0001 0001 0001 GPR28: c1556b90 c1565b80 c000e5343750 c000f9427480 NIP [c04f5bc8] .check_unmap+0x538/0x9c0 LR [c04f5bc4] .check_unmap+0x534/0x9c0 Call Trace: [c000e5343640] [c04f5bc4] .check_unmap+0x534/0x9c0 (unreliable) [c000e53436e0] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0 [c000e5343810] [c082f9d4] .caam_cleanup+0x1d4/0x240 [c000e53438a0] [c056cc88] .hwrng_unregister+0xd8/0x1c0 [c000e5343930] [c082fa74] .caam_rng_shutdown+0x34/0x60 [c000e53439a0] [c0817354] .caam_remove+0x54/0x420 [c000e5343a70] [c05791ac] .platform_drv_shutdown+0x3c/0x60 [c000e5343af0] [c0573728] .device_shutdown+0x128/0x240 [c000e5343b90] [c00880b4] .kernel_restart_prepare+0x54/0x70 [c000e5343c10] [c00e5cac] .kernel_kexec+0x9c/0xd0 [c000e5343c90] [c0088404] .SyS_reboot+0x244/0x2d0 [c000e5343e30] [c718] syscall_exit+0x0/0x8c Instruction dump: 7c641b78 41de0410 e8a90050 2fa5 419e0484 e8de0028 e8ff0030 3c62ff90 e91e0030 38638388 48546ed9 6000 0fe0 3c62ff8f 38637fc8 48546ec5 ---[ end trace e43fd1734d6600df ]--- Signed-off-by: Yanjiang Jin yanjiang@windriver.com Reviewed-by: Horia Geanta horia.gea...@freescale.com --- drivers/crypto/caam/caamrng.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c index ae31e55..314f73d 100644 --- a/drivers/crypto/caam/caamrng.c +++ b/drivers/crypto/caam/caamrng.c @@ -90,8 +90,8 @@ static inline void rng_unmap_ctx(struct caam_rng_ctx *ctx) struct device *jrdev = ctx-jrdev; if (ctx-sh_desc_dma) - dma_unmap_single(jrdev, ctx-sh_desc_dma, DESC_RNG_LEN, - DMA_TO_DEVICE); + dma_unmap_single(jrdev, ctx-sh_desc_dma, + desc_bytes(ctx-sh_desc), DMA_TO_DEVICE); rng_unmap_buf(jrdev, ctx-bufs[0]); rng_unmap_buf(jrdev, ctx-bufs[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
Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode
On 2/20/2015 7:00 PM, Martin Hicks wrote: This adds the AES-XTS mode, supported by the Freescale SEC 3.3.2. One of the nice things about this hardware is that it knows how to deal with encrypt/decrypt requests that are larger than sector size, but that also requires that that the sector size be passed into the crypto engine as an XTS cipher context parameter. When a request is larger than the sector size the sector number is incremented by the talitos engine and the tweak key is re-calculated for the new sector. I've tested this with 256bit and 512bit keys (tweak and data keys of 128bit and 256bit) to ensure interoperability with the software AES-XTS implementation. All testing was done using dm-crypt/LUKS with aes-xts-plain64. Is there a better solution that just hard coding the sector size to (1SECTOR_SHIFT)? Maybe dm-crypt should be modified to pass the sector size along with the plain/plain64 IV to an XTS algorithm? AFAICT, SW implementation of xts mode in kernel (crypto/xts.c) is not aware of a sector size (data unit size in IEEE P1619 terminology): There's a hidden assumption that all the data send to xts in one request belongs to a single sector. Even more, it's supposed that the first 16-byte block in the request is block 0 in the sector. These can be seen from the way the tweak (T) value is computed. (Side note: there's no support of ciphertext stealing in crypto/xts.c - i.e. sector sizes must be a multiple of underlying block cipher size - that is 16B.) If dm-crypt would be modified to pass sector size somehow, all in-kernel xts implementations would have to be made aware of the change. I have nothing against this, but let's see what crypto maintainers have to say... BTW, there were some discussions back in 2013 wrt. being able to configure / increase sector size, smth. crypto engines would benefit from: http://www.saout.de/pipermail/dm-crypt/2013-January/003125.html (experimental patch) http://www.saout.de/pipermail/dm-crypt/2013-March/003202.html The experimental patch sends sector size as the req-nbytes - hidden assumption: data size sent in an xts crypto request equals a sector. I am not sure if there was a follow-up though... Adding Milan - maybe he could shed some light. Thanks, Horia Martin Hicks (2): crypto: talitos: Clean ups and comment fixes for ablkcipher commands crypto: talitos: Add AES-XTS Support drivers/crypto/talitos.c | 45 + drivers/crypto/talitos.h |1 + 2 files changed, 38 insertions(+), 8 deletions(-) -- 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/4] crypto: caamhash: add two missed dma_mapping_error
On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote: From: Yanjiang Jin yanjiang@windriver.com Add two missed dma_mapping_error() after dma_map_single(). Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- drivers/crypto/caam/caamhash.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c index f347ab7..f6ad322 100644 --- a/drivers/crypto/caam/caamhash.c +++ b/drivers/crypto/caam/caamhash.c @@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, struct device *jrdev, dma_addr_t dst_dma; dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE); + if (dma_mapping_error(jrdev, dst_dma)) { + dev_err(jrdev, unable to map dst dma\n); + return -ENOMEM; + } append_seq_out_ptr(desc, dst_dma, digestsize, 0); return dst_dma; Value returned by map_seq_out_ptr_result() - dst_dma - is always fed to dma_mapping_error(). Note that using an invalid dst_dma in append_seq_out_ptr() doesn't break anything, so it's ok to check dst_dma later. @@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device *jrdev, dma_addr_t buf_dma; buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE); + if (dma_mapping_error(jrdev, buf_dma)) { + dev_err(jrdev, unable to map buf dma\n); + return 0; + } dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0); return buf_dma; These functions are expected to return dma_addr_t, not an error code. If dma_mapping_error() is needed within their scope, the return type will have to change. And return value will need to be checked by their callers. -- 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 4/4] crypto: caamhash: replace kmalloc with kzalloc
On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote: From: Yanjiang Jin yanjiang@windriver.com This can make sure we get a clean memory, else system would report the below warning: I'd avoid using kzalloc, it's an overhead on the hot path. kmalloc can be used with a bit of attention to detail, i.e. what params to explicitly initialize. I see that the stack trace reports using WR Linux and a modified caam driver - it uses NAPI (net_rx_action) instead of tasklet. Have you actually reproduced the problem on upstream linux? There are some fixes that already address similar (if not exact) problem: 76b99080ccc9 crypto: caam - fix uninitialized edesc-dst_dma fiel 45e9af78b1ab crypto: caam - fix uninitialized S/G table size in ahash_digest Thanks, Horia caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 bytes] [ cut here ] WARNING: at lib/dma-debug.c:877 Modules linked in: CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #175 task: c000f74bc400 ti: c000fffd task.ti: c000f775c000 NIP: c04f5ed8 LR: c04f5ed4 CTR: c055a160 REGS: c000fffd3650 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard) MSR: 80029000 CE,EE,ME CR: 24a48e84 XER: SOFTE: 1 004f5ed4 c000fffd38d0 c12af348 00a0 24a48e84 c125f1c8 01eb 0060 0001 10187373 0020 01eb c1fff780 c11ac928 c0007f003028 0097 0098 0098 c000f7758800 f7098c00 0001 0001 003f f7098c00 0014 c0007f003000 c11b0e98 c1565b80 c000fffd39e0 c000f72f2410 NIP [c04f5ed8] .check_unmap+0x848/0x9c0 LR [c04f5ed4] .check_unmap+0x844/0x9c0 Call Trace: [c000fffd38d0] [c04f5ed4] .check_unmap+0x844/0x9c0 (unreliable) [c000fffd3970] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0 [c000fffd3aa0] [c08295cc] .ahash_done+0x1dc/0x360 [c000fffd3ca0] [c081b7ec] .caam_jr_dequeue+0x26c/0x3a0 [c000fffd3da0] [c08be50c] .net_rx_action+0x1cc/0x330 [c000fffd3e80] [c007276c] .__do_softirq+0x19c/0x3d0 [c000fffd3f90] [c0017054] .call_do_softirq+0x14/0x24 [c000f775ef10] [c0005fe8] .do_softirq+0x118/0x150 sda: sda1 sda2 sda3 [c000f775efa0] [c0072c54] .irq_exit+0x124/0x140 [c000f775f020] [c0005ac4] .do_IRQ+0x184/0x370 [c000f775f0d0] [c001b93c] exc_0x500_common+0xfc/0x100 --- Exception: 501 at .rcu_note_context_switch+0x0/0x370 edule+0xbc/0x7f0 [c000f775f3c0] [c0a29944] .__schedule+0xa4/0x7f0 (unreliable) [c000f775f620] [c0a277f4] .schedule_timeout+0x1b4/0x2e0 [c000f775f700] [c0a29428] .wait_for_common+0xf8/0x1d0 [c000f775f7c0] [c0a295ac] .wait_for_completion_interruptible+0x2c/0x50 [c000f775f840] [c0494b64] .do_one_async_hash_op.isra.1.part.2+0x24/0x50 [c000f775f8c0] [c04951a8] .test_hash+0x618/0x7d0 [c000f775fb30] [c0495424] .alg_test_hash+0xc4/0xf0 [c000f775fbc0] [c0494928] .alg_test+0xa8/0x2c0 [c000f775fcb0] [c0491164] .cryptomgr_test+0x64/0x80 [c000f775fd30] [c009a8d0] .kthread+0xf0/0x100 [c000f775fe30] [ca08] .ret_from_kernel_thread+0x5c/0xd4 Instruction dump: 7c641b78 419e0160 e8a90050 2fa5 409e0008 e8a90010 e8de0028 e8fe0030 3c62ff90 38638320 48546b69 6000 0fe0 4b34 e87e0010 2fa3 ---[ end trace 52825d316d569f00 ]--- Signed-off-by: Yanjiang Jin yanjiang@windriver.com --- -- 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