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

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

[Josef Bacik]

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/2] 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 assume the first extent we find is the first valid spot, so
we don't actually add a hole entry if there is no extent at offset 0.
Fix this by setting our extent_start and extent_end to 0 so we can
properly catch the corruption.

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

diff --git a/check/main.c b/check/main.c
index 4115049a..4ce5a63c 100644
--- a/check/main.c
+++ b/check/main.c
@@ -1553,8 +1553,8 @@ static int process_file_extent(struct btrfs_root *root,
 	rec->found_file_extent = 1;
 
 	if (rec->extent_start == (u64)-1) {
-		rec->extent_start = key->offset;
-		rec->extent_end = key->offset;
+		rec->extent_start = 0;
+		rec->extent_end = 0;
 	}
 
 	if (rec->extent_end > key->offset)
-- 
2.24.1

[PATCH 2/2] 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

Re: [PATCH 0/2] Fix btrfs check handling of missing file extents

[David Sterba]

On Thu, Jan 30, 2020 at 03:47:34PM -0500, Josef Bacik wrote:
> 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,

Added to devel, thanks.

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

[David Sterba]

On Thu, Jan 30, 2020 at 03:47:35PM -0500, Josef Bacik wrote:
> 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 assume the first extent we find is the first valid spot, so
> we don't actually add a hole entry if there is no extent at offset 0.
> Fix this by setting our extent_start and extent_end to 0 so we can
> properly catch the corruption.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

This patch does not pass tests:

    [TEST/fsck]   020-extent-ref-cases                                                                   
failed: /labs/dsterba/gits/btrfs-progs/btrfs check ./keyed_data_ref_with_shared_leaf.img.restored        
test failed for case 020-extent-ref-cases