PROBLEM: potential concurrency bug between do_vfs_ioctl() and do_readv()

PROBLEM: potential concurrency bug between do_vfs_ioctl() and do_readv()

[Gong, Sishuai]

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

Re: PROBLEM: potential concurrency bug between do_vfs_ioctl() and do_readv()

[Gong, Sishuai]

We want to report another error message we saw which might be related to this bug.

---------------------------------------------
Kernel console output
[  139.414764] blk_update_request: I/O error, dev loop0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
[  139.628099] blk_update_request: I/O error, dev loop0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
[  139.840243] Buffer I/O error on dev loop0, logical block 0, async page read

---------------------------------------------
Description
This errors comes up when the following two kernel test input run concurrently. The reason we still think this is a concurrency bug because only a few interleavings can reproduce it.
However, due to the complexity, we are still diagnosing this problem and we will try to provide a more detailed analysis soon.

Test input 1
syz_mount_image$ext4(0x0, 0x0, 0x0, 0x1, &(0x7f0000000500)=[{&(0x7f00000002c0)="cc", 0x1, 0x7fff}], 0x0, 0x0)

Test input 2
syz_mount_image$msdos(&(0x7f0000000000)='msdos\x00', &(0x7f0000000040)='./file0\x00', 0x0, 0x0, &(0x7f0000000200), 0x0, &(0x7f0000000280)={[{@fat=@codepage={'codepage', 0x3d, '950'}}, {@fat=@fmask={'fmask', 0x3d, 0x100000001}}]})


Thanks,
Sishuai

> On Sep 28, 2020, at 10:44 AM, Gong, Sishuai <sishuai@purdue.edu> 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
> 

Re: PROBLEM: potential concurrency bug between do_vfs_ioctl() and do_readv()

[Matthew Wilcox]

On Wed, Dec 02, 2020 at 03:18:26AM +0000, Gong, Sishuai wrote:
> We want to report another error message we saw which might be related to this bug.

https://syzkaller.appspot.com/upstream

There are over a thousand open bugs already.  Why are the extra bugs
being found by your group useful?

Re: PROBLEM: potential concurrency bug between do_vfs_ioctl() and do_readv()

[Gong, Sishuai]

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@purdue.edu> 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
> 

Re: PROBLEM: potential concurrency bug between do_vfs_ioctl() and do_readv()

[Jan Kara]

On Tue 26-01-21 02:18:03, Gong, Sishuai wrote:
> 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);
> ...

So you are right a race like this can happen. However generally it is
expected that you will see IO errors when you run loop_clr_fd() while there
may be other processes submitting IO. The particular scenario you've shown
above could possibly be handled by more careful checking in lo_open() but if
we just slightly modify it like:

Thread 1				Thread 2
					// Execute lo_open()
					err = mutex_lock_killable(&loop_ctl_mutex);
					…
					lo = bdev->bd_disk->private_data;
					// lo_open return a success
// Execute loop_clr_fd()
lo->lo_state = Lo_rundown
mutex_unlock(&loop_ctl_mutex);
					// User makes a ksys_read() request
					// loop_queue_rq()
					if (lo->lo_state != Lo_bound)
// Execute __loop_clr_fd()
mutex_lock(&loop_ctl_mutex);

Then it's kind of obvious that no checking in lo_open() can prevent IO
errors when loop device is being torn down. So to summarize the error
messages are expected when loop device is torn down while IO is in
progress...

								Honza


> > On Sep 28, 2020, at 10:44 AM, Gong, Sishuai <sishuai@purdue.edu> 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
> > 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR