* [PATCH CK 4.19 1/4] fuse: fix page dereference after free
[not found] <1614569779-12114-1-git-send-email-tao.peng@linux.alibaba.com>
@ 2021-03-01 3:36 ` Peng Tao
2021-03-01 8:05 ` Greg Kroah-Hartman
2021-03-01 3:36 ` [PATCH CK 4.19 4/4] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS Peng Tao
1 sibling, 1 reply; 5+ messages in thread
From: Peng Tao @ 2021-03-01 3:36 UTC (permalink / raw)
To: alikernel-developer
Cc: Liu Bo, Ma Jie Yue, Joseph Qi, Eryu Guan, Miklos Szeredi, stable,
Greg Kroah-Hartman, Peng Tao
From: Miklos Szeredi <mszeredi@redhat.com>
commit d78092e4937de9ce55edcb4ee4c5e3c707be0190 upstream.
fix #32833505
After unlock_request() pages from the ap->pages[] array may be put (e.g. by
aborting the connection) and the pages can be freed.
Prevent use after free by grabbing a reference to the page before calling
unlock_request().
The original patch was created by Pradeep P V K.
Reported-by: Pradeep P V K <ppvk@codeaurora.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
---
fs/fuse/dev.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3202ead..8ad877a 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -877,15 +877,16 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep)
struct page *newpage;
struct pipe_buffer *buf = cs->pipebufs;
+ get_page(oldpage);
err = unlock_request(cs->req);
if (err)
- return err;
+ goto out_put_old;
fuse_copy_finish(cs);
err = pipe_buf_confirm(cs->pipe, buf);
if (err)
- return err;
+ goto out_put_old;
BUG_ON(!cs->nr_segs);
cs->currbuf = buf;
@@ -925,7 +926,7 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep)
err = replace_page_cache_page(oldpage, newpage, GFP_KERNEL);
if (err) {
unlock_page(newpage);
- return err;
+ goto out_put_old;
}
get_page(newpage);
@@ -944,14 +945,19 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep)
if (err) {
unlock_page(newpage);
put_page(newpage);
- return err;
+ goto out_put_old;
}
unlock_page(oldpage);
+ /* Drop ref for ap->pages[] array */
put_page(oldpage);
cs->len = 0;
- return 0;
+ err = 0;
+out_put_old:
+ /* Drop ref obtained in this function */
+ put_page(oldpage);
+ return err;
out_fallback_unlock:
unlock_page(newpage);
@@ -960,10 +966,10 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep)
cs->offset = buf->offset;
err = lock_request(cs->req);
- if (err)
- return err;
+ if (!err)
+ err = 1;
- return 1;
+ goto out_put_old;
}
static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
@@ -975,14 +981,16 @@ static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
if (cs->nr_segs == cs->pipe->buffers)
return -EIO;
+ get_page(page);
err = unlock_request(cs->req);
- if (err)
+ if (err) {
+ put_page(page);
return err;
+ }
fuse_copy_finish(cs);
buf = cs->pipebufs;
- get_page(page);
buf->page = page;
buf->offset = offset;
buf->len = count;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH CK 4.19 4/4] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS
[not found] <1614569779-12114-1-git-send-email-tao.peng@linux.alibaba.com>
2021-03-01 3:36 ` [PATCH CK 4.19 1/4] fuse: fix page dereference after free Peng Tao
@ 2021-03-01 3:36 ` Peng Tao
1 sibling, 0 replies; 5+ messages in thread
From: Peng Tao @ 2021-03-01 3:36 UTC (permalink / raw)
To: alikernel-developer
Cc: Liu Bo, Ma Jie Yue, Joseph Qi, Eryu Guan, Chirantan Ekbote,
stable, Miklos Szeredi, Greg Kroah-Hartman, Peng Tao
From: Chirantan Ekbote <chirantan@chromium.org>
commit 31070f6ccec09f3bd4f1e28cd1e592fa4f3ba0b6 upstream.
fix #32833505
The ioctl encoding for this parameter is a long but the documentation says
it should be an int and the kernel drivers expect it to be an int. If the
fuse driver treats this as a long it might end up scribbling over the stack
of a userspace process that only allocated enough space for an int.
This was previously discussed in [1] and a patch for fuse was proposed in
[2]. From what I can tell the patch in [2] was nacked in favor of adding
new, "fixed" ioctls and using those from userspace. However there is still
no "fixed" version of these ioctls and the fact is that it's sometimes
infeasible to change all userspace to use the new one.
Handling the ioctls specially in the fuse driver seems like the most
pragmatic way for fuse servers to support them without causing crashes in
userspace applications that call them.
[1]: https://lore.kernel.org/linux-fsdevel/20131126200559.GH20559@hall.aurel32.net/T/
[2]: https://sourceforge.net/p/fuse/mailman/message/31771759/
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Fixes: 59efec7b9039 ("fuse: implement ioctl support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
---
fs/fuse/file.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index e02fafc..441e154 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -18,6 +18,7 @@
#include <linux/swap.h>
#include <linux/falloc.h>
#include <linux/uio.h>
+#include <linux/fs.h>
static const struct file_operations fuse_direct_io_file_operations;
@@ -2544,7 +2545,16 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
struct iovec *iov = iov_page;
iov->iov_base = (void __user *)arg;
- iov->iov_len = _IOC_SIZE(cmd);
+
+ switch (cmd) {
+ case FS_IOC_GETFLAGS:
+ case FS_IOC_SETFLAGS:
+ iov->iov_len = sizeof(int);
+ break;
+ default:
+ iov->iov_len = _IOC_SIZE(cmd);
+ break;
+ }
if (_IOC_DIR(cmd) & _IOC_WRITE) {
in_iov = iov;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread