diff mbox series

Btrfs: fix assertion on fsync of regular file when using no-holes feature

Message ID 20181015085100.5720-1-fdmanana@kernel.org (mailing list archive)
State New, archived
Headers show
Series Btrfs: fix assertion on fsync of regular file when using no-holes feature | expand

Commit Message

Filipe Manana Oct. 15, 2018, 8:51 a.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

When using the NO_HOLES feature and logging a regular file, we were
expecting that if we find an inline extent, that either its size in ram
(uncompressed and unenconded) matches the size of the file or if it does
not, that it matches the sector size and it represents compressed data.
This assertion does not cover a case where the length of the inline extent
is smaller then the sector size and also smaller the file's size, such
case is possible through fallocate. Example:

  $ mkfs.btrfs -f -O no-holes /dev/sdb
  $ mount /dev/sdb /mnt

  $ xfs_io -f -c "pwrite -S 0xb60 0 21" /mnt/foobar
  $ xfs_io -c "falloc 40 40" /mnt/foobar
  $ xfs_io -c "fsync" /mnt/foobar

In the abobe example we trigger the assertion because the inline extent's
length is 21 bytes while the file size is 80 bytes. The fallocate() call
merely updated the file's size and did not touch the existing inline
extent, as expected.

So fix this by adjusting the assertion so that an inline extent length
smaller then the file size is valid if the file size is smaller then the
filesystem's sector size.

A test case for fstests follows soon.

Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
Link: https://lore.kernel.org/linux-btrfs/CAE5jQCfRSBC7n4pUTFJcmHh109=gwyT9mFkCOL+NKfzswmR=_Q@mail.gmail.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

David Sterba Oct. 17, 2018, 3:46 p.m. UTC | #1 
On Mon, Oct 15, 2018 at 09:51:00AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When using the NO_HOLES feature and logging a regular file, we were
> expecting that if we find an inline extent, that either its size in ram
> (uncompressed and unenconded) matches the size of the file or if it does
> not, that it matches the sector size and it represents compressed data.
> This assertion does not cover a case where the length of the inline extent
> is smaller then the sector size and also smaller the file's size, such
> case is possible through fallocate. Example:
> 
>   $ mkfs.btrfs -f -O no-holes /dev/sdb
>   $ mount /dev/sdb /mnt
> 
>   $ xfs_io -f -c "pwrite -S 0xb60 0 21" /mnt/foobar
>   $ xfs_io -c "falloc 40 40" /mnt/foobar
>   $ xfs_io -c "fsync" /mnt/foobar
> 
> In the abobe example we trigger the assertion because the inline extent's
> length is 21 bytes while the file size is 80 bytes. The fallocate() call
> merely updated the file's size and did not touch the existing inline
> extent, as expected.
> 
> So fix this by adjusting the assertion so that an inline extent length
> smaller then the file size is valid if the file size is smaller then the
> filesystem's sector size.
> 
> A test case for fstests follows soon.
> 
> Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/CAE5jQCfRSBC7n4pUTFJcmHh109=gwyT9mFkCOL+NKfzswmR=_Q@mail.gmail.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Added to misc-next, thanks.
diff mbox series

Patch

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index eeeeed455dba..1673dccc76c2 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -4652,7 +4652,8 @@  static int btrfs_log_trailing_hole(struct btrfs_trans_handle *trans,
 			ASSERT(len == i_size ||
 			       (len == fs_info->sectorsize &&
 				btrfs_file_extent_compression(leaf, extent) !=
-				BTRFS_COMPRESS_NONE));
+				BTRFS_COMPRESS_NONE) ||
+			       (len < i_size && i_size < fs_info->sectorsize));
 			return 0;
 		}