bcachefs: Stricter checks on "key allowed in this btree"

Syzbot managed to come up with a filesystem where check/repair got
rather confused at finding a reflink pointer in the inodes btree.

Currently, the "key allowed in this btree" checks only apply at commit
time, not read time - for forwards compatibility. It seems this is too
loose.

Now, strict key type allowed checks apply:
 - at commit time (no forward compatibility issues)
 - for btree node pointers
 - if it's a known btree, known key type, and the key type has the
   "BKEY_TYPE_strict_btree_checks" flag.

This means we still have the option of using generic key types - e.g.
KEY_TYPE_error, KEY_TYPE_set - on more existing btrees in the future,
while most key types that are intended for only a specific btree get
stricter checks.

Reported-by: syzbot+baee8591f336cab0958b@syzkaller.appspotmail.com
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/bcachefs_format.h b/fs/bcachefs/bcachefs_format.h
index a3db328dee31f5e1c7920ca76abd66679431a9d0..377f04d92a1443ffc67b8334315c929a360c49ff 100644 (file)
--- a/fs/bcachefs/bcachefs_format.h
+++ b/fs/bcachefs/bcachefs_format.h
@@ -366,6 +366,10 @@ static inline void bkey_init(struct bkey *k)
 #define __BKEY_PADDED(key, pad)                                        \
        struct bkey_i key; __u64 key ## _pad[pad]
 
+enum bch_bkey_type_flags {
+       BKEY_TYPE_strict_btree_checks   = BIT(0),
+};
+
 /*
  * - DELETED keys are used internally to mark keys that should be ignored but
  *   override keys in composition order.  Their version number is ignored.
@@ -383,46 +387,46 @@ static inline void bkey_init(struct bkey *k)
  *
  * - WHITEOUT: for hash table btrees
  */
