From: Miklos Szeredi <miklos@szeredi.hu>
To: Xiao Yang <ice_yangxiao@163.com>
Cc: Vivek Goyal <vgoyal@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
linux-fsdevel@vger.kernel.org, yangx.jy@cn.fujitsu.com
Subject: Re: [PATCH] fuse: Don't make buffered read forward overflow value to a userspace process
Date: Thu, 6 Feb 2020 13:14:44 +0100 [thread overview]
Message-ID: <CAJfpegsPfurF2fB+XgSjr-CnBNcjWiqYCB6bFwP8VKNp3sUChA@mail.gmail.com> (raw)
In-Reply-To: <CAJfpegsVjca2xGV=9xwE75a5NRSYqLpDu9x_q9CeDZ1vt-GyyQ@mail.gmail.com>
On Wed, Feb 5, 2020 at 3:37 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Mon, Feb 3, 2020 at 8:37 AM Xiao Yang <ice_yangxiao@163.com> wrote:
> >
> > Buffered read in fuse normally goes via:
> > -> generic_file_buffered_read()
> > ------------------------------
> > -> fuse_readpages()
> > -> fuse_send_readpages()
> > or
> > -> fuse_readpage() [if fuse_readpages() fails to get page]
> > -> fuse_do_readpage()
> > ------------------------------
> > -> fuse_simple_request()
> >
> > Buffered read changes original offset to page-aligned length by left-shift
> > and extends original count to be multiples of PAGE_SIZE and then fuse
> > forwards these new parameters to a userspace process, so it is possible
> > for the resulting offset(e.g page-aligned offset + extended count) to
> > exceed the whole file size(even the max value of off_t) when the userspace
> > process does read with new parameters.
> >
> > xfstests generic/525 gets "pread: Invalid argument" error on virtiofs
> > because it triggers this issue. See the following explanation:
> > PAGE_SIZE: 4096, file size: 2^63 - 1
> > Original: offset: 2^63 - 2, count: 1
> > Changed by buffered read: offset: 2^63 - 4096, count: 4096
> > New offset + new count exceeds the file size as well as LLONG_MAX
>
> Thanks for the report and analysis.
>
> However this patch may corrupt the cache if i_size changes between
> calls to fuse_page_length(). (e.g. first page length set to 33;
> second page length to 45; then 33-4095 will be zeroed and 4096-4140
> will be filled from offset 33-77). This will be mitigated by the
> pages being invalidated when i_size changes
> (fuse_change_attributes()). Filling the pages with wrong data is not
> a good idea regardless.
>
> I think the best approach is first to just fix the xfstest reported
> bug by clamping the end offset to LLONG_MAX. That's a simple patch,
> independent of i_size, and hence trivial to verify and hard to mess
> up.
Applied a fix and pushed to:
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#for-next
Thanks,
Miklos
next prev parent reply other threads:[~2020-02-06 12:14 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-03 7:36 [PATCH] fuse: Don't make buffered read forward overflow value to a userspace process Xiao Yang
2020-02-05 14:37 ` Miklos Szeredi
2020-02-06 12:14 ` Miklos Szeredi [this message]
2020-02-06 12:32 ` Xiao Yang
2020-02-06 15:40 ` Miklos Szeredi
2020-02-07 0:44 ` Xiao Yang
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=CAJfpegsPfurF2fB+XgSjr-CnBNcjWiqYCB6bFwP8VKNp3sUChA@mail.gmail.com \
--to=miklos@szeredi.hu \
--cc=ice_yangxiao@163.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=stefanha@redhat.com \
--cc=vgoyal@redhat.com \
--cc=yangx.jy@cn.fujitsu.com \
/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).