-
Notifications
You must be signed in to change notification settings - Fork 265
btrfs-progs: check: properly fix missing INODE_REF cases #1070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
adam900710
wants to merge
5
commits into
kdave:devel
Choose a base branch
from
adam900710:repair_missing_inode_ref
base: devel
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+87
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There is a bug report that a bitflip corrupted one tree block, causing a corruption that can not be repaired by btrfs-check. The corruption looks like this: item 10 key (730455 INODE_ITEM 0) itemoff 15456 itemsize 160 generation 7280 transid 9794 size 13829 nbytes 16384 block group 0 mode 100600 links 1 uid 1000 gid 1000 rdev 0 sequence 11 flags 0x0(none) atime 1765397443.29231914 (2025-12-11 06:40:43) ctime 1764798591.882909548 (2025-12-04 08:19:51) mtime 1764798591.882909548 (2025-12-04 08:19:51) otime 1764712848.413821734 (2025-12-03 08:30:48) item 11 key (730455 UNKNOWN.8 1924) itemoff 15406 itemsize 50 Note item 11, it has a unknown key, but the itemsize indicates it's an INODE_REF with 40 name_len (which matches the DIR_ITEM). bin(BTRFS_INODE_REF_KEY) = 0b1100 bin(8) = 0b1000 So it's indeed a bitflip. At least we should report such unknown inode key types as errors. The lowmem mode is already doing such report, although not treating them as an error. The original mode just ignores them completely. So this patch enhance btrfs check to: - Report such unknown item and treat them as error for the original mode - Treat such unknown item as error for the lowmem mode Reported-by: mikkel+btrfs@mikkel.cc Link: https://lore.kernel.org/linux-btrfs/5d5e344e-96be-4436-9a58-d60ba14fdb4f@gmx.com/T/#me22cef92653e660e88a4c005b10f5201a8fd83ac Signed-off-by: Qu Wenruo <wqu@suse.com>
The image has the following corrupted key in fs tree: item 4 key (257 INODE_ITEM 0) itemoff 15879 itemsize 160 generation 9 transid 9 size 0 nbytes 0 block group 0 mode 100644 links 1 uid 0 gid 0 rdev 0 sequence 1 flags 0x0(none) item 5 key (257 UNKNOWN.8 256) itemoff 15879 itemsize 0 <<< item 6 key (257 INODE_REF 256) itemoff 15863 itemsize 16 index 2 namelen 6 name: foobar This is inspired by a real world memory bitflip, which lead to a bitflip from 12 to 8, causing the above unknown key type in a subvolume. Although we will need to properly enhance btrfs-check to handle such case better, let's start from detecting and report such unknown keys as an error. The image is created by inserting an empty item with above unknown key type. Signed-off-by: Qu Wenruo <wqu@suse.com>
Currently if the original mode of btrfs-check hits a missing INODE_REF, but with valid DIR_INDEX/DIR_ITEM, then it will repair it by deleting the valid DIR_INDEX, which will just make things worse. Add a dedicated repair for missing INODE_REF which will just add back the missing INODE_REF, properly fixing the problem other than making it worse. Signed-off-by: Qu Wenruo <wqu@suse.com>
[BUG] Although lowmem has the logical to repair one missing INODE_REF/DIR_INDEX/DIR_ITEM, there is a catch in missing INODE_REF. If we're checking an DIR_ITEM (which we normally hits first), and failed to find the INODE_REF, we are in a situation where there is no @index to delete the original DIR_INDEX/DIR_ITEM pair. This can cause further damage to the fs, without really improving anything. [CAUSE] There is a minimal example where there is missing INODE_ITEM: item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160 generation 3 transid 9 size 12 nbytes 16384 block group 0 mode 40755 links 1 uid 0 gid 0 rdev 0 sequence 1 flags 0x0(none) item 1 key (256 INODE_REF 256) itemoff 16111 itemsize 12 index 0 namelen 2 name: .. item 2 key (256 DIR_ITEM 496027801) itemoff 16075 itemsize 36 location key (257 INODE_ITEM 0) type FILE transid 9 data_len 0 name_len 6 name: foobar item 3 key (256 DIR_INDEX 2) itemoff 16039 itemsize 36 location key (257 INODE_ITEM 0) type FILE transid 9 data_len 0 name_len 6 name: foobar item 4 key (257 INODE_ITEM 0) itemoff 15879 itemsize 160 generation 9 transid 9 size 0 nbytes 0 block group 0 mode 100644 links 1 uid 0 gid 0 rdev 0 sequence 1 flags 0x0(none) For above case, if we're check the DIR_ITEM, we can find the INODE_ITEM but not the INODE_REF. So we need to repair it, but at this stage we haven't checked DIR_INDEX, this means we have no idea which index we should delete, leaving the default @index as (-1). If we call btrfs_unlink() with that (-1) as index, it will not delete the (256 DIR_INDEX 2) one, then re-add a link using -1 as index, causing more damage. [FIX] Before calling btrfs_unlink(), do an extra check on if we're called from DIR_ITEM checks (aka, @index is -1) and we only detected a missing INODE_REF yet. If so, do find_dir_index() call to determine the DIR_INDEX, if that failed it means we have missing both DIR_INDEX and INODE_REF, thus should remove the lone DIR_ITEM instead. With this enhancement, lowmem mode can properly fix the missing INODE_REF by adding it back. Signed-off-by: Qu Wenruo <wqu@suse.com>
The image is created by removing the INODE_REF for inode 257. Now both original and lowmem mode should be able to repair it. Signed-off-by: Qu Wenruo <wqu@suse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is inspired by a recent corruption report that an INODE_REF key hits a
bitflip and becomes UNKNOWN.8 key.
That cause missing INODE_REF, and unfortunately neither original nor
lowmem mode can properly handle it.
For the original mode it just completely lacks the handling for such
case, thus fallback to delete the good DIR_INDEX, making things
worse.
For the lowmem mode, although it is trying to keep the good
DIR_INDEX/DIR_ITEM, it doesn't handle index number search, thus using
the default (-1) as index, causing incorrect new link to be added.
Fix both modes, and add a new test case for it.
Please note that this series is based on the previous patchset: https://lore.kernel.org/linux-btrfs/cover.1765876829.git.wqu@suse.com/T/#t