-#define BCH_BKEY_TYPES()                               \
-       x(deleted,              0)                      \
-       x(whiteout,             1)                      \
-       x(error,                2)                      \
-       x(cookie,               3)                      \
-       x(hash_whiteout,        4)                      \
-       x(btree_ptr,            5)                      \
-       x(extent,               6)                      \
-       x(reservation,          7)                      \
-       x(inode,                8)                      \
-       x(inode_generation,     9)                      \
-       x(dirent,               10)                     \
-       x(xattr,                11)                     \
-       x(alloc,                12)                     \
-       x(quota,                13)                     \
-       x(stripe,               14)                     \
-       x(reflink_p,            15)                     \
-       x(reflink_v,            16)                     \
-       x(inline_data,          17)                     \
-       x(btree_ptr_v2,         18)                     \
-       x(indirect_inline_data, 19)                     \
-       x(alloc_v2,             20)                     \
-       x(subvolume,            21)                     \
-       x(snapshot,             22)                     \
-       x(inode_v2,             23)                     \
-       x(alloc_v3,             24)                     \
-       x(set,                  25)                     \
-       x(lru,                  26)                     \
-       x(alloc_v4,             27)                     \
-       x(backpointer,          28)                     \
-       x(inode_v3,             29)                     \
-       x(bucket_gens,          30)                     \
-       x(snapshot_tree,        31)                     \
-       x(logged_op_truncate,   32)                     \
-       x(logged_op_finsert,    33)                     \
-       x(accounting,           34)                     \
-       x(inode_alloc_cursor,   35)
+#define BCH_BKEY_TYPES()                                               \
+       x(deleted,              0,      0)                              \
+       x(whiteout,             1,      0)                              \
+       x(error,                2,      0)                              \
+       x(cookie,               3,      0)                              \
+       x(hash_whiteout,        4,      BKEY_TYPE_strict_btree_checks)  \
+       x(btree_ptr,            5,      BKEY_TYPE_strict_btree_checks)  \
+       x(extent,               6,      BKEY_TYPE_strict_btree_checks)  \
+       x(reservation,          7,      BKEY_TYPE_strict_btree_checks)  \
+       x(inode,                8,      BKEY_TYPE_strict_btree_checks)  \
+       x(inode_generation,     9,      BKEY_TYPE_strict_btree_checks)  \
+       x(dirent,               10,     BKEY_TYPE_strict_btree_checks)  \
+       x(xattr,                11,     BKEY_TYPE_strict_btree_checks)  \
+       x(alloc,                12,     BKEY_TYPE_strict_btree_checks)  \
+       x(quota,                13,     BKEY_TYPE_strict_btree_checks)  \
+       x(stripe,               14,     BKEY_TYPE_strict_btree_checks)  \
+       x(reflink_p,            15,     BKEY_TYPE_strict_btree_checks)  \
+       x(reflink_v,            16,     BKEY_TYPE_strict_btree_checks)  \
+       x(inline_data,          17,     BKEY_TYPE_strict_btree_checks)  \
+       x(btree_ptr_v2,         18,     BKEY_TYPE_strict_btree_checks)  \
+       x(indirect_inline_data, 19,     BKEY_TYPE_strict_btree_checks)  \
+       x(alloc_v2,             20,     BKEY_TYPE_strict_btree_checks)  \
+       x(subvolume,            21,     BKEY_TYPE_strict_btree_checks)  \
+       x(snapshot,             22,     BKEY_TYPE_strict_btree_checks)  \
+       x(inode_v2,             23,     BKEY_TYPE_strict_btree_checks)  \
+       x(alloc_v3,             24,     BKEY_TYPE_strict_btree_checks)  \
+       x(set,                  25,     0)                              \
+       x(lru,                  26,     BKEY_TYPE_strict_btree_checks)  \
+       x(alloc_v4,             27,     BKEY_TYPE_strict_btree_checks)  \
+       x(backpointer,          28,     BKEY_TYPE_strict_btree_checks)  \
+       x(inode_v3,             29,     BKEY_TYPE_strict_btree_checks)  \
+       x(bucket_gens,          30,     BKEY_TYPE_strict_btree_checks)  \
+       x(snapshot_tree,        31,     BKEY_TYPE_strict_btree_checks)  \
+       x(logged_op_truncate,   32,     BKEY_TYPE_strict_btree_checks)  \
+       x(logged_op_finsert,    33,     BKEY_TYPE_strict_btree_checks)  \
+       x(accounting,           34,     BKEY_TYPE_strict_btree_checks)  \
+       x(inode_alloc_cursor,   35,     BKEY_TYPE_strict_btree_checks)
 
 enum bch_bkey_type {
-#define x(name, nr) KEY_TYPE_##name    = nr,
+#define x(name, nr, ...) KEY_TYPE_##name       = nr,
        BCH_BKEY_TYPES()
 #undef x
        KEY_TYPE_MAX,

diff --git a/fs/bcachefs/bkey_methods.c b/fs/bcachefs/bkey_methods.c
index 15c93576b5c2041c62f3fa6884e48d8271ddc47a..00d05ccfaf73bb7032b8b197b109ce6349e8a2b1 100644 (file)
--- a/fs/bcachefs/bkey_methods.c
+++ b/fs/bcachefs/bkey_methods.c
@@ -21,7 +21,7 @@
 #include "xattr.h"
 
 const char * const bch2_bkey_types[] = {
-#define x(name, nr) #name,
+#define x(name, nr, ...) #name,
        BCH_BKEY_TYPES()
 #undef x
        NULL
@@ -115,7 +115,7 @@ static bool key_type_set_merge(struct bch_fs *c, struct bkey_s l, struct bkey_s_
 })
 
 const struct bkey_ops bch2_bkey_ops[] = {
-#define x(name, nr) [KEY_TYPE_##name]  = bch2_bkey_ops_##name,
+#define x(name, nr, ...) [KEY_TYPE_##name]     = bch2_bkey_ops_##name,
        BCH_BKEY_TYPES()
 #undef x
 };
@@ -155,6 +155,12 @@ static u64 bch2_key_types_allowed[] = {
 #undef x
 };
 
+static const enum bch_bkey_type_flags bch2_bkey_type_flags[] = {
+#define x(name, nr, flags)     [KEY_TYPE_##name] = flags,
+       BCH_BKEY_TYPES()
+#undef x
+};
+
 const char *bch2_btree_node_type_str(enum btree_node_type type)
 {
        return type == BKEY_TYPE_btree ? "internal btree node" : bch2_btree_id_str(type - 1);
@@ -177,8 +183,18 @@ int __bch2_bkey_validate(struct bch_fs *c, struct bkey_s_c k,
        if (type >= BKEY_TYPE_NR)
                return 0;
 
-       bkey_fsck_err_on(k.k->type < KEY_TYPE_MAX &&
-                        (type == BKEY_TYPE_btree || (from.flags & BCH_VALIDATE_commit)) &&
+       enum bch_bkey_type_flags bkey_flags = k.k->type < KEY_TYPE_MAX
+               ? bch2_bkey_type_flags[k.k->type]
+               : 0;
+
+       bool strict_key_type_allowed =
+               (from.flags & BCH_VALIDATE_commit) ||
+               type == BKEY_TYPE_btree ||
+               (from.btree < BTREE_ID_NR &&
+                (bkey_flags & BKEY_TYPE_strict_btree_checks));
+
+       bkey_fsck_err_on(strict_key_type_allowed &&
+                        k.k->type < KEY_TYPE_MAX &&
                         !(bch2_key_types_allowed[type] & BIT_ULL(k.k->type)),
                         c, bkey_invalid_type_for_btree,
                         "invalid key type for btree %s (%s)",