Skip to content

Commit d9534cc

Browse files
author
Kent Overstreet
committed
bcachefs: fix buffer overflow in nocow write path
BCH_REPLICAS_MAX isn't the actual maximum number of pointers in an extent, it's the maximum number of dirty pointers. We don't have a real restriction on the number of cached pointers, and we don't want a fixed size array here anyways - so switch to DARRAY_PREALLOCATED(). Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> Reported-and-tested-by: Daniel J Blueman <daniel@quora.org>
1 parent 099dc5c commit d9534cc

File tree

1 file changed

+41
-41
lines changed

1 file changed

+41
-41
lines changed

fs/bcachefs/io_write.c

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,12 @@ static CLOSURE_CALLBACK(bch2_nocow_write_done)
12161216
bch2_write_done(cl);
12171217
}
12181218

1219+
struct bucket_to_lock {
1220+
struct bpos b;
1221+
unsigned gen;
1222+
struct nocow_lock_bucket *l;
1223+
};
1224+
12191225
static void bch2_nocow_write(struct bch_write_op *op)
12201226
{
12211227
struct bch_fs *c = op->c;
@@ -1224,18 +1230,16 @@ static void bch2_nocow_write(struct bch_write_op *op)
12241230
struct bkey_s_c k;
12251231
struct bkey_ptrs_c ptrs;
12261232
const struct bch_extent_ptr *ptr;
1227-
struct {
1228-
struct bpos b;
1229-
unsigned gen;
1230-
struct nocow_lock_bucket *l;
1231-
} buckets[BCH_REPLICAS_MAX];
1232-
unsigned nr_buckets = 0;
1233+
DARRAY_PREALLOCATED(struct bucket_to_lock, 3) buckets;
1234+
struct bucket_to_lock *i;
12331235
u32 snapshot;
1234-
int ret, i;
1236+
struct bucket_to_lock *stale_at;
1237+
int ret;
12351238

12361239
if (op->flags & BCH_WRITE_MOVE)
12371240
return;
12381241

1242+
darray_init(&buckets);
12391243
trans = bch2_trans_get(c);
12401244
retry:
12411245
bch2_trans_begin(trans);
@@ -1250,7 +1254,7 @@ static void bch2_nocow_write(struct bch_write_op *op)
12501254
while (1) {
12511255
struct bio *bio = &op->wbio.bio;
12521256

1253-
nr_buckets = 0;
1257+
buckets.nr = 0;
12541258

12551259
k = bch2_btree_iter_peek_slot(&iter);
12561260
ret = bkey_err(k);
@@ -1263,26 +1267,26 @@ static void bch2_nocow_write(struct bch_write_op *op)
12631267
break;
12641268

12651269
if (bch2_keylist_realloc(&op->insert_keys,
1266-
op->inline_keys,
1267-
ARRAY_SIZE(op->inline_keys),
1268-
k.k->u64s))
1270+
op->inline_keys,
1271+
ARRAY_SIZE(op->inline_keys),
1272+
k.k->u64s))
12691273
break;
12701274

