fs: move file_start_write() into direct_splice_actor()

The callers of do_splice_direct() hold file_start_write() on the output
file.

This may cause file permission hooks to be called indirectly on an
overlayfs lower layer, which is on the same filesystem of the output
file and could lead to deadlock with fanotify permission events.

To fix this potential deadlock, move file_start_write() from the callers
into the direct_splice_actor(), so file_start_write() will not be held
while splicing from the input file.

Suggested-by: Josef Bacik <josef@toxicpanda.com>
Link: https://lore.kernel.org/r/20231128214258.GA2398475@perftesting/
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Link: https://lore.kernel.org/r/20231130141624.3338942-3-amir73il@gmail.com
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 106f8643af3bb28da5a1f60f0bba442eb36e315c..2f587ee0b33421897dbe13f3c6d9ab76fa8a6bc5 100644 (file)
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -333,11 +333,9 @@ static int ovl_copy_up_file(struct ovl_fs *ofs, struct dentry *dentry,
                if (error)
                        break;
 
-               ovl_start_write(dentry);
                bytes = do_splice_direct(old_file, &old_pos,
                                         new_file, &new_pos,
                                         this_len, SPLICE_F_MOVE);
-               ovl_end_write(dentry);
                if (bytes <= 0) {
                        error = bytes;
                        break;

diff --git a/fs/read_write.c b/fs/read_write.c
index 642c7ce1ced1f66f989675f9eb5e8d6db5f04e20..0bc99f38e623d79df4ce68254329e895d15a0b38 100644 (file)
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1286,10 +1286,8 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
                retval = rw_verify_area(WRITE, out.file, &out_pos, count);
                if (retval < 0)
                        goto fput_out;
-               file_start_write(out.file);
                retval = do_splice_direct(in.file, &pos, out.file, &out_pos,
                                          count, fl);
-               file_end_write(out.file);
        } else {
                if (out.file->f_flags & O_NONBLOCK)
                        fl |= SPLICE_F_NONBLOCK;

diff --git a/fs/splice.c b/fs/splice.c
index 9007b2c8baa8840e980b07d3f2e5c54a02c15134..7cda013e5a1ef1f5781f6e1d6774c8865a30ab0d 100644 (file)
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1157,9 +1157,20 @@ static int direct_splice_actor(struct pipe_inode_info *pipe,
                               struct splice_desc *sd)
 {
        struct file *file = sd->u.file;
+       long ret;
+
+       file_start_write(file);
+       ret = do_splice_from(pipe, file, sd->opos, sd->total_len, sd->flags);
+       file_end_write(file);
+       return ret;
+}
 
-       return do_splice_from(pipe, file, sd->opos, sd->total_len,
-                             sd->flags);
+static int splice_file_range_actor(struct pipe_inode_info *pipe,
+                                       struct splice_desc *sd)
+{
+       struct file *file = sd->u.file;
+
+       return do_splice_from(pipe, file, sd->opos, sd->total_len, sd->flags);
 }
 
 static void direct_file_splice_eof(struct splice_desc *sd)
@@ -1233,6 +1244,8 @@ EXPORT_SYMBOL(do_splice_direct);
  *
  * Description:
  *    For use by generic_copy_file_range() and ->copy_file_range() methods.
+ *    Like do_splice_direct(), but vfs_copy_file_range() already holds
+ *    start_file_write() on @out file.
  *
  * Callers already called rw_verify_area() on the entire range.
  */
@@ -1242,7 +1255,7 @@ long splice_file_range(struct file *in, loff_t *ppos, struct file *out,
        lockdep_assert(file_write_started(out));
 
        return do_splice_direct_actor(in, ppos, out, opos, len, 0,
-                                     direct_splice_actor);
+                                     splice_file_range_actor);
 }
 EXPORT_SYMBOL(splice_file_range);