linux-kernel - Re: [PATCH 2/2] fuse: Replace page without copying in fuse_writepage_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Jan 2019 17:36:18 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Kirill Tkhai <ktkhai@...tuozzo.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] fuse: Replace page without copying in fuse_writepage_in_flight()

On Tue, Jan 15, 2019 at 5:14 PM Kirill Tkhai <ktkhai@...tuozzo.com> wrote:
>
> On 15.01.2019 18:39, Miklos Szeredi wrote:
> > On Mon, Nov 26, 2018 at 10:46 AM Kirill Tkhai <ktkhai@...tuozzo.com> wrote:
> >>
> >> It looks like we can optimize old_req page replacement
> >> and avoid copying by simple updating the request's page.
> >>
> >> Signed-off-by: Kirill Tkhai <ktkhai@...tuozzo.com>
> >> ---
> >>  fs/fuse/file.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> >> index c6650c68b31a..83b54b082c86 100644
> >> --- a/fs/fuse/file.c
> >> +++ b/fs/fuse/file.c
> >> @@ -1778,7 +1778,7 @@ static bool fuse_writepage_in_flight(struct fuse_req *new_req,
> >>         if (old_req->num_pages == 1 && old_req != first_req) {
> >>                 struct backing_dev_info *bdi = inode_to_bdi(page->mapping->host);
> >>
> >> -               copy_highpage(old_req->pages[0], page);
> >> +               swap(old_req->pages[0], page);
> >
> > This would mess up refcounting for all pages involved.   need to swap
> > with the temp page in new_req.    Fixed version in #for-next.
>
> You are sure, page is just a simple pointer, not struct **page.
> Then we would have had to change fuse_writepage_in_flight() to use ** pointer.

Using a struct page** would still have been broken, not because of
refcounting, but because of putting the wrong page into the request
(we do the temporary copy to avoid some issues with adding the page
cache page directly into the request)

Thanks,
Miklos