| Message ID | 20171006155108.6581-1-romain.izard.pro@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2017-10-06 17:51 GMT+02:00 Romain Izard <romain.izard.pro@gmail.com>: > > Certain cipher modes like CTS expect the IV (req->info) of > ablkcipher_request (or equivalently req->iv of skcipher_request) to > contain the last ciphertext block when the {en,de}crypt operation is done. > > Fix this issue for the Atmel AES hardware engine. The tcrypt test > case for cts(cbc(aes)) is now correctly passed. > > To handle the case of in-place decryption, copy the ciphertext in an > intermediate buffer before decryption. > Unfortunately this does not seem to be enough. The tcrypt module's tests pass, but I encounter more issues. If I run the libkcapi test suite, I end up randomly with the following type of panic: 8< ---------------------------------------------------------------------- Unable to handle kernel paging request at virtual address 7ffffffc pgd = dee9c000 [7ffffffc] *pgd=00000000 Internal error: Oops: 5 [#1] ARM Modules linked in: CPU: 0 PID: 2187 Comm: kcapi Not tainted 4.13.4+ #16 Hardware name: Atmel SAMA5 task: dec7f280 task.stack: dee82000 PC is at memcpy+0x114/0x330 LR is at atmel_aes_transfer_complete+0x64/0xe8 pc : [<c07ee5f4>] lr : [<c05e419c>] psr: 20000013 sp : dee83bcc ip : 00000003 fp : dee83bfc r10: 00000000 r9 : df638940 r8 : df638874 r7 : 00000010 r6 : 00000000 r5 : df638940 r4 : dec68110 r3 : 00004004 r2 : 0000000c r1 : 7ffffffc r0 : df638afc Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c53c7d Table: 3ee9c059 DAC: 00000051 Process kcapi (pid: 2187, stack limit = 0xdee82208) Stack: (0xdee83bcc to 0xdee84000) 3bc0: df638afc dec68110 c05e419c 00000000 00000000 3be0: 00000030 dec68110 df557040 00000030 dee83c3c dee83c00 c05e61cc c05e4144 3c00: 10031000 dec68110 df6388a4 00000030 df6388a4 dec68110 df6388a4 00000030 3c20: 00000030 df638874 df557070 00000000 dee83c6c dee83c40 c05e488c c05e6064 3c40: df6388a4 df638874 dee83c6c dec68110 00000030 00000030 df6388a4 df638874 3c60: dee83c94 dee83c70 c05e4998 c05e471c 00000030 dec68110 df557040 00000000 3c80: df638874 df557070 dee83cd4 dee83c98 c05e6198 c05e48d4 c05e6058 00000000 3ca0: dee83cbc df6388a4 c041ab04 dec68110 00000000 df557040 df638940 ffffff8d 3cc0: a0000013 00000004 dee83d04 dee83cd8 c05e62f8 c05e6064 dee83d3c dee83ce8 3ce0: c01dbe9c dec68110 00000000 df638940 df638940 ffffff8d dee83d2c dee83d08 3d00: c05e4ac8 c05e61f8 df638940 00004000 df638800 df557000 00000020 df638860 3d20: dee83d44 dee83d30 c05e4b50 c05e4a14 df638874 00000400 dee83d54 dee83d48 3d40: c05e4ba0 c05e4af0 dee83d74 dee83d58 c034997c c05e4b90 df638840 df638800 3d60: dec3e4c0 00000000 dee83db4 dee83d78 c0368f70 c0349918 00000000 c03bfab0 3d80: df6388a4 df638afc dfff1bc2 df63898c df638988 df608800 00000040 df701000 3da0: df63898c dee83e28 dee83e1c dee83db8 c0393514 c0368e88 df701000 df608800 3dc0: 00000020 df638afc 0000030c 00000188 df63898c df638800 00000044 014000c0 3de0: df638ae8 00000040 dee83e20 df701000 00000040 dee83e80 c0392d48 df61ff00 3e00: df241200 dee83e80 0004a150 00000000 dee83e6c dee83e20 c0641d8c c0392d54 3e20: 00000000 00000000 00000000 00000000 00000000 dee83ea0 00000000 c0101254 3e40: 00000000 00000000 00000000 dee4f400 00000000 df61ff00 dee4f400 00000000 3e60: dee83efc dee83e70 c0242950 c0641cfc dee83e80 c07edac4 80000013 00000000 3e80: 00000000 00000000 00000040 dee83e98 00000001 c0101254 0004a298 00000040 3ea0: f7f27003 00000055 b6f27000 df644000 00000000 000000f6 c0108ea4 0004a150 3ec0: ffffe000 c021636c dee83ee4 dee83ed8 c021636c c02162d8 dee83efc 00049148 3ee0: 00000000 dee4f400 df61ff00 df557200 dee83fa4 dee83f00 c0243db8 c024285c 3f00: dee83f1c c0d05f40 c0d98a98 014080c0 c0d9ad5c 00000000 00000001 ffffe000 3f20: dee83f20 dee83f20 dee83f28 dee83f28 dee83f30 dee83f30 00000000 00000000 3f40: 00000000 00000000 00000000 00000007 0004a298 00000000 00000040 00000000 3f60: 00000000 00000000 00000000 00000000 00000001 00000006 0000011d b6f2bce8 3f80: 00000000 00000000 000000f6 c0108ea4 dee82000 00000000 00000000 dee83fa8 3fa0: c0108ce0 c0243734 b6f2bce8 00000000 b6f27000 00000001 0004a150 00049188 3fc0: b6f2bce8 00000000 00000000 000000f6 00000000 00000001 000490b8 000490d4 3fe0: bee3d838 bee3d828 b6ee63bc b6e73810 60000010 b6f27000 00000000 00000000 [<c07ee5f4>] (memcpy) from [<c05e419c>] (atmel_aes_transfer_complete+0x64/0xe8) [<c05e419c>] (atmel_aes_transfer_complete) from [<c05e61cc>] (atmel_aes_ctr_transfer+0x174/0x194) [<c05e61cc>] (atmel_aes_ctr_transfer) from [<c05e488c>] (atmel_aes_cpu_transfer+0x17c/0x1b8) [<c05e488c>] (atmel_aes_cpu_transfer) from [<c05e4998>] (atmel_aes_cpu_start+0xd0/0xd4) [<c05e4998>] (atmel_aes_cpu_start) from [<c05e6198>] (atmel_aes_ctr_transfer+0x140/0x194) [<c05e6198>] (atmel_aes_ctr_transfer) from [<c05e62f8>] (atmel_aes_ctr_start+0x10c/0x15c) [<c05e62f8>] (atmel_aes_ctr_start) from [<c05e4ac8>] (atmel_aes_handle_queue+0xc0/0xdc) [<c05e4ac8>] (atmel_aes_handle_queue) from [<c05e4b50>] (atmel_aes_crypt+0x6c/0xa0) [<c05e4b50>] (atmel_aes_crypt) from [<c05e4ba0>] (atmel_aes_ctr_decrypt+0x1c/0x20) [<c05e4ba0>] (atmel_aes_ctr_decrypt) from [<c034997c>] (skcipher_decrypt_ablkcipher+0x70/0x74) [<c034997c>] (skcipher_decrypt_ablkcipher) from [<c0368f70>] (crypto_ccm_decrypt+0xf4/0x13c) [<c0368f70>] (crypto_ccm_decrypt) from [<c0393514>] (aead_recvmsg+0x7cc/0x8ec) [<c0393514>] (aead_recvmsg) from [<c0641d8c>] (sock_read_iter+0x9c/0xcc) [<c0641d8c>] (sock_read_iter) from [<c0242950>] (aio_read.constprop.4+0x100/0x184) [<c0242950>] (aio_read.constprop.4) from [<c0243db8>] (SyS_io_submit+0x690/0x7b0) [<c0243db8>] (SyS_io_submit) from [<c0108ce0>] (ret_fast_syscall+0x0/0x3c) Code: e211c003 0affffc4 e3c11003 e35c0002 (e491e004) ---[ end trace c5d62710d279e751 ]--- This looks like a race condition that I introduced to store the encrypted source for the final IV.
Hi Romain, May I ask why you're sending this patch to the MTD ML? While I'm here, can you have a look at this patch [1] and add you Reviewed-by/Tested-by? Thanks, Boris [1]http://patchwork.ozlabs.org/patch/821959/ On Fri, 6 Oct 2017 17:51:08 +0200 Romain Izard <romain.izard.pro@gmail.com> wrote: > Certain cipher modes like CTS expect the IV (req->info) of > ablkcipher_request (or equivalently req->iv of skcipher_request) to > contain the last ciphertext block when the {en,de}crypt operation is done. > > Fix this issue for the Atmel AES hardware engine. The tcrypt test > case for cts(cbc(aes)) is now correctly passed. > > To handle the case of in-place decryption, copy the ciphertext in an > intermediate buffer before decryption. > > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> > --- > drivers/crypto/atmel-aes.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c > index 29e20c37f3a6..f22300babb45 100644 > --- a/drivers/crypto/atmel-aes.c > +++ b/drivers/crypto/atmel-aes.c > @@ -156,6 +156,7 @@ struct atmel_aes_authenc_ctx { > > struct atmel_aes_reqctx { > unsigned long mode; > + u8 *backup_info; > }; > > #ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC > @@ -496,6 +497,12 @@ static void atmel_aes_authenc_complete(struct atmel_aes_dev *dd, int err); > > static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err) > { > + struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq); > + struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req); > + struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req); > + int ivsize = crypto_ablkcipher_ivsize(ablkcipher); > + bool enc = atmel_aes_is_encrypt(dd); > + > #ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC > atmel_aes_authenc_complete(dd, err); > #endif > @@ -503,6 +510,15 @@ static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err) > clk_disable(dd->iclk); > dd->flags &= ~AES_FLAGS_BUSY; > > + if (enc) { > + scatterwalk_map_and_copy(req->info, req->dst, > + req->nbytes - ivsize, ivsize, 0); > + } else if (rctx->backup_info) { > + memcpy(req->info, rctx->backup_info, ivsize); > + kfree(rctx->backup_info); > + rctx->backup_info = NULL; > + } > + > if (dd->is_async) > dd->areq->complete(dd->areq, err); > > @@ -959,13 +975,25 @@ static int atmel_aes_transfer_complete(struct atmel_aes_dev *dd) > static int atmel_aes_start(struct atmel_aes_dev *dd) > { > struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq); > + struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req); > struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req); > + int ivsize = crypto_ablkcipher_ivsize(ablkcipher); > + bool enc = atmel_aes_is_encrypt(dd); > bool use_dma = (req->nbytes >= ATMEL_AES_DMA_THRESHOLD || > dd->ctx->block_size != AES_BLOCK_SIZE); > int err; > > atmel_aes_set_mode(dd, rctx); > > + if (!enc) { > + rctx->backup_info = kzalloc(ivsize, GFP_KERNEL); > + if (rctx->backup_info == NULL) > + return atmel_aes_complete(dd, -ENOMEM); > + > + scatterwalk_map_and_copy(rctx->backup_info, req->src, > + (req->nbytes - ivsize), ivsize, 0); > + } > + > err = atmel_aes_hw_init(dd); > if (err) > return atmel_aes_complete(dd, err);
2017-10-10 16:16 GMT+02:00 Boris Brezillon <boris.brezillon@free-electrons.com>: > Hi Romain, > > May I ask why you're sending this patch to the MTD ML? > This is related to an issue reported by Nicolas Feignon with UBIFS and fscrypt, which was reported on both mtd and fscrypt mailing lists. The file names are encrypted with cts(cbc(aes)), and this patch tried to fix a SAMA5D2 issue where the file names are not correctly encrypted when hardware acceleration is enabled. > While I'm here, can you have a look at this patch [1] and add you > Reviewed-by/Tested-by? > > [1]http://patchwork.ozlabs.org/patch/821959/ I'll try it.
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c index 29e20c37f3a6..f22300babb45 100644 --- a/drivers/crypto/atmel-aes.c +++ b/drivers/crypto/atmel-aes.c @@ -156,6 +156,7 @@ struct atmel_aes_authenc_ctx { struct atmel_aes_reqctx { unsigned long mode; + u8 *backup_info; }; #ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC @@ -496,6 +497,12 @@ static void atmel_aes_authenc_complete(struct atmel_aes_dev *dd, int err); static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err) { + struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq); + struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req); + struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req); + int ivsize = crypto_ablkcipher_ivsize(ablkcipher); + bool enc = atmel_aes_is_encrypt(dd); + #ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC atmel_aes_authenc_complete(dd, err); #endif @@ -503,6 +510,15 @@ static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err) clk_disable(dd->iclk); dd->flags &= ~AES_FLAGS_BUSY; + if (enc) { + scatterwalk_map_and_copy(req->info, req->dst, + req->nbytes - ivsize, ivsize, 0); + } else if (rctx->backup_info) { + memcpy(req->info, rctx->backup_info, ivsize); + kfree(rctx->backup_info); + rctx->backup_info = NULL; + } + if (dd->is_async) dd->areq->complete(dd->areq, err); @@ -959,13 +975,25 @@ static int atmel_aes_transfer_complete(struct atmel_aes_dev *dd) static int atmel_aes_start(struct atmel_aes_dev *dd) { struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq); + struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req); struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req); + int ivsize = crypto_ablkcipher_ivsize(ablkcipher); + bool enc = atmel_aes_is_encrypt(dd); bool use_dma = (req->nbytes >= ATMEL_AES_DMA_THRESHOLD || dd->ctx->block_size != AES_BLOCK_SIZE); int err; atmel_aes_set_mode(dd, rctx); + if (!enc) { + rctx->backup_info = kzalloc(ivsize, GFP_KERNEL); + if (rctx->backup_info == NULL) + return atmel_aes_complete(dd, -ENOMEM); + + scatterwalk_map_and_copy(rctx->backup_info, req->src, + (req->nbytes - ivsize), ivsize, 0); + } + err = atmel_aes_hw_init(dd); if (err) return atmel_aes_complete(dd, err);
Certain cipher modes like CTS expect the IV (req->info) of ablkcipher_request (or equivalently req->iv of skcipher_request) to contain the last ciphertext block when the {en,de}crypt operation is done. Fix this issue for the Atmel AES hardware engine. The tcrypt test case for cts(cbc(aes)) is now correctly passed. To handle the case of in-place decryption, copy the ciphertext in an intermediate buffer before decryption. Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> --- drivers/crypto/atmel-aes.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)