[PATCH 0/3][v2] Fix btrfs check handling of missing file extents

[PATCH 0/3][v2] Fix btrfs check handling of missing file extents

[Josef Bacik]

v1->v2:
- While running full xfstests with the new progs I noticed failures for tests
  that should have been fine.  Turns out our check will not wander down shared
  blocks, instead it'll merge inode records, which makes setting the
  rec->extent_start to key.offset very important.  So instead fix the problem by
  checking for the start gap once we're done doing all the merging.
- Added a third patch to make the hole error message actually correct when we
  have a gap at the beginning.

--------------- Original message ------------------

While adding an xfstest for the missing file extent problem I fixed with the
series

  btrfs: fix hole corruption issue with !NO_HOLES

I was failing to fail my test without my patches, despite the file system being
actually wrong.

It turns out because the normal check mode sets its expected start to the first
file extent it finds, instead of 0.  This means it misses any gaps between 0 and
the first file extent item in the inode.

The lowmem check does not have this issue, instead it doesn't take into account
the isize of the inode, so improperly fails when we have a gap but that is
outside of the isize.  I fixed this as well.

With these patches we're able to properly find another set of corruptions, and
now my xfstest acts sanely.  Thanks,

Josef

[PATCH 1/3] btrfs-progs: fix check to catch gaps at the start of the file

[Josef Bacik]

When writing my test for the i_size patches, I noticed that I was not
actually failing without my patches as I should have been.  This is
because we only check if the inode record extent end is < isize, we
don't check if the inode record extent start is > 0.  Add this check to
make sure we're catching holes that start at the beginning of the file.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/check/main.c b/check/main.c
index 4115049a..22f4e24c 100644
--- a/check/main.c
+++ b/check/main.c
@@ -827,8 +827,9 @@ static void maybe_free_inode_rec(struct cache_tree *inode_cache,
 		/* Orphan inodes don't have correct nbytes */
 		if (rec->nlink > 0 && rec->found_size != rec->nbytes)
 			rec->errors |= I_ERR_FILE_NBYTES_WRONG;
-		if (rec->nlink > 0 && !no_holes &&
+		if (rec->nlink > 0 && !no_holes && rec->isize &&
 		    (rec->extent_end < rec->isize ||
+		     rec->extent_start != 0 ||
 		     first_extent_gap(&rec->holes) < rec->isize))
 			rec->errors |= I_ERR_FILE_EXTENT_DISCOUNT;
 	}
-- 
2.24.1

[PATCH 2/3] btrfs-progs: fix lowmem check's handling of holes

[Josef Bacik]

Lowmem check had the opposite problem of normal check, it caught gaps
that started at 0, but would still fail with my fixes in place.  This is
because lowmem check doesn't take into account the isize of the inode.
Address this by making sure we do not complain about gaps that are after
isize.  This makes lowmem pass with my fixes applied, and still fail
without my fixes.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check/mode-lowmem.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 2f76d634..fd503aa6 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2029,7 +2029,8 @@ static int check_file_extent_inline(struct btrfs_root *root,
  * Return 0 if no error occurred.
  */
 static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
-			     unsigned int nodatasum, u64 *size, u64 *end)
+			     unsigned int nodatasum, u64 isize, u64 *size,
+			     u64 *end)
 {
 	struct btrfs_file_extent_item *fi;
 	struct btrfs_key fkey;
@@ -2152,7 +2153,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 	}
 
 	/* Check EXTENT_DATA hole */
-	if (!no_holes && *end != fkey.offset) {
+	if (!no_holes && (fkey.offset < isize) && (*end != fkey.offset)) {
 		if (repair)
 			ret = punch_extent_hole(root, path, fkey.objectid,
 						*end, fkey.offset - *end);
@@ -2165,7 +2166,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 		}
 	}
 
-	*end = fkey.offset + extent_num_bytes;
+	if (fkey.offset + extent_num_bytes < isize)
+		*end = fkey.offset + extent_num_bytes;
 	if (!is_hole)
 		*size += extent_num_bytes;
 
@@ -2726,7 +2728,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 					root->objectid, inode_id, key.objectid,
 					key.offset);
 			}
-			ret = check_file_extent(root, path, nodatasum,
+			ret = check_file_extent(root, path, nodatasum, isize,
 						&extent_size, &extent_end);
 			err |= ret;
 			break;
-- 
2.24.1

[PATCH 3/3] btrfs-progs: fix hole error output in fsck

[Josef Bacik]

If we don't find holes in our hole rb tree we'll just assume there's a
gap from 0 to the length of the file and print that out.  But this
simply isn't correct, we could have a gap between the last extent and
the isize, or 0 and the start of the first extent.  Fix the error
message to tell us exactly where the hole is.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check/main.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/check/main.c b/check/main.c
index 22f4e24c..71b68ff9 100644
--- a/check/main.c
+++ b/check/main.c
@@ -639,10 +639,20 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 				hole->start, hole->len);
 			node = rb_next(node);
 		}
-		if (!found)
-			fprintf(stderr, "\tstart: 0, len: %llu\n",
-				round_up(rec->isize,
-					 root->fs_info->sectorsize));
+		if (!found) {
+			u64 start, len;
+			if (rec->extent_end < rec->isize) {
+				start = rec->extent_end;
+				len = round_up(rec->isize,
+					       root->fs_info->sectorsize) -
+					start;
+			} else {
+				start = 0;
+				len = rec->extent_start;
+			}
+			fprintf(stderr, "\tstart: %llu, len: %llu\n", start,
+				len);
+		}
 	}
 
 	/* Print dir item with mismatch hash */
-- 
2.24.1