bcachefs: Fix null ptr deref in bucket_gen_get()

bucket_gen() checks if we're lookup up a valid bucket and returns NULL
otherwise, but bucket_gen_get() was failing to check; other callers were
correct.

Also do a bit of cleanup on callers.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
This commit is contained in:
Kent Overstreet 2024-10-27 20:40:20 -04:00
parent 59b723cd2a
commit fd00045f38
4 changed files with 17 additions and 18 deletions

View File

@ -186,7 +186,7 @@ static void try_read_btree_node(struct find_btree_nodes *f, struct bch_dev *ca,
.ptrs[0].type = 1 << BCH_EXTENT_ENTRY_ptr, .ptrs[0].type = 1 << BCH_EXTENT_ENTRY_ptr,
.ptrs[0].offset = offset, .ptrs[0].offset = offset,
.ptrs[0].dev = ca->dev_idx, .ptrs[0].dev = ca->dev_idx,
.ptrs[0].gen = *bucket_gen(ca, sector_to_bucket(ca, offset)), .ptrs[0].gen = bucket_gen_get(ca, sector_to_bucket(ca, offset)),
}; };
rcu_read_unlock(); rcu_read_unlock();

View File

@ -103,12 +103,18 @@ static inline u8 *bucket_gen(struct bch_dev *ca, size_t b)
return gens->b + b; return gens->b + b;
} }
static inline u8 bucket_gen_get(struct bch_dev *ca, size_t b) static inline int bucket_gen_get_rcu(struct bch_dev *ca, size_t b)
{
u8 *gen = bucket_gen(ca, b);
return gen ? *gen : -1;
}
static inline int bucket_gen_get(struct bch_dev *ca, size_t b)
{ {
rcu_read_lock(); rcu_read_lock();
u8 gen = *bucket_gen(ca, b); int ret = bucket_gen_get_rcu(ca, b);
rcu_read_unlock(); rcu_read_unlock();
return gen; return ret;
} }
static inline size_t PTR_BUCKET_NR(const struct bch_dev *ca, static inline size_t PTR_BUCKET_NR(const struct bch_dev *ca,
@ -169,10 +175,8 @@ static inline int gen_after(u8 a, u8 b)
static inline int 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)
{ {
u8 *gen = bucket_gen(ca, PTR_BUCKET_NR(ca, ptr)); int gen = bucket_gen_get_rcu(ca, PTR_BUCKET_NR(ca, ptr));
if (!gen) return gen < 0 ? gen : gen_after(gen, ptr->gen);
return -1;
return gen_after(*gen, ptr->gen);
} }
/** /**
@ -184,7 +188,6 @@ static inline int dev_ptr_stale(struct bch_dev *ca, const struct bch_extent_ptr
rcu_read_lock(); rcu_read_lock();
int ret = dev_ptr_stale_rcu(ca, ptr); int ret = dev_ptr_stale_rcu(ca, ptr);
rcu_read_unlock(); rcu_read_unlock();
return ret; return ret;
} }

View File

@ -802,16 +802,15 @@ static noinline void read_from_stale_dirty_pointer(struct btree_trans *trans,
PTR_BUCKET_POS(ca, &ptr), PTR_BUCKET_POS(ca, &ptr),
BTREE_ITER_cached); BTREE_ITER_cached);
u8 *gen = bucket_gen(ca, iter.pos.offset); int gen = bucket_gen_get(ca, iter.pos.offset);
if (gen) { if (gen >= 0) {
prt_printf(&buf, "Attempting to read from stale dirty pointer:\n"); prt_printf(&buf, "Attempting to read from stale dirty pointer:\n");
printbuf_indent_add(&buf, 2); printbuf_indent_add(&buf, 2);
bch2_bkey_val_to_text(&buf, c, k); bch2_bkey_val_to_text(&buf, c, k);
prt_newline(&buf); prt_newline(&buf);
prt_printf(&buf, "memory gen: %u", *gen); prt_printf(&buf, "memory gen: %u", gen);
ret = lockrestart_do(trans, bkey_err(k = bch2_btree_iter_peek_slot(&iter))); ret = lockrestart_do(trans, bkey_err(k = bch2_btree_iter_peek_slot(&iter)));
if (!ret) { if (!ret) {

View File

@ -1300,11 +1300,8 @@ retry:
bucket_to_u64(i->b), bucket_to_u64(i->b),
BUCKET_NOCOW_LOCK_UPDATE); BUCKET_NOCOW_LOCK_UPDATE);
rcu_read_lock(); int gen = bucket_gen_get(ca, i->b.offset);
u8 *gen = bucket_gen(ca, i->b.offset); stale = gen < 0 ? gen : gen_after(gen, i->gen);
stale = !gen ? -1 : gen_after(*gen, i->gen);
rcu_read_unlock();
if (unlikely(stale)) { if (unlikely(stale)) {
stale_at = i; stale_at = i;
goto err_bucket_stale; goto err_bucket_stale;