12711275
/* Get iorefs before dropping btree locks: */
12721276
ptrs = bch2_bkey_ptrs_c(k);
12731277
bkey_for_each_ptr(ptrs, ptr) {
1274-
buckets[nr_buckets].b = PTR_BUCKET_POS(c, ptr);
1275-
buckets[nr_buckets].gen = ptr->gen;
1276-
buckets[nr_buckets].l =
1277-
bucket_nocow_lock(&c->nocow_locks,
1278-
bucket_to_u64(buckets[nr_buckets].b));
1279-
1280-
prefetch(buckets[nr_buckets].l);
1278+
struct bpos b = PTR_BUCKET_POS(c, ptr);
1279+
struct nocow_lock_bucket *l =
1280+
bucket_nocow_lock(&c->nocow_locks, bucket_to_u64(b));
1281+
prefetch(l);
12811282

12821283
if (unlikely(!bch2_dev_get_ioref(bch_dev_bkey_exists(c, ptr->dev), WRITE)))
12831284
goto err_get_ioref;
12841285

1285-
nr_buckets++;
1286+
/* XXX allocating memory with btree locks held - rare */
1287+
darray_push_gfp(&buckets, ((struct bucket_to_lock) {
1288+
.b = b, .gen = ptr->gen, .l = l,
1289+
}), GFP_KERNEL|__GFP_NOFAIL);
12861290

12871291
if (ptr->unwritten)
12881292
op->flags |= BCH_WRITE_CONVERT_UNWRITTEN;
@@ -1296,21 +1300,21 @@ static void bch2_nocow_write(struct bch_write_op *op)
12961300
if (op->flags & BCH_WRITE_CONVERT_UNWRITTEN)
12971301
bch2_cut_back(POS(op->pos.inode, op->pos.offset + bio_sectors(bio)), op->insert_keys.top);
12981302

1299-
for (i = 0; i < nr_buckets; i++) {
1300-
struct bch_dev *ca = bch_dev_bkey_exists(c, buckets[i].b.inode);
1301-
struct nocow_lock_bucket *l = buckets[i].l;
1302-
bool stale;
1303+
darray_for_each(buckets, i) {
1304+
struct bch_dev *ca = bch_dev_bkey_exists(c, i->b.inode);
13031305

1304-
__bch2_bucket_nocow_lock(&c->nocow_locks, l,
1305-
bucket_to_u64(buckets[i].b),
1306+
__bch2_bucket_nocow_lock(&c->nocow_locks, i->l,
1307+
bucket_to_u64(i->b),
13061308
BUCKET_NOCOW_LOCK_UPDATE);
13071309

13081310
rcu_read_lock();
1309-
stale = gen_after(*bucket_gen(ca, buckets[i].b.offset), buckets[i].gen);
1311+
bool stale = gen_after(*bucket_gen(ca, i->b.offset), i->gen);
13101312
rcu_read_unlock();
13111313

1312-
if (unlikely(stale))
1314+
if (unlikely(stale)) {
1315+
stale_at = i;
13131316
goto err_bucket_stale;
1317+
}
13141318
}
13151319

13161320
bio = &op->wbio.bio;
@@ -1346,15 +1350,14 @@ static void bch2_nocow_write(struct bch_write_op *op)
13461350

13471351
if (ret) {
13481352
bch_err_inum_offset_ratelimited(c,
1349-
op->pos.inode,
1350-
op->pos.offset << 9,
1351-
"%s: btree lookup error %s",
1352-
__func__, bch2_err_str(ret));
1353+
op->pos.inode, op->pos.offset << 9,
1354+
"%s: btree lookup error %s", __func__, bch2_err_str(ret));
13531355
op->error = ret;
13541356
op->flags |= BCH_WRITE_DONE;
13551357
}
13561358

13571359
bch2_trans_put(trans);
1360+
darray_exit(&buckets);
13581361

13591362
/* fallback to cow write path? */
13601363
if (!(op->flags & BCH_WRITE_DONE)) {
@@ -1374,24 +1377,21 @@ static void bch2_nocow_write(struct bch_write_op *op)
13741377
}
13751378
return;
13761379
err_get_ioref:
1377-
for (i = 0; i < nr_buckets; i++)
1378-
percpu_ref_put(&bch_dev_bkey_exists(c, buckets[i].b.inode)->io_ref);
1380+
darray_for_each(buckets, i)
1381+
percpu_ref_put(&bch_dev_bkey_exists(c, i->b.inode)->io_ref);
13791382

13801383
/* Fall back to COW path: */
13811384
goto out;
13821385
err_bucket_stale:
1383-
while (i >= 0) {
1384-
bch2_bucket_nocow_unlock(&c->nocow_locks,
1385-
buckets[i].b,
1386-
BUCKET_NOCOW_LOCK_UPDATE);
1387-
--i;
1386+
darray_for_each(buckets, i) {
1387+
bch2_bucket_nocow_unlock(&c->nocow_locks, i->b, BUCKET_NOCOW_LOCK_UPDATE);
1388+
if (i == stale_at)
1389+
break;
13881390
}
1389-
for (i = 0; i < nr_buckets; i++)
1390-
percpu_ref_put(&bch_dev_bkey_exists(c, buckets[i].b.inode)->io_ref);
13911391

13921392
/* We can retry this: */
13931393
ret = -BCH_ERR_transaction_restart;
1394-
goto out;
1394+
goto err_get_ioref;
13951395
}
13961396

13971397
static void __bch2_write(struct bch_write_op *op)

0 commit comments

Comments
 (0)