On Sun, Jul 25, 2021 at 3:31 AM Hillf Danton <hdanton@xxxxxxxx> wrote:
>
> On Sat, 24 Jul 2021 12:07:20 -0700
> >syzbot found the following issue on:
> >
> >HEAD commit: 8cae8cd89f05 seq_file: disallow extremely large seq buffer..
> >git tree: upstream
> >console output: https://syzkaller.appspot.com/x/log.txt?x=1083e8cc300000
> >kernel config: https://syzkaller.appspot.com/x/.config?x=7273c75708b55890
> >dashboard link: https://syzkaller.appspot.com/bug?extid=579885d1a9a833336209
> >syz repro: https://syzkaller.appspot.com/x/repro.syz?x=163905f2300000
> >C reproducer: https://syzkaller.appspot.com/x/repro.c?x=165bd0ea300000
> >
> >The issue was bisected to:
> >
> >commit 82a763e61e2b601309d696d4fa514c77d64ee1be
> >Author: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> >Date: Mon Dec 14 14:26:14 2020 +0000
> >
> > ovl: simplify file splice
>
>
> If this commit is innocent then is it false positive lockdep warning again,
> given another report [1]?
Appears to be legit.
Attached partial revert + sync with ovl_write_iter() should fix it
(fingers crossed).
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
Thanks,
Miklos
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 4d53d3b7e5fe..d081faa55e83 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -392,6 +392,51 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter)
return ret;
}
+/*
+ * Calling iter_file_splice_write() directly from overlay's f_op may deadlock
+ * due to lock order inversion between pipe->mutex in iter_file_splice_write()
+ * and file_start_write(real.file) in ovl_write_iter().
+ *
+ * So do everything ovl_write_iter() does and call iter_file_splice_write() on
+ * the real file.
+ */
+static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out,
+ loff_t *ppos, size_t len, unsigned int flags)
+{
+ struct fd real;
+ const struct cred *old_cred;
+ struct inode *inode = file_inode(out);
+ struct inode *realinode = ovl_inode_real(inode);
+ ssize_t ret;
+
+ inode_lock(inode);
+ /* Update mode */
+ ovl_copyattr(realinode, inode);
+ ret = file_remove_privs(out);
+ if (ret)
+ goto out_unlock;
+
+ ret = ovl_real_fdget(out, &real);
+ if (ret)
+ goto out_unlock;
+
+ old_cred = ovl_override_creds(inode->i_sb);
+ file_start_write(real.file);
+
+ ret = iter_file_splice_write(pipe, real.file, ppos, len, flags);
+
+ file_end_write(real.file);
+ /* Update size */
+ ovl_copyattr(realinode, inode);
+ revert_creds(old_cred);
+ fdput(real);
+
+out_unlock:
+ inode_unlock(inode);
+
+ return ret;
+}
+
static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
{
struct fd real;
@@ -603,7 +648,7 @@ const struct file_operations ovl_file_operations = {
.fadvise = ovl_fadvise,
.flush = ovl_flush,
.splice_read = generic_file_splice_read,
- .splice_write = iter_file_splice_write,
+ .splice_write = ovl_splice_write,
.copy_file_range = ovl_copy_file_range,
.remap_file_range = ovl_remap_file_range,
