[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <plhmwoygvh6plwq3d42f6kriyb6fsripwpe2njxn3zdd7yly4y@j6dgwf2ihubq>
Date: Fri, 25 Oct 2024 20:14:09 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Piotr Zalewski <pZ010001011111@...ton.me>
Cc: Alan Huang <mmpgouride@...il.com>, linux-bcachefs@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, skhan@...uxfoundation.org,
syzbot+005ef9aa519f30d97657@...kaller.appspotmail.com
Subject: Re: [PATCH] bcachefs: Fix NULL ptr dereference in
btree_node_iter_and_journal_peek
On Fri, Oct 25, 2024 at 11:39:38PM +0000, Piotr Zalewski wrote:
> Hi Alan and all,
>
> On Wednesday, October 23rd, 2024 at 10:06 AM, Piotr Zalewski <pZ010001011111@...ton.me> wrote:
>
> > Hi Alan,
> >
> > On Wednesday, October 23rd, 2024 at 9:33 AM, Alan Huang mmpgouride@...il.com wrote:
> >
> > > On Oct 23, 2024, at 15:21, Piotr Zalewski pZ010001011111@...ton.me 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.
> >
>
> There is a clean shutdown detected during recovery. Journal has no entries
> as it was confirmed by debugging bch2_fs_journal_start. seq and seq_ondisk
> members are equal so journal is "quiesced". Also, journal_keys size is 0.
>
> So in check_allocations when keys are being marked for GC journal replay is
> not done so it peeks into journal and there is nothing.
>
> Now, maybe in case journal is empty during start, replay done should be set
> already in bch2_fs_journal_start? I tested it and it fixed the issue as well.
this is "btree_and_journal_iter_peek()" - it's iterating over the btree
node and overlaying keys from the journal, so the journal being empty is
fine.
It's because syzbot is feeding us filesystem images that are broken in
interesting ways :)
Powered by blists - more mailing lists