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.

>  
>       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
> 
> 

Reply via email to