bcachefs: Check for invalid bucket from bucket_gen(), gc_bucket()

Turn more asserts into proper recoverable error paths.

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

diff --git a/fs/bcachefs/alloc_background.c b/fs/bcachefs/alloc_background.c
index 346cd91f91f9998ca73cddc309acd76c4047b6f3..c4b6601f5b7482bb55931b66e0f66a26dc18a62f 100644 (file)
--- a/fs/bcachefs/alloc_background.c
+++ b/fs/bcachefs/alloc_background.c
@@ -741,6 +741,7 @@ int bch2_trigger_alloc(struct btree_trans *trans,
                       enum btree_iter_update_trigger_flags flags)
 {
        struct bch_fs *c = trans->c;
+       struct printbuf buf = PRINTBUF;
        int ret = 0;
 
        struct bch_dev *ca = bch2_dev_bucket_tryget(c, new.k->p);
@@ -860,8 +861,14 @@ int bch2_trigger_alloc(struct btree_trans *trans,
                }
 
                percpu_down_read(&c->mark_lock);
-               if (new_a->gen != old_a->gen)
-                       *bucket_gen(ca, new.k->p.offset) = new_a->gen;
+               if (new_a->gen != old_a->gen) {
+                       u8 *gen = bucket_gen(ca, new.k->p.offset);
+                       if (unlikely(!gen)) {
+                               percpu_up_read(&c->mark_lock);
+                               goto invalid_bucket;
+                       }
+                       *gen = new_a->gen;
+               }
 
                bch2_dev_usage_update(c, ca, old_a, new_a, journal_seq, false);
                percpu_up_read(&c->mark_lock);
@@ -895,6 +902,11 @@ int bch2_trigger_alloc(struct btree_trans *trans,
 
                percpu_down_read(&c->mark_lock);
                struct bucket *g = gc_bucket(ca, new.k->p.offset);
+               if (unlikely(!g)) {
+                       percpu_up_read(&c->mark_lock);
+                       goto invalid_bucket;
+               }
+               g->gen_valid    = 1;
 
                bucket_lock(g);
 
@@ -910,8 +922,14 @@ int bch2_trigger_alloc(struct btree_trans *trans,
                percpu_up_read(&c->mark_lock);
        }
 err:
+       printbuf_exit(&buf);
        bch2_dev_put(ca);
        return ret;
+invalid_bucket:
+       bch2_fs_inconsistent(c, "reference to invalid bucket\n  %s",
+                            (bch2_bkey_val_to_text(&buf, c, new.s_c), buf.buf));
+       ret = -EIO;
+       goto err;
 }
 
 /*

diff --git a/fs/bcachefs/btree_gc.c b/fs/bcachefs/btree_gc.c
index 130a0131cd73e41b79046e74f49b3c4bdfa58e07..0e477a926579a77a1736483859f5469dd8d29d7d 100644 (file)
--- a/fs/bcachefs/btree_gc.c
+++ b/fs/bcachefs/btree_gc.c
@@ -874,6 +874,9 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
        const struct bch_alloc_v4 *old;
        int ret;
 
+       if (!bucket_valid(ca, k.k->p.offset))
+               return 0;
+
        old = bch2_alloc_to_v4(k, &old_convert);
        gc = new = *old;
 
@@ -1005,12 +1008,14 @@ static int bch2_gc_alloc_start(struct bch_fs *c)
                                continue;
                        }
 
-                       struct bch_alloc_v4 a_convert;
-                       const struct bch_alloc_v4 *a = bch2_alloc_to_v4(k, &a_convert);
+                       if (bucket_valid(ca, k.k->p.offset)) {
+                               struct bch_alloc_v4 a_convert;
+                               const struct bch_alloc_v4 *a = bch2_alloc_to_v4(k, &a_convert);
 
-                       struct bucket *g = gc_bucket(ca, k.k->p.offset);
-                       g->gen_valid    = 1;
-                       g->gen          = a->gen;
+                               struct bucket *g = gc_bucket(ca, k.k->p.offset);
+                               g->gen_valid    = 1;
+                               g->gen          = a->gen;
+                       }
                        0;
                })));
        bch2_dev_put(ca);

diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index 99a7824d0de2d8c070f0e8fa98dd27f810f1f416..743d57eba760704b7d2d5b0440983041f46904f1 100644 (file)
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -488,6 +488,17 @@ static int bch2_check_fix_ptr(struct btree_trans *trans,
        }
 
        struct bucket *g = PTR_GC_BUCKET(ca, &p.ptr);
+       if (!g) {
+               if (fsck_err(c, ptr_to_invalid_device,
+                            "pointer to invalid bucket on device %u\n"
+                            "while marking %s",
+                            p.ptr.dev,
+                            (printbuf_reset(&buf),
+                             bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
+                       *do_update = true;
+               goto out;
+       }
+
        enum bch_data_type data_type = bch2_bkey_ptr_data_type(k, p, entry);
 
        if (fsck_err_on(!g->gen_valid,
@@ -577,8 +588,8 @@ static int bch2_check_fix_ptr(struct btree_trans *trans,
        if (p.has_ec) {
                struct gc_stripe *m = genradix_ptr(&c->gc_stripes, p.ec.idx);
 
-               if (fsck_err_on(!m || !m->alive, c,
-                               ptr_to_missing_stripe,
+               if (fsck_err_on(!m || !m->alive,
+                               c, ptr_to_missing_stripe,
                                "pointer to nonexistent stripe %llu\n"
                                "while marking %s",
                                (u64) p.ec.idx,
@@ -586,8 +597,8 @@ static int bch2_check_fix_ptr(struct btree_trans *trans,
                                 bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
                        *do_update = true;
 
-               if (fsck_err_on(m && m->alive && !bch2_ptr_matches_stripe_m(m, p), c,
-                               ptr_to_incorrect_stripe,
+               if (fsck_err_on(m && m->alive && !bch2_ptr_matches_stripe_m(m, p),
+                               c, ptr_to_incorrect_stripe,
                                "pointer does not match stripe %llu\n"
                                "while marking %s",
                                (u64) p.ec.idx,
@@ -1004,6 +1015,7 @@ static int bch2_trigger_pointer(struct btree_trans *trans,
                        enum btree_iter_update_trigger_flags flags)
 {
        bool insert = !(flags & BTREE_TRIGGER_overwrite);
+       struct printbuf buf = PRINTBUF;
        int ret = 0;
 
        struct bch_fs *c = trans->c;
@@ -1036,6 +1048,13 @@ static int bch2_trigger_pointer(struct btree_trans *trans,
        if (flags & BTREE_TRIGGER_gc) {
                percpu_down_read(&c->mark_lock);
                struct bucket *g = gc_bucket(ca, bucket.offset);
+               if (bch2_fs_inconsistent_on(!g, c, "reference to invalid bucket on device %u\n  %s",
+                                           p.ptr.dev,
+                                           (bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
+                       ret = -EIO;
+                       goto err_unlock;
+               }
+
                bucket_lock(g);
                struct bch_alloc_v4 old = bucket_m_to_alloc(*g), new = old;
                ret = __mark_pointer(trans, ca, k, &p.ptr, *sectors, bp.data_type, &new);
@@ -1044,10 +1063,12 @@ static int bch2_trigger_pointer(struct btree_trans *trans,
                        bch2_dev_usage_update(c, ca, &old, &new, 0, true);
                }
                bucket_unlock(g);
+err_unlock:
                percpu_up_read(&c->mark_lock);
        }
 err:
        bch2_dev_put(ca);
+       printbuf_exit(&buf);
        return ret;
 }
 
@@ -1335,10 +1356,11 @@ static int bch2_mark_metadata_bucket(struct bch_fs *c, struct bch_dev *ca,
                        u64 b, enum bch_data_type data_type, unsigned sectors,
                        enum btree_iter_update_trigger_flags flags)
 {
-       int ret = 0;
-
        percpu_down_read(&c->mark_lock);
        struct bucket *g = gc_bucket(ca, b);
+       if (bch2_fs_inconsistent_on(!g, c, "reference to invalid bucket on device %u when marking metadata type %s",
+                                   ca->dev_idx, bch2_data_type_str(data_type)))
+               goto err_unlock;
 
        bucket_lock(g);
        struct bch_alloc_v4 old = bucket_m_to_alloc(*g);
@@ -1347,29 +1369,27 @@ static int bch2_mark_metadata_bucket(struct bch_fs *c, struct bch_dev *ca,
                        g->data_type != data_type, c,
                        "different types of data in same bucket: %s, %s",
                        bch2_data_type_str(g->data_type),
-                       bch2_data_type_str(data_type))) {
-               ret = -EIO;
+                       bch2_data_type_str(data_type)))
                goto err;
-       }
 
        if (bch2_fs_inconsistent_on((u64) g->dirty_sectors + sectors > ca->mi.bucket_size, c,
                        "bucket %u:%llu gen %u data type %s sector count overflow: %u + %u > bucket size",
                        ca->dev_idx, b, g->gen,
                        bch2_data_type_str(g->data_type ?: data_type),
-                       g->dirty_sectors, sectors)) {
-               ret = -EIO;
+                       g->dirty_sectors, sectors))
                goto err;
-       }
 
        g->data_type = data_type;
        g->dirty_sectors += sectors;
        struct bch_alloc_v4 new = bucket_m_to_alloc(*g);
+       bch2_dev_usage_update(c, ca, &old, &new, 0, true);
+       percpu_up_read(&c->mark_lock);
+       return 0;
 err:
        bucket_unlock(g);
-       if (!ret)
-               bch2_dev_usage_update(c, ca, &old, &new, 0, true);
+err_unlock:
        percpu_up_read(&c->mark_lock);
-       return ret;
+       return -EIO;
 }
 
 int bch2_trans_mark_metadata_bucket(struct btree_trans *trans,

diff --git a/fs/bcachefs/buckets.h b/fs/bcachefs/buckets.h
index e1a5e3082bbfb8d2d0316d71ba1288ebace93f40..80ee0be9793e605242f21122cadce2ab01cefd99 100644 (file)
--- a/fs/bcachefs/buckets.h
+++ b/fs/bcachefs/buckets.h
@@ -172,19 +172,22 @@ static inline int gen_after(u8 a, u8 b)
        return r > 0 ? r : 0;
 }
 
-static inline u8 dev_ptr_stale_rcu(struct bch_dev *ca, const struct bch_extent_ptr *ptr)
+static inline int dev_ptr_stale_rcu(struct bch_dev *ca, const struct bch_extent_ptr *ptr)
 {
-       return gen_after(*bucket_gen(ca, PTR_BUCKET_NR(ca, ptr)), ptr->gen);
+       u8 *gen = bucket_gen(ca, PTR_BUCKET_NR(ca, ptr));
+       if (!gen)
+               return -1;
+       return gen_after(*gen, ptr->gen);
 }
 
 /**
  * dev_ptr_stale() - check if a pointer points into a bucket that has been
  * invalidated.
  */
-static inline u8 dev_ptr_stale(struct bch_dev *ca, const struct bch_extent_ptr *ptr)
+static inline int dev_ptr_stale(struct bch_dev *ca, const struct bch_extent_ptr *ptr)
 {
        rcu_read_lock();
-       u8 ret = dev_ptr_stale_rcu(ca, ptr);
+       int ret = dev_ptr_stale_rcu(ca, ptr);
        rcu_read_unlock();
 
        return ret;

diff --git a/fs/bcachefs/ec.c b/fs/bcachefs/ec.c
index d8b9beca377627200d1bda8ffd75136ff5fdaa25..83e279d41829d0b2e5ab797cac44eb504272d886 100644 (file)
--- a/fs/bcachefs/ec.c
+++ b/fs/bcachefs/ec.c
@@ -268,6 +268,7 @@ static int mark_stripe_bucket(struct btree_trans *trans,
 {
        struct bch_fs *c = trans->c;
        const struct bch_extent_ptr *ptr = s.v->ptrs + ptr_idx;
+       struct printbuf buf = PRINTBUF;
        int ret = 0;
 
        struct bch_dev *ca = bch2_dev_tryget(c, ptr->dev);
@@ -289,6 +290,13 @@ static int mark_stripe_bucket(struct btree_trans *trans,
        if (flags & BTREE_TRIGGER_gc) {
                percpu_down_read(&c->mark_lock);
                struct bucket *g = gc_bucket(ca, bucket.offset);
+               if (bch2_fs_inconsistent_on(!g, c, "reference to invalid bucket on device %u\n  %s",
+                                           ptr->dev,
+                                           (bch2_bkey_val_to_text(&buf, c, s.s_c), buf.buf))) {
+                       ret = -EIO;
+                       goto err_unlock;
+               }
+
                bucket_lock(g);
                struct bch_alloc_v4 old = bucket_m_to_alloc(*g), new = old;
                ret = __mark_stripe_bucket(trans, ca, s, ptr_idx, deleting, bucket, &new, flags);
@@ -297,10 +305,12 @@ static int mark_stripe_bucket(struct btree_trans *trans,
                        bch2_dev_usage_update(c, ca, &old, &new, 0, true);
                }
                bucket_unlock(g);
+err_unlock:
                percpu_up_read(&c->mark_lock);
        }
 err:
        bch2_dev_put(ca);
+       printbuf_exit(&buf);
        return ret;
 }
 
@@ -714,10 +724,12 @@ static void ec_block_endio(struct bio *bio)
                               bch2_blk_status_to_str(bio->bi_status)))
                clear_bit(ec_bio->idx, ec_bio->buf->valid);
 
-       if (dev_ptr_stale(ca, ptr)) {
+       int stale = dev_ptr_stale(ca, ptr);
+       if (stale) {
                bch_err_ratelimited(ca->fs,
-                                   "error %s stripe: stale pointer after io",
-                                   bio_data_dir(bio) == READ ? "reading from" : "writing to");
+                                   "error %s stripe: stale/invalid pointer (%i) after io",
+                                   bio_data_dir(bio) == READ ? "reading from" : "writing to",
+                                   stale);
                clear_bit(ec_bio->idx, ec_bio->buf->valid);
        }
 
@@ -743,10 +755,12 @@ static void ec_block_io(struct bch_fs *c, struct ec_stripe_buf *buf,
                return;
        }
 
-       if (dev_ptr_stale(ca, ptr)) {
+       int stale = dev_ptr_stale(ca, ptr);
+       if (stale) {
                bch_err_ratelimited(c,
-                                   "error %s stripe: stale pointer",
-                                   rw == READ ? "reading from" : "writing to");
+                                   "error %s stripe: stale pointer (%i)",
+                                   rw == READ ? "reading from" : "writing to",
+                                   stale);
                clear_bit(idx, buf->valid);
                return;
        }

diff --git a/fs/bcachefs/extents.c b/fs/bcachefs/extents.c
index 469037929685bc3411fc3b9b3df3ff090a150cbf..410b8bd81b5a6ef8f0c0d3a9e3731e20de193bb5 100644 (file)
--- a/fs/bcachefs/extents.c
+++ b/fs/bcachefs/extents.c
@@ -137,7 +137,7 @@ int bch2_bkey_pick_read_device(struct bch_fs *c, struct bkey_s_c k,
 
                struct bch_dev *ca = bch2_dev_rcu(c, p.ptr.dev);
 
-               if (p.ptr.cached && (!ca || dev_ptr_stale(ca, &p.ptr)))
+               if (p.ptr.cached && (!ca || dev_ptr_stale_rcu(ca, &p.ptr)))
                        continue;
 
                f = failed ? dev_io_failures(failed, p.ptr.dev) : NULL;
@@ -999,7 +999,7 @@ bool bch2_extent_normalize(struct bch_fs *c, struct bkey_s k)
        bch2_bkey_drop_ptrs(k, ptr,
                ptr->cached &&
                (ca = bch2_dev_rcu(c, ptr->dev)) &&
-               dev_ptr_stale_rcu(ca, ptr));
+               dev_ptr_stale_rcu(ca, ptr) > 0);
        rcu_read_unlock();
 
        return bkey_deleted(k.k);
@@ -1024,8 +1024,11 @@ void bch2_extent_ptr_to_text(struct printbuf *out, struct bch_fs *c, const struc
                        prt_str(out, " cached");
                if (ptr->unwritten)
                        prt_str(out, " unwritten");
-               if (bucket_valid(ca, b) && dev_ptr_stale_rcu(ca, ptr))
+               int stale = dev_ptr_stale_rcu(ca, ptr);
+               if (stale > 0)
                        prt_printf(out, " stale");
+               else if (stale)
+                       prt_printf(out, " invalid");
        }
        rcu_read_unlock();
        --out->atomic;

diff --git a/fs/bcachefs/io_read.c b/fs/bcachefs/io_read.c
index 862b79f86b91e2ffe17b981ac1eae2a55b34ff1d..c97fa7002b06e0ac9a172d23e5f627d47b9c9e7f 100644 (file)
--- a/fs/bcachefs/io_read.c
+++ b/fs/bcachefs/io_read.c
@@ -777,18 +777,32 @@ static noinline void read_from_stale_dirty_pointer(struct btree_trans *trans,
                             PTR_BUCKET_POS(ca, &ptr),
                             BTREE_ITER_cached);
 
-       prt_printf(&buf, "Attempting to read from stale dirty pointer:\n");
-       printbuf_indent_add(&buf, 2);
+       u8 *gen = bucket_gen(ca, iter.pos.offset);
+       if (gen) {
 
-       bch2_bkey_val_to_text(&buf, c, k);
-       prt_newline(&buf);
+               prt_printf(&buf, "Attempting to read from stale dirty pointer:\n");
+               printbuf_indent_add(&buf, 2);
 
-       prt_printf(&buf, "memory gen: %u", *bucket_gen(ca, iter.pos.offset));
-
-       ret = lockrestart_do(trans, bkey_err(k = bch2_btree_iter_peek_slot(&iter)));
-       if (!ret) {
+               bch2_bkey_val_to_text(&buf, c, k);
                prt_newline(&buf);
+
+               prt_printf(&buf, "memory gen: %u", *gen);
+
+               ret = lockrestart_do(trans, bkey_err(k = bch2_btree_iter_peek_slot(&iter)));
+               if (!ret) {
+                       prt_newline(&buf);
+                       bch2_bkey_val_to_text(&buf, c, k);
+               }
+       } else {
+               prt_printf(&buf, "Attempting to read from invalid bucket %llu:%llu:\n",
+                          iter.pos.inode, iter.pos.offset);
+               printbuf_indent_add(&buf, 2);
+
+               prt_printf(&buf, "first bucket %u nbuckets %llu\n",
+                          ca->mi.first_bucket, ca->mi.nbuckets);
+
                bch2_bkey_val_to_text(&buf, c, k);
+               prt_newline(&buf);
        }
 
        bch2_fs_inconsistent(c, "%s", buf.buf);

diff --git a/fs/bcachefs/io_write.c b/fs/bcachefs/io_write.c
index 9401d13e31bb63cdf0e99977b628a940629325aa..05e0cbef420bc790a5694cc7c85aba3724e580ed 100644 (file)
--- a/fs/bcachefs/io_write.c
+++ b/fs/bcachefs/io_write.c
@@ -1220,7 +1220,7 @@ static void bch2_nocow_write(struct bch_write_op *op)
        DARRAY_PREALLOCATED(struct bucket_to_lock, 3) buckets;
        u32 snapshot;
        struct bucket_to_lock *stale_at;
-       int ret;
+       int stale, ret;
 
        if (op->flags & BCH_WRITE_MOVE)
                return;
@@ -1299,7 +1299,8 @@ retry:
                                                 BUCKET_NOCOW_LOCK_UPDATE);
 
                        rcu_read_lock();
-                       bool stale = gen_after(*bucket_gen(ca, i->b.offset), i->gen);
+                       u8 *gen = bucket_gen(ca, i->b.offset);
+                       stale = !gen ? -1 : gen_after(*gen, i->gen);
                        rcu_read_unlock();
 
                        if (unlikely(stale)) {
@@ -1380,8 +1381,18 @@ err_bucket_stale:
                        break;
        }
 
-       /* We can retry this: */
-       ret = -BCH_ERR_transaction_restart;
+       struct printbuf buf = PRINTBUF;
+       if (bch2_fs_inconsistent_on(stale < 0, c,
+                                   "pointer to invalid bucket in nocow path on device %llu\n  %s",
+                                   stale_at->b.inode,
+                                   (bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
+               ret = -EIO;
+       } else {
+               /* We can retry this: */
+               ret = -BCH_ERR_transaction_restart;
+       }
+       printbuf_exit(&buf);
+
        goto err_get_ioref;
 }