linux-kernel - Re: [PATCH v2] bcachefs: Fix NULL ptr dereference in btree_node_iter_and_journal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BdSD2g3SGfWQrsTULTjViyG_t0llBh4NeIVbIE_3pvf1bWxk52ErCdP5yOErBZO3NkhyOv9Mgq2xc6CsGFliqDQrTgTwoquYKJKEUY5OE9g=@proton.me>
Date: Sun, 27 Oct 2024 11:34:02 +0000
From: Piotr Zalewski <pZ010001011111@...ton.me>
To: Kent Overstreet <kent.overstreet@...ux.dev>
Cc: linux-bcachefs@...r.kernel.org, linux-kernel@...r.kernel.org, skhan@...uxfoundation.org, syzbot+005ef9aa519f30d97657@...kaller.appspotmail.com, Alan Huang <mmpgouride@...il.com>
Subject: Re: [PATCH v2] bcachefs: Fix NULL ptr dereference in btree_node_iter_and_journal_peek

On Sunday, October 27th, 2024 at 2:28 AM, Kent Overstreet <kent.overstreet@...ux.dev> 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: syzbot+005ef9aa519f30d97657@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=005ef9aa519f30d97657
> > Fixes: 5222a4607cd8 ("bcachefs: BTREE_ITER_WITH_JOURNAL")
> > Suggested-by: Alan Huang mmpgouride@...il.com
> > Suggested-by: Kent Overstreet kent.overstreet@...ux.dev
> > Signed-off-by: Piotr Zalewski pZ010001011111@...ton.me
> > ---
> > 
> > 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/20241023072024.98915-3-pZ010001011111@proton.me/
> > 
> > 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