On Sunday, October 27th, 2024 at 2:28 AM, Kent Overstreet <[email protected]> wrote:
> On Sat, Oct 26, 2024 at 05:46:33PM +0000, Piotr Zalewski 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. > > > > When key returned from bch2_btree_and_journal_iter_peek is NULL it means > > that btree topology needs repair. Print error message with position at > > which node wasn't found and its parent node information. Call > > bch2_topology_error and return error code returned by it to ensure that > > topology error is handled properly. > > > > Reported-by: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=005ef9aa519f30d97657 > > Fixes: 5222a4607cd8 ("bcachefs: BTREE_ITER_WITH_JOURNAL") > > Suggested-by: Alan Huang [email protected] > > Suggested-by: Kent Overstreet [email protected] > > Signed-off-by: Piotr Zalewski [email protected] > > --- > > > > Notes: > > changes in v2: > > - make commit message more verbose. > > - set topology error, print error message and return > > appropriate error code. > > > > link to v1: > > https://lore.kernel.org/linux-bcachefs/[email protected]/ > > > > fs/bcachefs/btree_iter.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c > > index 15ac72b1af51..40c824779b15 100644 > > --- a/fs/bcachefs/btree_iter.c > > +++ b/fs/bcachefs/btree_iter.c > > @@ -880,6 +880,18 @@ 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) { > > + struct printbuf buf = PRINTBUF; > > + > > + prt_str(&buf, "node not found at pos "); > > + bch2_bpos_to_text(&buf, path->pos); > > + prt_str(&buf, " within parent node "); > > + bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(&l->b->key)); > > + > > + ret = bch2_fs_topology_error(c, "%s", buf.buf); > > + printbuf_exit(&buf); > > + goto err; > > + } > > > We'll want to add at least the btree ID and level to that. > > Could you also look over the other places we report topology errors and > inconstencies for any commonality? btree_cache.c has some stuff, and I > think there's a helper in there that might give you the error message > you want (instead of just the btree node key), and I'd have a glance at > the topology error reporting in btree_update_interior.c and btree_gc.c > as well. I scanned through mentioned files and some others. There are commonalities but every error message seem to be different in some way so that there is no truly common part. Information contained is usually a mixture of: - btree id and level. - position at which node wasn't found or if it was found but something else is wrong its key info. - parent node key information. - prev/next node key info. - min/max key pos info. Only appropriate helpers I found were (what I already used) - bch2_fs_topology_error and, in btree_iter.c, bch2_btree_path_to_text_short but there it doesn't print full parent key info just min_key pos and parent key pos so I'm not sure. To what you already said I could also add min pos info. > > bch2_bkey_buf_reassemble(out, c, k); > > > > @@ -887,6 +899,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
