Re: [PATCH] bcachefs: Fix NULL ptr dereference in btree_node_iter_and_journal_peek

[Piotr Zalewski]

Hi Alan,

On Wednesday, October 23rd, 2024 at 9:33 AM, Alan Huang <mmpgouride@xxxxxxxxx> wrote:

> On Oct 23, 2024, at 15:21, Piotr Zalewski pZ010001011111@xxxxxxxxx wrote:
> 
> > Add NULL check for key returned from bch2_btree_and_journal_iter_peek in
> > btree_node_iter_and_journal_peek to avoid NULL ptr dereference in
> > bch2_bkey_buf_reassemble.
> 
> 
> It would be helpful if the commit message explained why k.k is null in this case
 
I will debug this more thoroughly and provide details. For now I see that
during GC it sees journal replay hasn't finished but journal turns out to
be empty? Which seems weird, so maybe underlying issue should be solved on
a deeper level.

Log from the reproducer is:
```
[   27.391332] bcachefs (loop0): accounting_read... done
[   27.408141] bcachefs (loop0): alloc_read... done
[   27.409118] bcachefs (loop0): stripes_read... done
[   27.410059] bcachefs (loop0): snapshots_read... done
[   27.411161] bcachefs (loop0): check_allocations...
[   27.415003] bucket 0:26 data type btree ptr gen 0 missing in alloc btree
[   27.415024] while marking u64s 11 type btree_ptr_v2 SPOS_MAX len 0 ver 0: seq ac62141f8dc7e261 written 24 min_kg
[   27.422560] bucket 0:38 data type btree ptr gen 0 missing in alloc btree
[   27.422571] while marking u64s 11 type btree_ptr_v2 SPOS_MAX len 0 ver 0: seq 7589ab5e0c11cc7a written 24 min_kg
[   27.428033] bucket 0:41 data type btree ptr gen 0 missing in alloc btree
[   27.428042] while marking u64s 11 type btree_ptr_v2 SPOS_MAX len 0 ver 0: seq 9aa2895aefce4bdf written 24 min_kg
[   27.432315] bcachefs (loop0): btree topology error: attempting to get btree node with non-btree key u64s 0 type
[   27.435343] bcachefs (loop0): inconsistency detected - emergency read only at journal seq 10
[   27.436947] bcachefs (loop0): bch2_gc_btree(): error btree_need_topology_repair
[   27.438756] btree node read error for xattrs, shutting down
[   27.440081] bcachefs (loop0): bch2_gc_btrees(): error fsck_errors_not_fixed
[   27.441915] bcachefs (loop0): bch2_check_allocations(): error fsck_errors_not_fixed
[   27.443349] bcachefs (loop0): bch2_fs_recovery(): error fsck_errors_not_fixed
[   27.444802] bcachefs (loop0): bch2_fs_start(): error starting filesystem fsck_errors_not_fixed
[   27.446270] bcachefs (loop0): shutting down
[   27.456042] bcachefs (loop0): shutdown complete
[   27.835683] bcachefs: bch2_fs_get_tree() error: fsck_errors_not_fixed
```


> > Reported-by: syzbot+005ef9aa519f30d97657@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=005ef9aa519f30d97657
> > Fixes: 5222a4607cd8 ("bcachefs: BTREE_ITER_WITH_JOURNAL")
> > Signed-off-by: Piotr Zalewski pZ010001011111@xxxxxxxxx
> > ---
> > fs/bcachefs/btree_iter.c | 3 +++
> > 1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
> > index 0883cf6e1a3e..625167ce191f 100644
> > --- a/fs/bcachefs/btree_iter.c
> > +++ b/fs/bcachefs/btree_iter.c
> > @@ -882,6 +882,8 @@ static noinline int btree_node_iter_and_journal_peek(struct btree_trans *trans,
> > __bch2_btree_and_journal_iter_init_node_iter(trans, &jiter, l->b, l->iter, path->pos);
> > 
> > k = bch2_btree_and_journal_iter_peek(&jiter);
> > + if (!k.k)
> > + goto err;
> > 
> > bch2_bkey_buf_reassemble(out, c, k);
> > 
> > @@ -889,6 +891,7 @@ static noinline int btree_node_iter_and_journal_peek(struct btree_trans *trans,
> > c->opts.btree_node_prefetch)
> > ret = btree_path_prefetch_j(trans, path, &jiter);
> > 
> > +err:
> > bch2_btree_and_journal_iter_exit(&jiter);
> > return ret;
> > }
> > --
> > 2.47.0

Best regards, Piotr Zalewski