Re: [PATCH] dm-crypt, dm-integrity: allow unaligned bv_offset

2017-11-07 Thread Milan Broz
On 11/07/2017 09:13 AM, Bruno Prémont wrote:
>> Signed-off-by: Mikulas Patocka 
>> Cc: sta...@vger.kernel.org   # v4.12+
>> Fixes: 8f0009a22517 ("dm crypt: optionally support larger encryption sector 
>> size")
>> Fixes: 7eada909bfd7 ("dm: add integrity target")
> 
> Reported-by: Bruno Prémont 
> Tested-by: Bruno Prémont 
> 
> Successfully tested on 4.12.14, 4.13.11 and 4.14-rc8+ (e4880bc5dfb1).

Thanks!

(That alignment check was added after our discussion - we wanted
to have dm-crypt and dm-integrity to check the same thing, just to trigger
possible data corruption, but obviously it was incorrect check.)

It must go to stable well, I think there are possibly more situations
we broke for dm-crypt by this.

Reviewed-by: Milan Broz 

m.


Re: [PATCH] dm-crypt, dm-integrity: allow unaligned bv_offset (was: [Regression, Bisected] dm-crypt IO failures with active slub_debug in 4.12 and later)

2017-11-07 Thread Bruno Prémont
gt; > support larger encryption sector size
> > > > git bisect bad 8f0009a225171cc1b76a6b443de5137b26e1374b
> > > > # first bad commit: [8f0009a225171cc1b76a6b443de5137b26e1374b] dm 
> > > > crypt: optionally support larger encryption sector size
> > > > 
> > > > In order to test on 4.12 I had to revert the following commits,
> > > > the first two having been applied on top of bad commit:
> > > >   583fe7474c05ee5523e14ef791ea59604cd846dc (dm crypt: fix large block 
> > > > integrity support)
> > > >   ff3af92b4461be773337111daea80bb91b2cd545 (dm crypt: use shifts 
> > > > instead of sector_div)
> > > >   8f0009a225171cc1b76a6b443de5137b26e1374b (dm crypt: optionally 
> > > > support larger encryption sector size)
> > > > 
> > > > From looking at 8f0009a225171cc1b76a6b443de5137b26e1374b I can't spot
> > > > direct cause though I assume there might be a mismatch in memory
> > > > allocation versus access sizes as a consequence of support for larger
> > > > encryption sector size (someplace 512 byte versus page size access
> > > > tripping on memory poison?).
> > > 
> > > Thanks for bisecting this.
> > > 
> > > Can you apply the following debug patch to see if either of these newer
> > > -EIO returns (introduced by commit 8f0009a225) are causing you problems
> > > for some reason?
> > > 
> > > Thanks
> > > 
> > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > > index 05acc42..daefe37 100644
> > > --- a/drivers/md/dm-crypt.c
> > > +++ b/drivers/md/dm-crypt.c
> > > @@ -1056,7 +1056,7 @@ static int crypt_convert_block_aead(struct 
> > > crypt_config *cc,
> > >   BUG_ON(cc->integrity_iv_size && cc->integrity_iv_size != cc->iv_size);
> > >  
> > >   /* Reject unexpected unaligned bio. */
> > > - if (unlikely(bv_in.bv_offset & (cc->sector_size - 1)))
> > > + if (WARN_ON_ONCE(unlikely(bv_in.bv_offset & (cc->sector_size - 1)
> > >   return -EIO;
> > >  
> > >   dmreq = dmreq_of_req(cc, req);
> > > @@ -1149,7 +1149,7 @@ static int crypt_convert_block_skcipher(struct 
> > > crypt_config *cc,
> > >   int r = 0;
> > >  
> > >   /* Reject unexpected unaligned bio. */
> > > - if (unlikely(bv_in.bv_offset & (cc->sector_size - 1)))
> > > + if (WARN_ON_ONCE(unlikely(bv_in.bv_offset & (cc->sector_size - 1
> > >   return -EIO;
> > >  
> > >   dmreq = dmreq_of_req(cc, req);  
> > 
> > This second one triggers:
> > [  143.238220] [ cut here ]
> > [  143.238228] WARNING: CPU: 1 PID: 379 at 
> > /usr/src/linux-git/drivers/md/dm-crypt.c:1158 crypt_convert+0xd53/0x1070
> > [  143.238229] Modules linked in:
> > [  143.238233] CPU: 1 PID: 379 Comm: kworker/u17:3 Not tainted 4.12.14+ #2
> > [  143.238234] Hardware name: FUJITSU LIFEBOOK U904/FJNB27D, BIOS Version 
> > 1.09 03/20/2014
> > [  143.238236] Workqueue: kcryptd kcryptd_crypt
> > [  143.238238] task: 982ccdf628c0 task.stack: b3b5c0bb8000
> > [  143.238240] RIP: 0010:crypt_convert+0xd53/0x1070
> > [  143.238242] RSP: 0018:b3b5c0bbbd28 EFLAGS: 00010202
> > [  143.238243] RAX: 01ff RBX: 982ccbebafe0 RCX: 
> > d8d08a2fc880
> > [  143.238244] RDX: 0868 RSI: 982ccbebaf40 RDI: 
> > 
> > [  143.238245] RBP: b3b5c0bbbdd8 R08:  R09: 
> > 0018
> > [  143.238246] R10: b3b5c0073e60 R11: d8d08a2fc880 R12: 
> > 
> > [  143.238247] R13: 982ccbebaf40 R14: 982ccdf5f308 R15: 
> > 982cd00dd288
> > [  143.238248] FS:  () GS:982cde24() 
> > knlGS:
> > [  143.238249] CS:  0010 DS: 0000 ES: 0000 CR0: 000080050033
> > [  143.238251] CR2: 7f45350793c4 CR3: 6b20a000 CR4: 
> > 001406e0
> > [  143.238251] DR0:  DR1:  DR2: 
> > 
> > [  143.238252] DR3:  DR6: fffe0ff0 DR7: 
> > 0400
> > [  143.238253] Call Trace:
> > [  143.238256]  kcryptd_crypt+0x32c/0x3a0
> > [  143.238261]  process_one_work+0x11c/0x340
> > [  143.238264]  worker_thread+0x43/0x3e0
> > [  143.238267]  kthread+0xfe/0x140
> > [  143.238269]  ? create_worker+0x1a0/0x1a0
> > [  143.238271]  ? kthread_create_on_nod

[PATCH] dm-crypt, dm-integrity: allow unaligned bv_offset (was: [Regression, Bisected] dm-crypt IO failures with active slub_debug in 4.12 and later)

2017-11-06 Thread Mikulas Patocka
applied on top of bad commit:
> > >   583fe7474c05ee5523e14ef791ea59604cd846dc (dm crypt: fix large block 
> > > integrity support)
> > >   ff3af92b4461be773337111daea80bb91b2cd545 (dm crypt: use shifts instead 
> > > of sector_div)
> > >   8f0009a225171cc1b76a6b443de5137b26e1374b (dm crypt: optionally support 
> > > larger encryption sector size)
> > > 
> > > From looking at 8f0009a225171cc1b76a6b443de5137b26e1374b I can't spot
> > > direct cause though I assume there might be a mismatch in memory
> > > allocation versus access sizes as a consequence of support for larger
> > > encryption sector size (someplace 512 byte versus page size access
> > > tripping on memory poison?).  
> > 
> > Thanks for bisecting this.
> > 
> > Can you apply the following debug patch to see if either of these newer
> > -EIO returns (introduced by commit 8f0009a225) are causing you problems
> > for some reason?
> > 
> > Thanks
> > 
> > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > index 05acc42..daefe37 100644
> > --- a/drivers/md/dm-crypt.c
> > +++ b/drivers/md/dm-crypt.c
> > @@ -1056,7 +1056,7 @@ static int crypt_convert_block_aead(struct 
> > crypt_config *cc,
> > BUG_ON(cc->integrity_iv_size && cc->integrity_iv_size != cc->iv_size);
> >  
> > /* Reject unexpected unaligned bio. */
> > -   if (unlikely(bv_in.bv_offset & (cc->sector_size - 1)))
> > +   if (WARN_ON_ONCE(unlikely(bv_in.bv_offset & (cc->sector_size - 1)
> > return -EIO;
> >  
> > dmreq = dmreq_of_req(cc, req);
> > @@ -1149,7 +1149,7 @@ static int crypt_convert_block_skcipher(struct 
> > crypt_config *cc,
> > int r = 0;
> >  
> > /* Reject unexpected unaligned bio. */
> > -   if (unlikely(bv_in.bv_offset & (cc->sector_size - 1)))
> > +   if (WARN_ON_ONCE(unlikely(bv_in.bv_offset & (cc->sector_size - 1
> > return -EIO;
> >  
> > dmreq = dmreq_of_req(cc, req);
> 
> This second one triggers:
> [  143.238220] [ cut here ]
> [  143.238228] WARNING: CPU: 1 PID: 379 at 
> /usr/src/linux-git/drivers/md/dm-crypt.c:1158 crypt_convert+0xd53/0x1070
> [  143.238229] Modules linked in:
> [  143.238233] CPU: 1 PID: 379 Comm: kworker/u17:3 Not tainted 4.12.14+ #2
> [  143.238234] Hardware name: FUJITSU LIFEBOOK U904/FJNB27D, BIOS Version 
> 1.09 03/20/2014
> [  143.238236] Workqueue: kcryptd kcryptd_crypt
> [  143.238238] task: 982ccdf628c0 task.stack: b3b5c0bb8000
> [  143.238240] RIP: 0010:crypt_convert+0xd53/0x1070
> [  143.238242] RSP: 0018:b3b5c0bbbd28 EFLAGS: 00010202
> [  143.238243] RAX: 01ff RBX: 982ccbebafe0 RCX: 
> d8d08a2fc880
> [  143.238244] RDX: 0868 RSI: 982ccbebaf40 RDI: 
> 
> [  143.238245] RBP: b3b5c0bbbdd8 R08:  R09: 
> 0018
> [  143.238246] R10: b3b5c0073e60 R11: d8d08a2fc880 R12: 
> 
> [  143.238247] R13: 982ccbebaf40 R14: 982ccdf5f308 R15: 
> 982cd00dd288
> [  143.238248] FS:  () GS:982cde24() 
> knlGS:
> [  143.238249] CS:  0010 DS:  ES:  CR0: 80050033
> [  143.238251] CR2: 7f45350793c4 CR3: 6b20a000 CR4: 
> 001406e0
> [  143.238251] DR0:  DR1:  DR2: 
> 
> [  143.238252] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [  143.238253] Call Trace:
> [  143.238256]  kcryptd_crypt+0x32c/0x3a0
> [  143.238261]  process_one_work+0x11c/0x340
> [  143.238264]  worker_thread+0x43/0x3e0
> [  143.238267]  kthread+0xfe/0x140
> [  143.238269]  ? create_worker+0x1a0/0x1a0
> [  143.238271]  ? kthread_create_on_node+0x40/0x40
> [  143.238275]  ret_from_fork+0x22/0x30
> [  143.238276] Code: ff ff 49 8b 7e 10 be 00 00 40 01 e8 a8 2b 9c ff 49 89 45 
> 70 e9 42 f3 ff ff 48 8b 75 c0 48 8b 7d c8 e8 52 34 c1 ff e9 d2 fa ff ff <0f> 
> ff e9 0f ff ff ff 41 3b 86 d8 00 00 00 0f 84 c2 f3 ff ff 0f
> [  143.238306] ---[ end trace 81f0c82a360b9fb6 ]---
> [  143.238369] XFS (dm-1): metadata I/O error: block 0x2 
> ("xfs_trans_read_buf_map") error 5 numblks 1
> [  143.238373] XFS (dm-1): xfs_do_force_shutdown(0x1) called from line 315 of 
> file /usr/src/linux-git/fs/xfs/xfs_trans_buf.c.  Return address = 
> 0x9734a15c
> [  143.238378] XFS (dm-1): I/O Error Detected. Shutting down filesystem
> [  143.238378] XFS (dm-1): Please umount the filesystem and rectify the 
> problem(s)
> 
&