Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode

2015-03-02 Thread Martin Hicks

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

2015-03-02 Thread Martin Hicks
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

2015-03-02 Thread Martin Hicks
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.

2015-03-02 Thread yanjiang.jin
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

2015-03-02 Thread yjin


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

2015-03-02 Thread yanjiang.jin
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

2015-03-02 Thread yanjiang.jin
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

2015-03-02 Thread yanjiang.jin
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

2015-03-02 Thread yjin


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

2015-03-02 Thread Milan Broz
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

2015-03-02 Thread Horia Geantă
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

2015-03-02 Thread Horia Geantă
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

2015-03-02 Thread Horia Geantă
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

2015-03-02 Thread Horia Geantă
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

2015-03-02 Thread Horia Geantă
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