[PATCH CK 4.19 1/4] fuse: fix page dereference after free

[PATCH CK 4.19 1/4] fuse: fix page dereference after free

[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

[PATCH CK 4.19 4/4] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS

[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

Re: [PATCH CK 4.19 1/4] fuse: fix page dereference after free

[Greg Kroah-Hartman]

On Mon, Mar 01, 2021 at 11:36:16AM +0800, Peng Tao wrote:
> From: Miklos Szeredi <mszeredi@redhat.com>
> 
> commit d78092e4937de9ce55edcb4ee4c5e3c707be0190 upstream.
> 
> fix #32833505

What does this mean?

And why are you all backporting random stable kernel patches to your
tree and not just taking all of them with a simple merge?

By selectivly cherry-picking patches like this, you are guaranteed to be
doing more work, and have a much more insecure and buggy kernel.  The
opposite of what your end goal should be, correct?

good luck!

greg k-h

Re: [PATCH CK 4.19 1/4] fuse: fix page dereference after free

[Peng Tao]

On 2021/3/1 16:05, Greg Kroah-Hartman wrote:
> On Mon, Mar 01, 2021 at 11:36:16AM +0800, Peng Tao wrote:
>> From: Miklos Szeredi <mszeredi@redhat.com>
>>
>> commit d78092e4937de9ce55edcb4ee4c5e3c707be0190 upstream.
>>
>> fix #32833505
> 
> What does this mean?
> 
> And why are you all backporting random stable kernel patches to your
> tree and not just taking all of them with a simple merge?
> 
> By selectivly cherry-picking patches like this, you are guaranteed to be
> doing more work, and have a much more insecure and buggy kernel.  The
> opposite of what your end goal should be, correct?
> 

Hi Greg,

My apology for the noise. It was due to a mistake in my git config. And 
thanks for your suggestions. Our tree is actually a mixture of stable 
backports and feature backports. I guess that's why the cherry-picking 
method was chosen, since a simple merge creates too many conflicts and 
it is error prone to fix them in one shoot.

Cheers,
Tao

Re: [PATCH CK 4.19 1/4] fuse: fix page dereference after free

[Greg Kroah-Hartman]

On Mon, Mar 01, 2021 at 04:52:03PM +0800, Peng Tao wrote:
> 
> On 2021/3/1 16:05, Greg Kroah-Hartman wrote:
> > On Mon, Mar 01, 2021 at 11:36:16AM +0800, Peng Tao wrote:
> > > From: Miklos Szeredi <mszeredi@redhat.com>
> > > 
> > > commit d78092e4937de9ce55edcb4ee4c5e3c707be0190 upstream.
> > > 
> > > fix #32833505
> > 
> > What does this mean?
> > 
> > And why are you all backporting random stable kernel patches to your
> > tree and not just taking all of them with a simple merge?
> > 
> > By selectivly cherry-picking patches like this, you are guaranteed to be
> > doing more work, and have a much more insecure and buggy kernel.  The
> > opposite of what your end goal should be, correct?
> > 
> 
> Hi Greg,
> 
> My apology for the noise. It was due to a mistake in my git config. And
> thanks for your suggestions. Our tree is actually a mixture of stable
> backports and feature backports. I guess that's why the cherry-picking
> method was chosen, since a simple merge creates too many conflicts and it is
> error prone to fix them in one shoot.

A "simple merge" will cause initial problems, but after you have
resolved them the first time, all should be good.

As proof that this can work, see the android common kernel trees, which
receive a "simple merge" into all of the different branches within a day
or two of a stable release, with no problems at all.

You need to take all stable patches, doing this cherry-picking will
cause you problems and in the end, takes more time and effort!

good luck,

greg k-h