On Wed, Mar 01, 2017 at 10:44:53AM +0800, Qu Wenruo wrote:
>
>
> At 03/01/2017 09:04 AM, Liu Bo wrote:
> > Btrfs creates hole extents to cover any unwritten section right before
> > doing buffer writes after commit 3ac0d7b96a26 ("btrfs: Change the expanding
> > write sequence to fix snapshot related bug.").
> >
> > However, that takes the start position of the buffered write to compare
> > against the current EOF, hole extents would be created only if (EOF <
> > start).
> >
> > If the EOF is at the middle of the buffered write, no hole extents will be
> > created and a file hole without a hole extent is left in this file.
> >
> > This bug was revealed by generic/019 in fstests. 'fsstress' in this test
> > may create the above situation and the test then fails all requests
> > including writes, so the buffer write which is supposed to cover the
> > hole (without the hole extent) couldn't make it on disk. Running fsck
> > against such btrfs ends up with detecting file extent holes.
> >
> > Things could be more serious, some stale data would be exposed to
> > userspace if files with this kind of hole are truncated to a position of
> > the hole, because the on-disk inode size is beyond the last extent in the
> > file.
> >
> > This fixes the bug by comparing the end position against the EOF.
> >
> > Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
>
> Patch looks good to me.
> Reviewed-by: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
>
> > ---
> > fs/btrfs/file.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > index b5c5da2..0be837b 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1861,11 +1861,10 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
> > pos = iocb->ki_pos;
> > count = iov_iter_count(from);
> > start_pos = round_down(pos, fs_info->sectorsize);
> > + end_pos = round_up(pos + count, fs_info->sectorsize);
> > oldsize = i_size_read(inode);
> > - if (start_pos > oldsize) {
> > + if (end_pos > oldsize) {
> > /* Expand hole size to cover write data, preventing empty gap */
>
> The comment still makes sense here, but it could be better to explain why to
> insert the hole to cover the whole write range (in case write fails)
>
Sounds good, I'll update.
Thanks,
-liubo
> Thanks,
> Qu
>
> > - end_pos = round_up(pos + count,
> > - fs_info->sectorsize);
> > err = btrfs_cont_expand(inode, oldsize, end_pos);
> > if (err) {
> > inode_unlock(inode);
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
