From: Amir Goldstein <amir73il@gmail.com>
To: Christian Brauner <brauner@kernel.org>, Jeff Layton <jlayton@kernel.org>
Cc: Josef Bacik <josef@toxicpanda.com>,
Christoph Hellwig <hch@lst.de>, Jan Kara <jack@suse.cz>,
David Howells <dhowells@redhat.com>, Jens Axboe <axboe@kernel.dk>,
Miklos Szeredi <miklos@szeredi.hu>,
Al Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org
Subject: [PATCH 2/2] fs: move file_start_write() into direct_splice_actor()
Date: Wed, 29 Nov 2023 22:07:09 +0200 [thread overview]
Message-ID: <20231129200709.3154370-3-amir73il@gmail.com> (raw)
In-Reply-To: <20231129200709.3154370-1-amir73il@gmail.com>
The two 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 reading the input file.
Suggested-by: Josef Bacik <josef@toxicpanda.com>
Link: https://lore.kernel.org/r/20231128214258.GA2398475@perftesting/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/overlayfs/copy_up.c | 2 --
fs/read_write.c | 2 --
fs/splice.c | 8 +++++++-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 7a44c8212331..294b330aba9f 100644
--- 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 555514cdad53..b7110ee77c1c 100644
--- 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 3bb4936f8b70..261adfdfed9d 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1157,8 +1157,12 @@ static int direct_splice_actor(struct pipe_inode_info *pipe,
struct splice_desc *sd)
{
struct file *file = sd->u.file;
+ long ret;
- return do_splice_from(pipe, file, sd->opos, sd->total_len, sd->flags);
+ file_start_write(file);
+ ret = do_splice_from(pipe, file, sd->opos, sd->total_len, sd->flags);
+ file_end_write(file);
+ return ret;
}
static int copy_file_range_splice_actor(struct pipe_inode_info *pipe,
@@ -1240,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 called
+ * start_file_write() on @out file.
*
* Callers already called rw_verify_area() on the entire range.
*/
--
2.34.1
next prev parent reply other threads:[~2023-11-29 20:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-29 20:07 [PATCH 0/2] Avert possible deadlock with splice() and fanotify Amir Goldstein
2023-11-29 20:07 ` [PATCH 1/2] fs: fork do_splice_copy_file_range() from do_splice_direct() Amir Goldstein
2023-11-30 10:09 ` Amir Goldstein
2023-11-30 13:30 ` Jan Kara
2023-11-30 13:18 ` Christian Brauner
2023-11-30 13:37 ` Amir Goldstein
2023-11-29 20:07 ` Amir Goldstein [this message]
2023-11-30 13:32 ` [PATCH 2/2] fs: move file_start_write() into direct_splice_actor() Jan Kara
2023-11-30 8:32 ` [PATCH 0/2] Avert possible deadlock with splice() and fanotify Amir Goldstein
2023-11-30 10:07 ` Amir Goldstein
2023-11-30 13:37 ` Jan Kara
2023-11-30 13:46 ` Amir Goldstein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231129200709.3154370-3-amir73il@gmail.com \
--to=amir73il@gmail.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=dhowells@redhat.com \
--cc=hch@lst.de \
--cc=jack@suse.cz \
--cc=jlayton@kernel.org \
--cc=josef@toxicpanda.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).