1

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>
This commit is contained in:
Kent Overstreet 2024-06-06 15:06:22 -04:00
parent 9c4acd19bb
commit 9432e90df1
8 changed files with 136 additions and 48 deletions

View File

@ -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;
}
/*

View File

@ -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);

View File

@ -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,

View File

@ -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;

View File

@ -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;
}

View File

@ -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;

View File

@ -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) {
prt_newline(&buf);
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);

View File

@ -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;
}