Hello,
We have found out why this bug happens. When a kernel thread is executing loop_clr_fd(), it will release the loop_ctl_mutex lock for a short period of time, before calling __loop_clr_fd(). However, another kernel thread may take use of this small gap, open the loop device, read it and cause a BLK_STS_IOERR eventually. This bug may lead to error messages on the kernel console, as mentioned in the previous email.
The following interleavings of this bug is shown below:
Thread 1 Thread 2
// Execute loop_clr_fd()
lo->lo_state = Lo_rundown
mutex_unlock(&loop_ctl_mutex);
// Execute lo_open()
err = mutex_lock_killable(&loop_ctl_mutex);
…
lo = bdev->bd_disk->private_data;
// lo_open return a success
// User makes a ksys_read() request
// loop_queue_rq()
if (lo->lo_state != Lo_bound)
return BLK_STS_IOERR;
// Execute __loop_clr_fd()
mutex_lock(&loop_ctl_mutex);
...
Thanks,
Sishuai
> On Sep 28, 2020, at 10:44 AM, Gong, Sishuai <sishuai@xxxxxxxxxx> wrote:
>
> Hi,
>
> We found a potential concurrency bug in linux kernel 5.3.11. We are able to reproduce this bug in x86 under specific thread interleavings. This bug causes a blk_update_request I/O error.
>
> ------------------------------------------
> Kernel console output
> blk_update_request: I/O error, dev loop0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
>
> ------------------------------------------
> Test input
> This bug occurs when kernel functions do_vfs_ioctl() and do_readv() are executed with certain parameters in two separate threads and run concurrently.
>
> The test program is generated in Syzkaller’s format as follows:
> Test 1 [run in thread 1]
> syz_read_part_table(0x0, 0x1, &(0x7f00000006c0)=[{0x0, 0x0, 0x100}])
> Test 2 [run in thread 2]
> r0 = syz_open_dev$loop(&(0x7f0000000000)='/dev/loop#\x00', 0x0, 0x0)
> readv(r0, &(0x7f0000000340)=[{&(0x7f0000000440)=""/4096, 0x1000}], 0x1)
>
> ------------------------------------------
> Interleaving
> Thread 1 Thread 2
> do_readv()
> -vfs_readv()
> --do_iter_read()
> ---do_iter_readv_writev()
> ----blkdev_read_iter()
> do_vfs_ioctl()
> --vfs_ioctl()
> --blkdev_ioctl()
> ---blkdev_driver_ioctl()
> ----loop_set_fd()
> -----bd_set_size()
> (fs/blk_dev.c:1999)
> loff_t size = i_size_read(bd_inode);
> loff_t pos = iocb->ki_pos;
> if (pos >= size)
> return 0;
> size -= pos;
>
> ----generic_file_read_iter()
> (mm/filemap.c:2069)
> page = find_get_page(mapping, index);
> if (!page) {
> if (iocb->ki_flags & IOCB_NOWAIT)
> goto would_block;
> page_cache_sync_readahead(mapping,
>
> -----page_cache_sync_readahead()
> ------ondemand_readahead()
> …
> -----------...blk_update_request()
> (error)
> -----loop_sysfs_init()
> …
>
> ------------------------------------------
> Analysis
> We observed that when thread 2 is executed alone without thread 1, i_size_read() at fs/blk_dev.c:1999 returns a size of 0, thus in sequential mode blkdev_read_iter() returns directly at “return 0;” However, when two threads are executed concurrently, thread 1 changes the size of the same inode that thread 2 is concurrently accessing, then thread 2 goes into a different path, eventually causing the blk_update_request I/O error.
>
>
> Thanks,
> Sishuai
>
