diff options
author | Boris Burkov <boris@bur.io> | 2021-06-30 22:01:48 +0200 |
---|---|---|
committer | David Sterba <dsterba@suse.com> | 2021-08-23 13:19:09 +0200 |
commit | 77eea05e7851d910b7992c8c237a6b5d462050da (patch) | |
tree | b697cf488a1ada71970869a8647ee638674eda02 /fs/btrfs/tree-checker.c | |
parent | btrfs: simplify return values in btrfs_check_raid_min_devices (diff) | |
download | linux-77eea05e7851d910b7992c8c237a6b5d462050da.tar.xz linux-77eea05e7851d910b7992c8c237a6b5d462050da.zip |
btrfs: add ro compat flags to inodes
Currently, inode flags are fully backwards incompatible in btrfs. If we
introduce a new inode flag, then tree-checker will detect it and fail.
This can even cause us to fail to mount entirely. To make it possible to
introduce new flags which can be read-only compatible, like VERITY, we
add new ro flags to btrfs without treating them quite so harshly in
tree-checker. A read-only file system can survive an unexpected flag,
and can be mounted.
As for the implementation, it unfortunately gets a little complicated.
The on-disk representation of the inode, btrfs_inode_item, has an __le64
for flags but the in-memory representation, btrfs_inode, uses a u32.
David Sterba had the nice idea that we could reclaim those wasted 32 bits
on disk and use them for the new ro_compat flags.
It turns out that the tree-checker code which checks for unknown flags
is broken, and ignores the upper 32 bits we are hoping to use. The issue
is that the flags use the literal 1 rather than 1ULL, so the flags are
signed ints, and one of them is specifically (1 << 31). As a result, the
mask which ORs the flags is a negative integer on machines where int is
32 bit twos complement. When tree-checker evaluates the expression:
btrfs_inode_flags(leaf, iitem) & ~BTRFS_INODE_FLAG_MASK)
The mask is something like 0x80000abc, which gets promoted to u64 with
sign extension to 0xffffffff80000abc. Negating that 64 bit mask leaves
all the upper bits zeroed, and we can't detect unexpected flags.
This suggests that we can't use those bits after all. Luckily, we have
good reason to believe that they are zero anyway. Inode flags are
metadata, which is always checksummed, so any bit flips that would
introduce 1s would cause a checksum failure anyway (excluding the
improbable case of the checksum getting corrupted exactly badly).
Further, unless the 1 << 31 flag is used, the cast to u64 of the 32 bit
inode flag should preserve its value and not add leading zeroes
(at least for twos complement). The only place that flag
(BTRFS_INODE_ROOT_ITEM_INIT) is used is in a special inode embedded in
the root item, and indeed for that inode we see 0xffffffff80000000 as
the flags on disk. However, that inode is never seen by tree checker,
nor is it used in a context where verity might be meaningful.
Theoretically, a future ro flag might cause trouble on that inode, so we
should proactively clean up that mess before it does.
With the introduction of the new ro flags, keep two separate unsigned
masks and check them against the appropriate u32. Since we no longer run
afoul of sign extension, this also stops writing out 0xffffffff80000000
in root_item inodes going forward.
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Diffstat (limited to 'fs/btrfs/tree-checker.c')
-rw-r--r-- | fs/btrfs/tree-checker.c | 17 |
1 files changed, 13 insertions, 4 deletions
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c index 7ba94b683ee3..7733e8ac0a69 100644 --- a/fs/btrfs/tree-checker.c +++ b/fs/btrfs/tree-checker.c @@ -24,6 +24,7 @@ #include "compression.h" #include "volumes.h" #include "misc.h" +#include "btrfs_inode.h" /* * Error message should follow the following format: @@ -1008,6 +1009,8 @@ static int check_inode_item(struct extent_buffer *leaf, u32 valid_mask = (S_IFMT | S_ISUID | S_ISGID | S_ISVTX | 0777); u32 mode; int ret; + u32 flags; + u32 ro_flags; ret = check_inode_key(leaf, key, slot); if (unlikely(ret < 0)) @@ -1063,11 +1066,17 @@ static int check_inode_item(struct extent_buffer *leaf, btrfs_inode_nlink(leaf, iitem)); return -EUCLEAN; } - if (unlikely(btrfs_inode_flags(leaf, iitem) & ~BTRFS_INODE_FLAG_MASK)) { + btrfs_inode_split_flags(btrfs_inode_flags(leaf, iitem), &flags, &ro_flags); + if (unlikely(flags & ~BTRFS_INODE_FLAG_MASK)) { inode_item_err(leaf, slot, - "unknown flags detected: 0x%llx", - btrfs_inode_flags(leaf, iitem) & - ~BTRFS_INODE_FLAG_MASK); + "unknown incompat flags detected: 0x%x", flags); + return -EUCLEAN; + } + if (unlikely(!sb_rdonly(fs_info->sb) && + (ro_flags & ~BTRFS_INODE_RO_FLAG_MASK))) { + inode_item_err(leaf, slot, + "unknown ro-compat flags detected on writeable mount: 0x%x", + ro_flags); return -EUCLEAN; } return 0; |