fs: possible deadlock in do_iter_write/do_splice

fs: possible deadlock in do_iter_write/do_splice

[Andrey Konovalov]

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

On commit 9b51f04424e17051a89ab32d892ca66b2a104825 (4.12+).

============================================
WARNING: possible recursive locking detected
4.12.0+ #42 Not tainted
--------------------------------------------
syz-executor4/6792 is trying to acquire lock:
 (sb_writers#5){.+.+.+}, at: [<ffffffff818d1598>] file_start_write
include/linux/fs.h:2601 [inline]
 (sb_writers#5){.+.+.+}, at: [<ffffffff818d1598>]
do_iter_write+0x428/0x540 fs/read_write.c:928

but task is already holding lock:
 (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>] file_start_write
include/linux/fs.h:2601 [inline]
 (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>] do_splice
fs/splice.c:1162 [inline]
 (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>] SYSC_splice
fs/splice.c:1418 [inline]
 (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>]
SyS_splice+0x1170/0x16d0 fs/splice.c:1398

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(sb_writers#5);
  lock(sb_writers#5);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

2 locks held by syz-executor4/6792:
 #0:  (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>]
file_start_write include/linux/fs.h:2601 [inline]
 #0:  (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>] do_splice
fs/splice.c:1162 [inline]
 #0:  (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>] SYSC_splice
fs/splice.c:1418 [inline]
 #0:  (sb_writers#5){.+.+.+}, at: [<ffffffff819ab890>]
SyS_splice+0x1170/0x16d0 fs/splice.c:1398
 #1:  (&pipe->mutex/1){+.+.+.}, at: [<ffffffff818f1900>]
pipe_lock_nested fs/pipe.c:66 [inline]
 #1:  (&pipe->mutex/1){+.+.+.}, at: [<ffffffff818f1900>] pipe_lock
fs/pipe.c:74 [inline]
 #1:  (&pipe->mutex/1){+.+.+.}, at: [<ffffffff818f1900>]
pipe_wait+0x240/0x2f0 fs/pipe.c:122

stack backtrace:
CPU: 0 PID: 6792 Comm: syz-executor4 Not tainted 4.12.0+ #42
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:16 [inline]
 dump_stack+0x292/0x395 lib/dump_stack.c:52
 print_deadlock_bug kernel/locking/lockdep.c:1743 [inline]
 check_deadlock kernel/locking/lockdep.c:1787 [inline]
 validate_chain kernel/locking/lockdep.c:2265 [inline]
 __lock_acquire+0xf7f/0x3690 kernel/locking/lockdep.c:3367
 ? 0xffffffffa0000000
 lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
 percpu_down_read_preempt_disable include/linux/percpu-rwsem.h:35 [inline]
 percpu_down_read include/linux/percpu-rwsem.h:58 [inline]
 __sb_start_write+0x19c/0x2a0 fs/super.c:1341
 file_start_write include/linux/fs.h:2601 [inline]
 do_iter_write+0x428/0x540 fs/read_write.c:928
 vfs_iter_write+0x7c/0xb0 fs/read_write.c:944
 iter_file_splice_write+0x83b/0xfa0 fs/splice.c:765
 do_splice_from fs/splice.c:867 [inline]
 do_splice fs/splice.c:1163 [inline]
 SYSC_splice fs/splice.c:1418 [inline]
 SyS_splice+0x7f2/0x16d0 fs/splice.c:1398
 entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x446729
RSP: 002b:00007f5e1cd8ec08 EFLAGS: 00000292 ORIG_RAX: 0000000000000113
RAX: ffffffffffffffda RBX: 0000000000005430 RCX: 0000000000446729
RDX: 0000000000000018 RSI: 0000000000000000 RDI: 0000000000000019
RBP: 00000000ffffffff R08: 0000000000000006 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000292 R12: 0000000000000019
R13: 0000000000003be0 R14: 00000000006e5ca0 R15: 0000000000000000

Re: fs: possible deadlock in do_iter_write/do_splice

[Al Viro]

On Thu, Jul 06, 2017 at 03:01:11PM +0200, Andrey Konovalov wrote:
> Hi,
> 
> I've got the following error report while fuzzing the kernel with syzkaller.
> 
> On commit 9b51f04424e17051a89ab32d892ca66b2a104825 (4.12+).

It's "fs: implement vfs_iter_write using do_iter_write", actually.

vfs_iter_write() has no business playing with file_start_write(); do_iter_write()
does.  

commit 62473a2d6f513296e896b64ccfd7ceb7b963795e
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Thu Jul 6 09:15:47 2017 -0400

    move file_{start,end}_write() out of do_iter_write()
    
    ... and do *not* grab it in vfs_write_iter().
    
    Fixes: "fs: implement vfs_iter_read using do_iter_read"
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/read_write.c b/fs/read_write.c
index acfd3dc63d57..be1125b30aec 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -931,12 +931,10 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter,
 	if (ret < 0)
 		return ret;
 
-	file_start_write(file);
 	if (file->f_op->write_iter)
 		ret = do_iter_readv_writev(file, iter, pos, WRITE, flags);
 	else
 		ret = do_loop_readv_writev(file, iter, pos, WRITE, flags);
-	file_end_write(file);
 	if (ret > 0)
 		fsnotify_modify(file);
 	return ret;
@@ -979,7 +977,9 @@ ssize_t vfs_writev(struct file *file, const struct iovec __user *vec,
 
 	ret = import_iovec(WRITE, vec, vlen, ARRAY_SIZE(iovstack), &iov, &iter);
 	if (ret >= 0) {
+		file_start_write(file);
 		ret = do_iter_write(file, &iter, pos, flags);
+		file_end_write(file);
 		kfree(iov);
 	}
 	return ret;
@@ -1247,7 +1247,9 @@ static size_t compat_writev(struct file *file,
 
 	ret = compat_import_iovec(WRITE, vec, vlen, UIO_FASTIOV, &iov, &iter);
 	if (ret >= 0) {
+		file_start_write(file);
 		ret = do_iter_write(file, &iter, pos, flags);
+		file_end_write(file);
 		kfree(iov);
 	}
 	if (ret > 0)