From: Miklos Szeredi <miklos@szeredi.hu>
To: Pradeep P V K <ppvk@codeaurora.org>
Cc: Pradeep P V K <pragalla@qti.qualcomm.com>,
linux-fsdevel@vger.kernel.org,
Matthew Wilcox <willy@infradead.org>,
Sahitya Tummala <stummala@codeaurora.org>,
sayalil@codeaurora.org
Subject: Re: [PATCH V4] fuse: Fix VM_BUG_ON_PAGE issue while accessing zero ref count page
Date: Mon, 14 Sep 2020 10:11:29 +0200 [thread overview]
Message-ID: <CAJfpegtpLoskZDWwZpsEi=L_5jrvr7=xFG9GZJd8dTdJr647ww@mail.gmail.com> (raw)
In-Reply-To: <0101017478aef256-c8471520-26b1-4b87-a3b8-8266627b704f-000000@us-west-2.amazonses.com>
[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]
On Thu, Sep 10, 2020 at 5:42 PM <ppvk@codeaurora.org> wrote:
>
> On 2020-09-08 16:55, Miklos Szeredi wrote:
> > On Tue, Sep 8, 2020 at 10:17 AM Pradeep P V K
> > <pragalla@qti.qualcomm.com> wrote:
> >>
> >> From: Pradeep P V K <ppvk@codeaurora.org>
> >>
> >> There is a potential race between fuse_abort_conn() and
> >> fuse_copy_page() as shown below, due to which VM_BUG_ON_PAGE
> >> crash is observed for accessing a free page.
> >>
> >> context#1: context#2:
> >> fuse_dev_do_read() fuse_abort_conn()
> >> ->fuse_copy_args() ->end_requests()
> >
> > This shouldn't happen due to FR_LOCKED logic. Are you seeing this on
> > an upstream kernel? Which version?
> >
> > Thanks,
> > Miklos
>
> This is happen just after unlock_request() in fuse_ref_page(). In
> unlock_request(), it will clear the FR_LOCKED bit.
> As there is no protection between context#1 & context#2 during
> unlock_request(), there are chances that it could happen.
Ah, indeed, I missed that one.
Similar issue in fuse_try_move_page(), which dereferences oldpage
after unlock_request().
Fix for both is to grab a reference to the page from ap->pages[] array
*before* calling unlock_request().
Attached untested patch. Could you please verify that it fixes the bug?
Thanks,
Miklos
[-- Attachment #2: fuse-fix-page-dereference-after-free.patch --]
[-- Type: text/x-patch, Size: 2325 bytes --]
From: Miklos Szeredi <mszeredi@redhat.com>
Subject: fuse: fix page dereference after free
After unlock_request() pages from the ap->pages[] array may be put and can
be freed.
Prevent use after free by grabbing a reference to the page before calling
unlock_request().
This was originally reported by kernel test robot and the original patch
was created by Pradeep P V K.
Reported-by: Pradeep P V K <ppvk@codeaurora.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/dev.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -785,15 +785,16 @@ static int fuse_try_move_page(struct fus
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;
@@ -833,7 +834,7 @@ static int fuse_try_move_page(struct fus
err = replace_page_cache_page(oldpage, newpage, GFP_KERNEL);
if (err) {
unlock_page(newpage);
- return err;
+ goto out_put_old;
}
get_page(newpage);
@@ -852,14 +853,19 @@ static int fuse_try_move_page(struct fus
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);
@@ -868,10 +874,10 @@ static int fuse_try_move_page(struct fus
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,
@@ -883,14 +889,16 @@ static int fuse_ref_page(struct fuse_cop
if (cs->nr_segs >= cs->pipe->max_usage)
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;
next prev parent reply other threads:[~2020-09-14 8:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-08 8:17 [PATCH V4] fuse: Fix VM_BUG_ON_PAGE issue while accessing zero ref count page Pradeep P V K
2020-09-08 11:25 ` Miklos Szeredi
2020-09-10 15:42 ` ppvk
[not found] ` <0101017478aef256-c8471520-26b1-4b87-a3b8-8266627b704f-000000@us-west-2.amazonses.com>
2020-09-14 8:11 ` Miklos Szeredi [this message]
2020-09-14 13:32 ` ppvk
2020-09-16 15:31 ` ppvk
2020-09-18 8:40 ` Miklos Szeredi
2020-09-08 11:27 ` Matthew Wilcox
2020-09-10 15:49 ` ppvk
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='CAJfpegtpLoskZDWwZpsEi=L_5jrvr7=xFG9GZJd8dTdJr647ww@mail.gmail.com' \
--to=miklos@szeredi.hu \
--cc=linux-fsdevel@vger.kernel.org \
--cc=ppvk@codeaurora.org \
--cc=pragalla@qti.qualcomm.com \
--cc=sayalil@codeaurora.org \
--cc=stummala@codeaurora.org \
--cc=willy@infradead.org \
/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).