On Wed, Feb 5, 2020 at 3:37 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Mon, Feb 3, 2020 at 8:37 AM Xiao Yang <ice_yangxiao@xxxxxxx> 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
