From: Miklos Szeredi mszeredi@redhat.com
commit 0c4bcfdecb1ac0967619ee7ff44871d93c08c909 upstream.
In FOPEN_DIRECT_IO mode, fuse_file_write_iter() calls fuse_direct_write_iter(), which normally calls fuse_direct_io(), which then imports the write buffer with fuse_get_user_pages(), which uses iov_iter_get_pages() to grab references to userspace pages instead of actually copying memory.
On the filesystem device side, these pages can then either be read to userspace (via fuse_dev_read()), or splice()d over into a pipe using fuse_dev_splice_read() as pipe buffers with &nosteal_pipe_buf_ops.
This is wrong because after fuse_dev_do_read() unlocks the FUSE request, the userspace filesystem can mark the request as completed, causing write() to return. At that point, the userspace filesystem should no longer have access to the pipe buffer.
Fix by copying pages coming from the user address space to new pipe buffers.
Reported-by: Jann Horn jannh@google.com Fixes: c3021629a0d8 ("fuse: support splice() reading from fuse device") Cc: stable@vger.kernel.org Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Zach O'Keefe zokeefe@google.com
--- Applies against stable-v4.14 and stable-v4.19
struct fuse_args hasn't been piped through relevant functions yet, so place user_pages flag in an empty hole in struct fuse_req instead.
fs/fuse/dev.c | 12 +++++++++++- fs/fuse/file.c | 1 + fs/fuse/fuse_i.h | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index db7d746633cf..1c98b5b7bead 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -991,7 +991,17 @@ static int fuse_copy_page(struct fuse_copy_state *cs, struct page **pagep,
while (count) { if (cs->write && cs->pipebufs && page) { - return fuse_ref_page(cs, page, offset, count); + /* + * Can't control lifetime of pipe buffers, so always + * copy user pages. + */ + if (cs->req->user_pages) { + err = fuse_copy_fill(cs); + if (err) + return err; + } else { + return fuse_ref_page(cs, page, offset, count); + } } else if (!cs->len) { if (cs->move_pages && page && offset == 0 && count == PAGE_SIZE) { diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 5f5da2911cea..a32b2ca3de6f 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1325,6 +1325,7 @@ static int fuse_get_user_pages(struct fuse_req *req, struct iov_iter *ii, (PAGE_SIZE - ret) & (PAGE_SIZE - 1); }
+ req->user_pages = true; if (write) req->in.argpages = 1; else diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index fac1f08dd32e..30fdede2ea64 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -312,6 +312,8 @@ struct fuse_req { /** refcount */ refcount_t count;
+ bool user_pages; + /** Unique ID for the interrupt request */ u64 intr_unique;
On Wed, Mar 30, 2022 at 08:45:04AM -0700, Zach O'Keefe wrote:
From: Miklos Szeredi mszeredi@redhat.com
commit 0c4bcfdecb1ac0967619ee7ff44871d93c08c909 upstream.
In FOPEN_DIRECT_IO mode, fuse_file_write_iter() calls fuse_direct_write_iter(), which normally calls fuse_direct_io(), which then imports the write buffer with fuse_get_user_pages(), which uses iov_iter_get_pages() to grab references to userspace pages instead of actually copying memory.
On the filesystem device side, these pages can then either be read to userspace (via fuse_dev_read()), or splice()d over into a pipe using fuse_dev_splice_read() as pipe buffers with &nosteal_pipe_buf_ops.
This is wrong because after fuse_dev_do_read() unlocks the FUSE request, the userspace filesystem can mark the request as completed, causing write() to return. At that point, the userspace filesystem should no longer have access to the pipe buffer.
Fix by copying pages coming from the user address space to new pipe buffers.
Reported-by: Jann Horn jannh@google.com Fixes: c3021629a0d8 ("fuse: support splice() reading from fuse device") Cc: stable@vger.kernel.org Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Zach O'Keefe zokeefe@google.com
Applies against stable-v4.14 and stable-v4.19
struct fuse_args hasn't been piped through relevant functions yet, so place user_pages flag in an empty hole in struct fuse_req instead.
Thanks for the backport, now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org