Skip to content

Commit fd00045

Browse files
author
Kent Overstreet
committed
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 <[email protected]>
1 parent 59b723c commit fd00045

File tree

4 files changed

+17
-18
lines changed

4 files changed

+17
-18
lines changed

fs/bcachefs/btree_node_scan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ static void try_read_btree_node(struct find_btree_nodes *f, struct bch_dev *ca,
186186
.ptrs[0].type = 1 << BCH_EXTENT_ENTRY_ptr,
187187
.ptrs[0].offset = offset,
188188
.ptrs[0].dev = ca->dev_idx,
189-
.ptrs[0].gen = *bucket_gen(ca, sector_to_bucket(ca, offset)),
189+
.ptrs[0].gen = bucket_gen_get(ca, sector_to_bucket(ca, offset)),
190190
};
191191
rcu_read_unlock();
192192

fs/bcachefs/buckets.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,18 @@ static inline u8 *bucket_gen(struct bch_dev *ca, size_t b)
103103
return gens->b + b;
104104
}
105105

106-
static inline u8 bucket_gen_get(struct bch_dev *ca, size_t b)
106+
static inline int bucket_gen_get_rcu(struct bch_dev *ca, size_t b)
107+
{
108+
u8 *gen = bucket_gen(ca, b);
109+
return gen ? *gen : -1;
110+
}
111+
112+
static inline int bucket_gen_get(struct bch_dev *ca, size_t b)
107113
{
108114
rcu_read_lock();
109-
u8 gen = *bucket_gen(ca, b);
115+
int ret = bucket_gen_get_rcu(ca, b);
110116
rcu_read_unlock();
111-
return gen;
117+
return ret;
112118
}
113119

114120
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)
169175

170176
static inline int dev_ptr_stale_rcu(struct bch_dev *ca, const struct bch_extent_ptr *ptr)
171177
{
172-
u8 *gen = bucket_gen(ca, PTR_BUCKET_NR(ca, ptr));
173-
if (!gen)
174-
return -1;
175-
return gen_after(*gen, ptr->gen);
178+
int gen = bucket_gen_get_rcu(ca, PTR_BUCKET_NR(ca, ptr));
179+
return gen < 0 ? gen : gen_after(gen, ptr->gen);
176180
}
177181

178182
/**
@@ -184,7 +188,6 @@ static inline int dev_ptr_stale(struct bch_dev *ca, const struct bch_extent_ptr
184188
rcu_read_lock();
185189
int ret = dev_ptr_stale_rcu(ca, ptr);
186190
rcu_read_unlock();
187-
188191
return ret;
189192
}
190193

fs/bcachefs/io_read.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -802,16 +802,15 @@ static noinline void read_from_stale_dirty_pointer(struct btree_trans *trans,
802802
PTR_BUCKET_POS(ca, &ptr),
803803
BTREE_ITER_cached);
804804

805-
u8 *gen = bucket_gen(ca, iter.pos.offset);
806-
if (gen) {
807-
805+
int gen = bucket_gen_get(ca, iter.pos.offset);
806+
if (gen >= 0) {
808807
prt_printf(&buf, "Attempting to read from stale dirty pointer:\n");
809808
printbuf_indent_add(&buf, 2);
810809

811810
bch2_bkey_val_to_text(&buf, c, k);
812811
prt_newline(&buf);
813812

814-
prt_printf(&buf, "memory gen: %u", *gen);
813+
prt_printf(&buf, "memory gen: %u", gen);
815814

816815
ret = lockrestart_do(trans, bkey_err(k = bch2_btree_iter_peek_slot(&iter)));
817816
if (!ret) {

fs/bcachefs/io_write.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,11 +1300,8 @@ static void bch2_nocow_write(struct bch_write_op *op)
13001300
bucket_to_u64(i->b),
13011301
BUCKET_NOCOW_LOCK_UPDATE);
13021302

1303-
rcu_read_lock();
1304-
u8 *gen = bucket_gen(ca, i->b.offset);
1305-
stale = !gen ? -1 : gen_after(*gen, i->gen);
1306-
rcu_read_unlock();
1307-
1303+
int gen = bucket_gen_get(ca, i->b.offset);
1304+
stale = gen < 0 ? gen : gen_after(gen, i->gen);
13081305
if (unlikely(stale)) {
13091306
stale_at = i;
13101307
goto err_bucket_stale;

0 commit comments

Comments
 (0)