Re: [RFC PATCH v3] crypto: Add IV generation algorithms

[Gilad Ben-Yossef <gilad@benyossef.com>]

[-- Attachment #1: Type: text/plain, Size: 4693 bytes --]

Hi Binoy,


On Wed, Jan 18, 2017 at 11:40 AM, Binoy Jayan <binoy.jayan@linaro.org> wrote:
> Currently, the iv generation algorithms are implemented in dm-crypt.c.
> The goal is to move these algorithms from the dm layer to the kernel
> crypto layer by implementing them as template ciphers so they can be
> implemented in hardware for performance. As part of this patchset, the
> iv-generation code is moved from the dm layer to the crypto layer and
> adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
> at a time. Each bio contains an in memory representation of physically
> contiguous disk blocks. The dm layer sets up a chained scatterlist of
> these blocks split into physically contiguous segments in memory so that
> DMA can be performed. Also, the key management code is moved from dm layer
> to the cryto layer since the key selection for encrypting neighboring
> sectors depend on the keycount.
>
> Synchronous crypto requests to encrypt/decrypt a sector are processed
> sequentially. Asynchronous requests if processed in parallel, are freed
> in the async callback. The dm layer allocates space for iv. The hardware
> implementations can choose to make use of this space to generate their IVs
> sequentially or allocate it on their own.
> Interface to the crypto layer - include/crypto/geniv.h
>
> Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
> ---

I have some review comments and a bug report -

<snip>

>   */
> -static int crypt_convert(struct crypt_config *cc,
> -                        struct convert_context *ctx)
> +
> +static int crypt_convert_bio(struct crypt_config *cc,
> +                            struct convert_context *ctx)
>  {
> +       unsigned int cryptlen, n1, n2, nents, i = 0, bytes = 0;
> +       struct skcipher_request *req;
> +       struct dm_crypt_request *dmreq;
> +       struct geniv_req_info rinfo;
> +       struct bio_vec bv_in, bv_out;
>         int r;
> +       u8 *iv;
>
>         atomic_set(&ctx->cc_pending, 1);
> +       crypt_alloc_req(cc, ctx);
> +
> +       req = ctx->req;
> +       dmreq = dmreq_of_req(cc, req);
> +       iv = iv_of_dmreq(cc, dmreq);
>
> -       while (ctx->iter_in.bi_size && ctx->iter_out.bi_size) {
> +       n1 = bio_segments(ctx->bio_in);
> +       n2 = bio_segments(ctx->bio_in);


I'm pretty sure this needs to be

 n2 = bio_segments(ctx->bio_out);

> +       nents = n1 > n2 ? n1 : n2;
> +       nents = nents > MAX_SG_LIST ? MAX_SG_LIST : nents;
> +       cryptlen = ctx->iter_in.bi_size;
>
> -               crypt_alloc_req(cc, ctx);
> +       DMDEBUG("dm-crypt:%s: segments:[in=%u, out=%u] bi_size=%u\n",
> +               bio_data_dir(ctx->bio_in) == WRITE ? "write" : "read",
> +               n1, n2, cryptlen);
>
<Snip>

>
> -               /* There was an error while processing the request. */
> -               default:
> -                       atomic_dec(&ctx->cc_pending);
> -                       return r;
> -               }
> +               sg_set_page(&dmreq->sg_in[i], bv_in.bv_page, bv_in.bv_len,
> +                           bv_in.bv_offset);
> +               sg_set_page(&dmreq->sg_out[i], bv_out.bv_page, bv_out.bv_len,
> +                           bv_out.bv_offset);
> +
> +               bio_advance_iter(ctx->bio_in, &ctx->iter_in, bv_in.bv_len);
> +               bio_advance_iter(ctx->bio_out, &ctx->iter_out, bv_out.bv_len);
> +
> +               bytes += bv_in.bv_len;
> +               i++;
>         }
>
> -       return 0;
> +       DMDEBUG("dm-crypt: Processed %u of %u bytes\n", bytes, cryptlen);
> +
> +       rinfo.is_write = bio_data_dir(ctx->bio_in) == WRITE;

Please consider wrapping the above boolean expression in parenthesis.


> +       rinfo.iv_sector = ctx->cc_sector;
> +       rinfo.nents = nents;
> +       rinfo.iv = iv;
> +
> +       skcipher_request_set_crypt(req, dmreq->sg_in, dmreq->sg_out,

Also, where do the scatterlist src2 and dst2 that you use
sg_set_page() get sg_init_table() called on?
I couldn't figure it out...

Last but not least, when performing the following sequence on Arm64
(on latest Qemu Virt platform) -

1. cryptsetup luksFormat fs3.img
2. cryptsetup open --type luks fs3.img croot
3. mke2fs /dev/mapper/croot


[ fs3.img is a 16MB file for loopback ]

The attached kernel panic happens. The same does not occur without the patch.

Let me know if you need any additional information to recreate it.

I've tried to debug it a little but did not came up with anything
useful aside from above review notes.

Thanks!


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

[-- Attachment #2: panic.txt --]
[-- Type: text/plain, Size: 8333 bytes --]

# 
# 
# mk
mkdir     mkdosfs   mke2fs    mkfifo    mknod     mkpasswd  mkswap    mktemp
# mk
mkdir     mkdosfs   mke2fs    mkfifo    mknod     mkpasswd  mkswap    mktemp
# mke2fs /dev/mapper/croot 
Filesystem label=
OS type: Linux
Block size=1024 (log=0)
Fragment size=1024 (log=0)
3584 inodes, 14336 blocks
716 blocks (5%) reserved for the super user
First data block=1
Maximum filesystem blocks=262144
2 block groups
8192 blocks per group, 8192 fragments per group
1792 inodes per group
Superblock backups stored on blocks:
	8193
[ 1288.475373] Unable to handle kernel paging request at virtual address 00001028
[ 1288.483568] pgd = ffff0000093ed000
[ 1288.483708] [00001028] *pgd=00000000beffe003, *pud=00000000beffd003, *pmd=0000000000000000
[ 1288.484071] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[ 1288.484266] Modules linked in:
[ 1288.484526] CPU: 0 PID: 16 Comm: kworker/u2:1 Not tainted 4.10.0-rc4-00805-g9a4c309 #32
[ 1288.484712] Hardware name: linux,dummy-virt (DT)
[ 1288.485233] Workqueue: kcryptd kcryptd_crypt
[ 1288.485385] task: ffff80007c4abe80 task.stack: ffff80007c4dc000
[ 1288.485572] PC is at scatterwalk_copychunks+0x144/0x1e8
[ 1288.485750] LR is at scatterwalk_copychunks+0xdc/0x1e8
[ 1288.485904] pc : [<ffff00000834f46c>] lr : [<ffff00000834f404>] pstate: 20000145
[ 1288.486115] sp : ffff80007c4dfa50
[ 1288.486264] x29: ffff80007c4dfa50 x28: 0000000000000001 
[ 1288.486539] x27: ffff80007c4abe80 x26: ffff80007bacab81 
[ 1288.486709] x25: ffff80007c4dfb70 x24: 000000000000000f 
[ 1288.486850] x23: 0000000000000001 x22: 0000000000000001 
[ 1288.486991] x21: 000081ffffffffff x20: ffff80007c4abe80 
[ 1288.487362] x19: ffff80007c4abe80 x18: 0000000000000001 
[ 1288.487544] x17: 00000000004b3768 x16: ffff0000081ec678 
[ 1288.487696] x15: ffffffffffffffff x14: ffff80007bab7c88 
[ 1288.487846] x13: 0000000100000000 x12: 0000000000000000 
[ 1288.487995] x11: 0000000000000010 x10: 0000000000000200 
[ 1288.488178] x9 : 0000000000000000 x8 : 0000000000000000 
[ 1288.488339] x7 : 0000000000000001 x6 : ffff800000040201 
[ 1288.488540] x5 : ffff80007ba85b68 x4 : 0000000000001008 
[ 1288.488690] x3 : 0000000000001008 x2 : 0000000000001008 
[ 1288.488869] x1 : 0000000000000001 x0 : ffff80007ba838a0 
[ 1288.489187] 
[ 1288.489350] Process kworker/u2:1 (pid: 16, stack limit = 0xffff80007c4dc000)
[ 1288.489646] Stack: (0xffff80007c4dfa50 to 0xffff80007c4e0000)
[ 1288.489881] fa40:                                   ffff80007c4dfac0 ffff0000083525d8
[ 1288.490134] fa60: ffff80007c4dfb38 0000000000000000 00000000000001f0 ffff80007ba859f0
[ 1288.490329] fa80: ffff80007babe900 0000000000000000 ffff80007b45adc8 ffff000008cec000
[ 1288.490548] faa0: ffff80007b45ad98 0000000000000200 ffff80007c4dfb38 0000000100000001
[ 1288.490764] fac0: ffff80007c4dfb00 ffff0000080b9774 000000000000000a ffff80007ba85ae8
[ 1288.490962] fae0: 0000000000000000 ffff80007ba859f0 ffff80007babe900 ffff0000080b9764
[ 1288.491155] fb00: ffff80007c4dfbd0 ffff000008391840 ffff80007ba83880 ffff80007babea80
[ 1288.491372] fb20: 0000000000000008 ffff80007b45ad00 ffff80007c4dfb60 00000000ffffffc8
[ 1288.491586] fb40: ffff80007bacab80 ffff80007c4dfba0 ffff80007bacab80 ffff80007ba83880
[ 1288.491798] fb60: ffff80007b45ae90 0000000000000010 ffff80007ba838a0 0000000000000001
[ 1288.491991] fb80: 0000000000000200 0000000000000000 0000000000001000 0000000000000000
[ 1288.492183] fba0: ffff80007bacab80 ffff80007b45ae80 ffff80007b45ae80 ffff800200000010
[ 1288.492394] fbc0: 0000001000000010 ffff800000000007 ffff80007c4dfbf0 ffff0000087969c4
[ 1288.492609] fbe0: ffff80007b45ad80 ffff80007ba83800 ffff80007c4dfc70 ffff000008796688
[ 1288.492836] fc00: 0000000000000000 ffff80007b45ad00 0000000000001000 fffffffffffffff8
[ 1288.493020] fc20: ffff80007b45ac78 ffff80007b45ac30 ffff80007b45ae60 0000000000001000
[ 1288.493213] fc40: 0000000000000001 0000000000000001 ffff000009373f75 ffff80007b45ada8
[ 1288.493482] fc60: 0000000000000001 0000020000000000 ffff80007c4dfd20 ffff000008797480
[ 1288.495928] fc80: ffff80007ba83f00 ffff80007b45ac30 ffff80007b45ac10 ffff80007c408000
[ 1288.496370] fca0: 0000000000000000 ffff80007b45ac10 ffff0000092a7edb ffff80007b45ac00
[ 1288.496762] fcc0: ffff80007c408078 ffff80007c4082a8 0000000000000001 ffff80007ba09800
[ 1288.497182] fce0: ffff00000924b000 ffff0000080e6e10 ffff80007c4dfd80 ffff000000001000
[ 1288.497590] fd00: ffff80007efdf800 0000000000000000 ffff000000000001 ffff80007b45ae80
[ 1288.498014] fd20: ffff80007c4dfdc0 ffff0000080d9d94 0000000000000020 ffff80007c4c5800
[ 1288.498412] fd40: ffff80007ba7dd00 ffff80007c408000 0000000000000000 ffff80007b45ac10
[ 1288.498787] fd60: ffff0000092a7edb ffff80007c408020 ffff80007c408078 ffff80007c4082a8
[ 1288.499147] fd80: ffff80007c4dfde0 ffff000008971800 ffff80007c4abe80 ffff80007c408000
[ 1288.499373] fda0: ffff80007c4c5830 ffff80007c408020 ffff000009287000 ffff80007b45ac30
[ 1288.499571] fdc0: ffff80007c4dfe00 ffff0000080d9f80 ffff80007c4c5800 ffff80007c408000
[ 1288.499766] fde0: ffff80007c4c5830 ffff80007c408020 ffff000009287000 ffff80007c4abe80
[ 1288.499963] fe00: ffff80007c4dfe60 ffff0000080dfe30 ffff80007c4c3b00 ffff80007c4c3680
[ 1288.500159] fe20: ffff00000938fb08 ffff80007c4abe80 ffff000008c50570 ffff80007c4c5800
[ 1288.500373] fe40: ffff0000080d9f38 ffff80007c4c3b38 ffff80007c487d20 0000000000000000
[ 1288.500569] fe60: 0000000000000000 ffff000008082ec0 ffff0000080dfd40 ffff80007c4c3680
[ 1288.500777] fe80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.500975] fea0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.501204] fec0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.501401] fee0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.501621] ff00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.501879] ff20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.502115] ff40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.502329] ff60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.502530] ff80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.502742] ffa0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.502996] ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
[ 1288.503304] ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1288.503761] Call trace:
[ 1288.504274] Exception stack(0xffff80007c4df880 to 0xffff80007c4df9b0)
[ 1288.504572] f880: ffff80007c4abe80 0001000000000000 ffff80007c4dfa50 ffff00000834f46c
[ 1288.504771] f8a0: ffff80007c4df8d0 ffff0000080f410c ffff80007ba09800 ffff80007efdf8e0
[ 1288.504968] f8c0: ffff80007c4df8d0 ffff0000080f4124 ffff80007c4df8e0 ffff0000083ca188
[ 1288.505239] f8e0: ffff80007c4df960 ffff0000083cce80 ffff000009395375 ffff0000089e5510
[ 1288.505439] f900: 0000000000000020 00000000000003e0 00000000fffffff8 ffff000009395328
[ 1288.505693] f920: ffff80007ba838a0 0000000000000001 0000000000001008 0000000000001008
[ 1288.505878] f940: 0000000000001008 ffff80007ba85b68 ffff800000040201 0000000000000001
[ 1288.506150] f960: 0000000000000000 0000000000000000 0000000000000200 0000000000000010
[ 1288.506455] f980: 0000000000000000 0000000100000000 ffff80007bab7c88 ffffffffffffffff
[ 1288.507201] f9a0: ffff0000081ec678 00000000004b3768
[ 1288.507466] [<ffff00000834f46c>] scatterwalk_copychunks+0x144/0x1e8
[ 1288.507627] [<ffff0000083525d8>] skcipher_walk_done+0x250/0x2b0
[ 1288.507781] [<ffff0000080b9774>] xts_decrypt+0x84/0xb0
[ 1288.507913] [<ffff000008391840>] simd_skcipher_decrypt+0x70/0xa8
[ 1288.508088] [<ffff0000087969c4>] geniv_decrypt+0x1b4/0x320
[ 1288.508226] [<ffff000008796688>] crypt_convert_bio+0x790/0x7e0
[ 1288.508368] [<ffff000008797480>] kcryptd_crypt+0x2f0/0x358
[ 1288.508504] [<ffff0000080d9d94>] process_one_work+0x1bc/0x360
[ 1288.508646] [<ffff0000080d9f80>] worker_thread+0x48/0x480
[ 1288.508778] [<ffff0000080dfe30>] kthread+0xf0/0x120
[ 1288.508905] [<ffff000008082ec0>] ret_from_fork+0x10/0x50
[ 1288.509154] Code: d34c7c42 f9400003 927ef463 8b021862 (f9401044) 
[ 1288.509756] ---[ end trace 8de15ab91f16458a ]---
[ 1288.509951] note: kworker/u2:1[16] exited with preempt_count 1