bcachefs: Add checks for invalid snapshot IDs

Previously, we assumed that keys were consistent with the snapshots
btree - but that's not correct as fsck may not have been run or may not
be complete.

This adds checks and error handling when using the in-memory snapshots
table (that mirrors the snapshots btree).

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/btree_trans_commit.c b/fs/bcachefs/btree_trans_commit.c
index 30d69a6d133eec77c76c7e64a5de0d896ad6b732..96669fede7d344d1214389acd11e4e06c0eeedaa 100644 (file)
--- a/fs/bcachefs/btree_trans_commit.c
+++ b/fs/bcachefs/btree_trans_commit.c
@@ -318,7 +318,7 @@ static inline void btree_insert_entry_checks(struct btree_trans *trans,
                !(i->flags & BTREE_UPDATE_INTERNAL_SNAPSHOT_NODE) &&
                test_bit(JOURNAL_REPLAY_DONE, &trans->c->journal.flags) &&
                i->k->k.p.snapshot &&
-               bch2_snapshot_is_internal_node(trans->c, i->k->k.p.snapshot));
+               bch2_snapshot_is_internal_node(trans->c, i->k->k.p.snapshot) > 0);
 }
 
 static __always_inline int bch2_trans_journal_res_get(struct btree_trans *trans,

diff --git a/fs/bcachefs/data_update.c b/fs/bcachefs/data_update.c
index 4150feca42a2e65e63a59234a3e806ebbd09e1ac..b564404dad711bdb46bde1157494e98df3b0beaa 100644 (file)
--- a/fs/bcachefs/data_update.c
+++ b/fs/bcachefs/data_update.c
@@ -14,6 +14,7 @@
 #include "move.h"
 #include "nocow_locking.h"
 #include "rebalance.h"
+#include "snapshot.h"
 #include "subvolume.h"
 #include "trace.h"
 
@@ -509,6 +510,14 @@ int bch2_data_update_init(struct btree_trans *trans,
        unsigned ptrs_locked = 0;
        int ret = 0;
 
+       /*
+        * fs is corrupt  we have a key for a snapshot node that doesn't exist,
+        * and we have to check for this because we go rw before repairing the
+        * snapshots table - just skip it, we can move it later.
+        */
+       if (unlikely(k.k->p.snapshot && !bch2_snapshot_equiv(c, k.k->p.snapshot)))
+               return -BCH_ERR_data_update_done;
+
        bch2_bkey_buf_init(&m->k);
        bch2_bkey_buf_reassemble(&m->k, c, k);
        m->btree_id     = btree_id;

diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h
index af25d8ec60f221d9d935a0ef4ad7aef3641a9e3d..01a79fa3eacb211cb7cd779616f512d427102fd4 100644 (file)
--- a/fs/bcachefs/errcode.h
+++ b/fs/bcachefs/errcode.h
@@ -252,7 +252,8 @@
        x(BCH_ERR_nopromote,            nopromote_in_flight)                    \
        x(BCH_ERR_nopromote,            nopromote_no_writes)                    \
        x(BCH_ERR_nopromote,            nopromote_enomem)                       \
-       x(0,                            need_inode_lock)
+       x(0,                            need_inode_lock)                        \
+       x(0,                            invalid_snapshot_node)
 
 enum bch_errcode {
        BCH_ERR_START           = 2048,

diff --git a/fs/bcachefs/snapshot.c b/fs/bcachefs/snapshot.c
index 9cd71e613dc9b10db8fe26505e58ff4dead58982..4e074136c490793cdd11a42ccf061dcb5cf31176 100644 (file)
--- a/fs/bcachefs/snapshot.c
+++ b/fs/bcachefs/snapshot.c
@@ -93,8 +93,10 @@ static int bch2_snapshot_tree_create(struct btree_trans *trans,
 
 static bool __bch2_snapshot_is_ancestor_early(struct snapshot_table *t, u32 id, u32 ancestor)
 {
-       while (id && id < ancestor)
-               id = __snapshot_t(t, id)->parent;
+       while (id && id < ancestor) {
+               const struct snapshot_t *s = __snapshot_t(t, id);
+               id = s ? s->parent : 0;
+       }
        return id == ancestor;
 }
 
@@ -110,6 +112,8 @@ static bool bch2_snapshot_is_ancestor_early(struct bch_fs *c, u32 id, u32 ancest
 static inline u32 get_ancestor_below(struct snapshot_table *t, u32 id, u32 ancestor)
 {
        const struct snapshot_t *s = __snapshot_t(t, id);
+       if (!s)
+               return 0;
 
        if (s->skip[2] <= ancestor)
                return s->skip[2];

diff --git a/fs/bcachefs/snapshot.h b/fs/bcachefs/snapshot.h
index 8538b7fddfed3830dd8e4b70d4cb1bc78a11e2c1..331f20fd8d03d17c363a7ac060f704c84123d0b5 100644 (file)
--- a/fs/bcachefs/snapshot.h
+++ b/fs/bcachefs/snapshot.h
@@ -48,7 +48,8 @@ static inline const struct snapshot_t *snapshot_t(struct bch_fs *c, u32 id)
 static inline u32 bch2_snapshot_tree(struct bch_fs *c, u32 id)
 {
        rcu_read_lock();
-       id = snapshot_t(c, id)->tree;
+       const struct snapshot_t *s = snapshot_t(c, id);
+       id = s ? s->tree : 0;
        rcu_read_unlock();
 
        return id;
@@ -56,7 +57,8 @@ static inline u32 bch2_snapshot_tree(struct bch_fs *c, u32 id)
 
 static inline u32 __bch2_snapshot_parent_early(struct bch_fs *c, u32 id)
 {
-       return snapshot_t(c, id)->parent;
+       const struct snapshot_t *s = snapshot_t(c, id);
+       return s ? s->parent : 0;
 }
 
 static inline u32 bch2_snapshot_parent_early(struct bch_fs *c, u32 id)
@@ -70,19 +72,19 @@ static inline u32 bch2_snapshot_parent_early(struct bch_fs *c, u32 id)
 
 static inline u32 __bch2_snapshot_parent(struct bch_fs *c, u32 id)
 {
-#ifdef CONFIG_BCACHEFS_DEBUG
-       u32 parent = snapshot_t(c, id)->parent;
+       const struct snapshot_t *s = snapshot_t(c, id);
+       if (!s)
+               return 0;
 
-       if (parent &&
-           snapshot_t(c, id)->depth != snapshot_t(c, parent)->depth + 1)
+       u32 parent = s->parent;
+       if (IS_ENABLED(CONFIG_BCACHEFS_DEBU) &&
+           parent &&
+           s->depth != snapshot_t(c, parent)->depth + 1)
                panic("id %u depth=%u parent %u depth=%u\n",
                      id, snapshot_t(c, id)->depth,
                      parent, snapshot_t(c, parent)->depth);
 
        return parent;
-#else
-       return snapshot_t(c, id)->parent;
-#endif
 }
 
 static inline u32 bch2_snapshot_parent(struct bch_fs *c, u32 id)
@@ -120,7 +122,8 @@ static inline u32 bch2_snapshot_root(struct bch_fs *c, u32 id)
 
 static inline u32 __bch2_snapshot_equiv(struct bch_fs *c, u32 id)
 {
-       return snapshot_t(c, id)->equiv;
+       const struct snapshot_t *s = snapshot_t(c, id);
+       return s ? s->equiv : 0;
 }
 
 static inline u32 bch2_snapshot_equiv(struct bch_fs *c, u32 id)
@@ -137,38 +140,22 @@ static inline bool bch2_snapshot_is_equiv(struct bch_fs *c, u32 id)
        return id == bch2_snapshot_equiv(c, id);
 }
 
-static inline bool bch2_snapshot_is_internal_node(struct bch_fs *c, u32 id)
+static inline int bch2_snapshot_is_internal_node(struct bch_fs *c, u32 id)
 {
-       const struct snapshot_t *s;
-       bool ret;
-
        rcu_read_lock();
-       s = snapshot_t(c, id);
-       ret = s->children[0];
+       const struct snapshot_t *s = snapshot_t(c, id);
+       int ret = s ? s->children[0] : -BCH_ERR_invalid_snapshot_node;
        rcu_read_unlock();
 
        return ret;
 }
 
-static inline u32 bch2_snapshot_is_leaf(struct bch_fs *c, u32 id)
+static inline int bch2_snapshot_is_leaf(struct bch_fs *c, u32 id)
 {
-       return !bch2_snapshot_is_internal_node(c, id);
-}
-
-static inline u32 bch2_snapshot_sibling(struct bch_fs *c, u32 id)
-{
-       const struct snapshot_t *s;
-       u32 parent = __bch2_snapshot_parent(c, id);
-
-       if (!parent)
-               return 0;
-
-       s = snapshot_t(c, __bch2_snapshot_parent(c, id));
-       if (id == s->children[0])
-               return s->children[1];
-       if (id == s->children[1])
-               return s->children[0];
-       return 0;
+       int ret = bch2_snapshot_is_internal_node(c, id);
+       if (ret < 0)
+               return ret;
+       return !ret;
 }
 
 static inline u32 bch2_snapshot_depth(struct bch_fs *c, u32 parent)
@@ -253,7 +240,7 @@ static inline int bch2_key_has_snapshot_overwrites(struct btree_trans *trans,
                                          struct bpos pos)
 {
        if (!btree_type_has_snapshots(id) ||
-           bch2_snapshot_is_leaf(trans->c, pos.snapshot))
+           bch2_snapshot_is_leaf(trans->c, pos.snapshot) > 0)
                return 0;
 
        return __bch2_key_has_snapshot_overwrites(trans, id, pos);