Re: [PATCH v2] Revert "block: Add warning for bi_next not NULL in bio_endio()"

2018-06-19 Thread Jens Axboe
On 6/19/18 12:17 PM, Bart Van Assche wrote:
> On Tue, 2018-06-19 at 14:16 -0400, Kent Overstreet wrote:
>> I take it if we had a test for request based dm in blktests or somewhere that
>> probably would have caught this much easier :/
> 
> I'm working on porting the srp-test software to the blktests framework.

That's awesome, would be a great addition!

-- 
Jens Axboe



Re: [PATCH v2] Revert "block: Add warning for bi_next not NULL in bio_endio()"

2018-06-19 Thread Bart Van Assche
On Tue, 2018-06-19 at 14:16 -0400, Kent Overstreet wrote:
> I take it if we had a test for request based dm in blktests or somewhere that
> probably would have caught this much easier :/

I'm working on porting the srp-test software to the blktests framework.

Bart.




Re: [PATCH v2] Revert "block: Add warning for bi_next not NULL in bio_endio()"

2018-06-19 Thread Jens Axboe
On 6/19/18 11:26 AM, Bart Van Assche wrote:
> Commit 0ba99ca4838b ("block: Add warning for bi_next not NULL in
> bio_endio()") breaks the dm driver. end_clone_bio() detects whether
> or not a bio is the last bio associated with a request by checking
> the .bi_next field. Commit 0ba99ca4838b clears that field before
> end_clone_bio() has had a chance to inspect that field. Hence revert
> commit 0ba99ca4838b.

Applied, thanks Bart.

-- 
Jens Axboe



Re: [PATCH v2] Revert "block: Add warning for bi_next not NULL in bio_endio()"

2018-06-19 Thread Mike Snitzer
On Tue, Jun 19 2018 at  1:26pm -0400,
Bart Van Assche  wrote:

> Commit 0ba99ca4838b ("block: Add warning for bi_next not NULL in
> bio_endio()") breaks the dm driver. end_clone_bio() detects whether
> or not a bio is the last bio associated with a request by checking
> the .bi_next field. Commit 0ba99ca4838b clears that field before
> end_clone_bio() has had a chance to inspect that field. Hence revert
> commit 0ba99ca4838b.
> 
> This patch avoids that KASAN reports the following complaint when
> running the srp-test software (srp-test/run_tests -c -d -r 10 -t 02-mq):
> 
> ==
> BUG: KASAN: use-after-free in bio_advance+0x11b/0x1d0
> Read of size 4 at addr 8801300e06d0 by task ksoftirqd/0/9
> 
> CPU: 0 PID: 9 Comm: ksoftirqd/0 Not tainted 4.18.0-rc1-dbg+ #1
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
> 1.0.0-prebuilt.qemu-project.org 04/01/2014
> Call Trace:
>  dump_stack+0xa4/0xf5
>  print_address_description+0x6f/0x270
>  kasan_report+0x241/0x360
>  __asan_load4+0x78/0x80
>  bio_advance+0x11b/0x1d0
>  blk_update_request+0xa7/0x5b0
>  scsi_end_request+0x56/0x320 [scsi_mod]
>  scsi_io_completion+0x7d6/0xb20 [scsi_mod]
>  scsi_finish_command+0x1c0/0x280 [scsi_mod]
>  scsi_softirq_done+0x19a/0x230 [scsi_mod]
>  blk_mq_complete_request+0x160/0x240
>  scsi_mq_done+0x50/0x1a0 [scsi_mod]
>  srp_recv_done+0x515/0x1330 [ib_srp]
>  __ib_process_cq+0xa0/0xf0 [ib_core]
>  ib_poll_handler+0x38/0xa0 [ib_core]
>  irq_poll_softirq+0xe8/0x1f0
>  __do_softirq+0x128/0x60d
>  run_ksoftirqd+0x3f/0x60
>  smpboot_thread_fn+0x352/0x460
>  kthread+0x1c1/0x1e0
>  ret_from_fork+0x24/0x30
> 
> Allocated by task 1918:
>  save_stack+0x43/0xd0
>  kasan_kmalloc+0xad/0xe0
>  kasan_slab_alloc+0x11/0x20
>  kmem_cache_alloc+0xfe/0x350
>  mempool_alloc_slab+0x15/0x20
>  mempool_alloc+0xfb/0x270
>  bio_alloc_bioset+0x244/0x350
>  submit_bh_wbc+0x9c/0x2f0
>  __block_write_full_page+0x299/0x5a0
>  block_write_full_page+0x16b/0x180
>  blkdev_writepage+0x18/0x20
>  __writepage+0x42/0x80
>  write_cache_pages+0x376/0x8a0
>  generic_writepages+0xbe/0x110
>  blkdev_writepages+0xe/0x10
>  do_writepages+0x9b/0x180
>  __filemap_fdatawrite_range+0x178/0x1c0
>  file_write_and_wait_range+0x59/0xc0
>  blkdev_fsync+0x46/0x80
>  vfs_fsync_range+0x66/0x100
>  do_fsync+0x3d/0x70
>  __x64_sys_fsync+0x21/0x30
>  do_syscall_64+0x77/0x230
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 9:
>  save_stack+0x43/0xd0
>  __kasan_slab_free+0x137/0x190
>  kasan_slab_free+0xe/0x10
>  kmem_cache_free+0xd3/0x380
>  mempool_free_slab+0x17/0x20
>  mempool_free+0x63/0x160
>  bio_free+0x81/0xa0
>  bio_put+0x59/0x60
>  end_bio_bh_io_sync+0x5d/0x70
>  bio_endio+0x1a7/0x360
>  blk_update_request+0xd0/0x5b0
>  end_clone_bio+0xa3/0xd0 [dm_mod]
>  bio_endio+0x1a7/0x360
>  blk_update_request+0xd0/0x5b0
>  scsi_end_request+0x56/0x320 [scsi_mod]
>  scsi_io_completion+0x7d6/0xb20 [scsi_mod]
>  scsi_finish_command+0x1c0/0x280 [scsi_mod]
>  scsi_softirq_done+0x19a/0x230 [scsi_mod]
>  blk_mq_complete_request+0x160/0x240
>  scsi_mq_done+0x50/0x1a0 [scsi_mod]
>  srp_recv_done+0x515/0x1330 [ib_srp]
>  __ib_process_cq+0xa0/0xf0 [ib_core]
>  ib_poll_handler+0x38/0xa0 [ib_core]
>  irq_poll_softirq+0xe8/0x1f0
>  __do_softirq+0x128/0x60d
> 
> The buggy address belongs to the object at 8801300e0640
>  which belongs to the cache bio-0 of size 200
> The buggy address is located 144 bytes inside of
>  200-byte region [8801300e0640, 8801300e0708)
> The buggy address belongs to the page:
> page:ea0004c03800 count:1 mapcount:0 mapping:88015a563a00 index:0x0 
> compound_mapcount: 0
> flags: 0x80008100(slab|head)
> raw: 80008100 dead0100 dead0200 88015a563a00
> raw:  00330033 0001 
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>  8801300e0580: fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc
>  8801300e0600: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
> >8801300e0680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ^
>  8801300e0700: fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  8801300e0780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==
> 
> Fixes: 0ba99ca4838b ("block: Add warning for bi_next not NULL in bio_endio()")
> Signed-off-by: Bart Van Assche 
> Cc: Kent Overstreet 
> Cc: Mike Snitzer 

Acked-by: Mike Snitzer 

Thanks Bart.