[v2,4/6] btrfs-progs: check/lowmem: Repair bad imode early
diff mbox series

Message ID 20190905075800.1633-5-wqu@suse.com
State New
Headers show
Series
  • btrfs-progs: check: Repair invalid inode mode in
Related show

Commit Message

Qu WenRuo Sept. 5, 2019, 7:57 a.m. UTC
For lowmem mode, if we hit a bad inode mode, normally it is reported
when we checking the DIR_INDEX/DIR_ITEM of the parent inode.

If we didn't repair at that timing, the error will be recorded even we
fixed it later.

So this patch will check for INODE_ITEM_MISMATCH error type, and if it's
really caused by invalid imode, repair it and clear the error.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Nikolay Borisov Sept. 9, 2019, 2:55 p.m. UTC | #1
On 5.09.19 г. 10:57 ч., Qu Wenruo wrote:
> For lowmem mode, if we hit a bad inode mode, normally it is reported
> when we checking the DIR_INDEX/DIR_ITEM of the parent inode.
> 
> If we didn't repair at that timing, the error will be recorded even we
> fixed it later.
> 
> So this patch will check for INODE_ITEM_MISMATCH error type, and if it's
> really caused by invalid imode, repair it and clear the error.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/mode-lowmem.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 5f7f101daab1..5d0c520217fa 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1550,6 +1550,35 @@ static int lowmem_delete_corrupted_dir_item(struct btrfs_root *root,
>  	return ret;
>  }
>  
> +static int try_repair_imode(struct btrfs_root *root, u64 ino)
> +{
> +	struct btrfs_inode_item *iitem;
> +	struct btrfs_path path;
> +	struct btrfs_key key;
> +	int ret;
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_INODE_ITEM_KEY;
> +	key.offset = 0;
> +	btrfs_init_path(&path);
> +
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret < 0)
> +		goto out;
> +	iitem = btrfs_item_ptr(path.nodes[0], path.slots[0],
> +			       struct btrfs_inode_item);
> +	if (!is_valid_imode(btrfs_inode_mode(path.nodes[0], iitem))) {

INODE_ITEM_MISMATCH is only set if:

1. The first inode item is not a directory (it should always be)
2. There is a mismatch between the filetype in the inode item and in the
dir/index item pointing to it.

By using this check you could possibly miss case 1 above, if the first
inode has a valid type e.g. regular whereas it should really be a
directory.

I'm not entirely sure whether it makes sense to handle this situation,
since admittedly, it would require a perfect storm to get such a
corruption.


<snip>
Qu Wenruo Sept. 10, 2019, 2:35 a.m. UTC | #2
On 2019/9/9 下午10:55, Nikolay Borisov wrote:
>
>
> On 5.09.19 г. 10:57 ч., Qu Wenruo wrote:
>> For lowmem mode, if we hit a bad inode mode, normally it is reported
>> when we checking the DIR_INDEX/DIR_ITEM of the parent inode.
>>
>> If we didn't repair at that timing, the error will be recorded even we
>> fixed it later.
>>
>> So this patch will check for INODE_ITEM_MISMATCH error type, and if it's
>> really caused by invalid imode, repair it and clear the error.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/mode-lowmem.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 5f7f101daab1..5d0c520217fa 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1550,6 +1550,35 @@ static int lowmem_delete_corrupted_dir_item(struct btrfs_root *root,
>>  	return ret;
>>  }
>>
>> +static int try_repair_imode(struct btrfs_root *root, u64 ino)
>> +{
>> +	struct btrfs_inode_item *iitem;
>> +	struct btrfs_path path;
>> +	struct btrfs_key key;
>> +	int ret;
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_INODE_ITEM_KEY;
>> +	key.offset = 0;
>> +	btrfs_init_path(&path);
>> +
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret > 0)
>> +		ret = -ENOENT;
>> +	if (ret < 0)
>> +		goto out;
>> +	iitem = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> +			       struct btrfs_inode_item);
>> +	if (!is_valid_imode(btrfs_inode_mode(path.nodes[0], iitem))) {
>
> INODE_ITEM_MISMATCH is only set if:
>
> 1. The first inode item is not a directory (it should always be)
> 2. There is a mismatch between the filetype in the inode item and in the
> dir/index item pointing to it.
>
> By using this check you could possibly miss case 1 above, if the first
> inode has a valid type e.g. regular whereas it should really be a
> directory.

That's indeed a special rare case.

>
> I'm not entirely sure whether it makes sense to handle this situation,
> since admittedly, it would require a perfect storm to get such a
> corruption.

Let's wait to see if this could happen.
If it happened, we can easily modify the code to handle it anyway.

On the other hand, let's just focus on the real corruption where some
old 2014 kernel screwed up the imode of some inodes.

Thanks,
Qu

>
>
> <snip>
>

Patch
diff mbox series

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 5f7f101daab1..5d0c520217fa 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1550,6 +1550,35 @@  static int lowmem_delete_corrupted_dir_item(struct btrfs_root *root,
 	return ret;
 }
 
+static int try_repair_imode(struct btrfs_root *root, u64 ino)
+{
+	struct btrfs_inode_item *iitem;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	int ret;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	btrfs_init_path(&path);
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+	iitem = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			       struct btrfs_inode_item);
+	if (!is_valid_imode(btrfs_inode_mode(path.nodes[0], iitem))) {
+		ret = repair_imode_common(root, &path);
+	} else {
+		ret = -ENOTTY;
+	}
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
 /*
  * Call repair_inode_item_missing and repair_ternary_lowmem to repair
  *
@@ -1574,6 +1603,16 @@  static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *di_key,
 			err &= ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING);
 	}
 
+	if (err & INODE_ITEM_MISMATCH) {
+		/*
+		 * INODE_ITEM mismatch can be caused by bad imode,
+		 * so check if it's a bad imode, then repair if possible.
+		 */
+		ret = try_repair_imode(root, ino);
+		if (!ret)
+			err &= ~INODE_ITEM_MISMATCH;
+	}
+
 	if (err & ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
 		ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
 					    name_len, filetype, err);