All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ryusuke Konishi <konishi.ryusuke@gmail.com>
To: Liu Shixin <liushixin2@huawei.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-nilfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nilfs2: fix NULL pointer dereference in nilfs_segctor_prepare_write()
Date: Tue, 8 Nov 2022 20:10:36 +0900	[thread overview]
Message-ID: <CAKFNMo=bbR+ZgJQosEoj=yfoY2y=PmYHVfz3CdLCvCWVK7igTQ@mail.gmail.com> (raw)
In-Reply-To: <CAKFNMokcSj9YSLeXm=S4rY5nMx6DjQvRHXVaLVu2CbNEia7-2Q@mail.gmail.com>

On Tue, Nov 8, 2022 at 7:33 PM Ryusuke Konishi  wrote:
> On Tue, Nov 8, 2022 at 3:49 PM Liu Shixin wrote:
> > Then in nilfs_segctor_extend_segments(), we set sb_segnum by prev->sb_nextnum directly,
> > and calculate next sb_segnum by nilfs_sufile_alloc(), since last_alloc is not updated,
> > we will get sb_segnum again.
>
> nilfs_segctor_extend_segments() pre-allocates one or more segments
> large enough to store updated blocks of metadata files that need to be
> written in a series of logs at once, and sets up a chain of segbufs.
> (Sorry for the missing function comment).
>
> sb_segnum is set by prev->sb_nextnum to form a chain of buffers for
> segments.  This is expected behavior.
> And, the sb_nextnum (= next sb_segnum) will be given by
> nilfs_sufile_alloc().   This is also expected.
> It looks like the problem is that nilfs_sufile_alloc() here allocates
> the same segnum again.
>
> Because sb_segnum is set by prev->sb_nextnum which is allocated by the
> previous nilfs_sufile_alloc() call,
> this usually does not happen.
>
> A possible anomaly is if the segment pointed by the first nextnum (or
> segnum) was not marked dirty on sufile.
> This may happen if the sufile is corrupted on the test disk image that
> syzbot provided (mounted).
>
> Can you confirm if this is actually happening?

If we can mount the test disk image, the state of sufile can be
confirmed quickly with lssu command:

$ lssu
              SEGNUM        DATE     TIME STAT     NBLOCKS
                   3  2022-11-04 23:23:49  -d-        2048
                   4  2022-11-04 23:23:50  ad-         103
                   5  ---------- --:--:--  ad-           0

Here, the flag "d" in STAT means the segment is dirty (in-use) and the
segment of ns_segnum or ns_nextnum is indicated with the "a" flag.
This is an example of a normal disk image.
Or, if it's easy to insert debug code to check, that's fine too.

Regards,
Ryusuke Konishi

  reply	other threads:[~2022-11-08 11:10 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08  2:29 [PATCH] nilfs2: fix NULL pointer dereference in nilfs_segctor_prepare_write() Liu Shixin
2022-11-08  2:29 ` Liu Shixin
2022-11-08  4:41 ` Ryusuke Konishi
2022-11-08  4:41   ` Ryusuke Konishi
2022-11-08  6:19   ` Ryusuke Konishi
2022-11-08  6:19     ` Ryusuke Konishi
2022-11-08  6:49   ` Liu Shixin
2022-11-08  6:49     ` Liu Shixin
2022-11-08 10:33     ` Ryusuke Konishi
2022-11-08 10:33       ` Ryusuke Konishi
2022-11-08 11:10       ` Ryusuke Konishi [this message]
2022-11-08 11:53       ` Liu Shixin
2022-11-08 11:53         ` Liu Shixin
2022-11-08 12:24         ` Ryusuke Konishi
2022-11-08 12:24           ` Ryusuke Konishi
2022-11-08 15:13           ` Ryusuke Konishi
2022-11-08 15:13             ` Ryusuke Konishi
2022-11-08 17:50             ` Ryusuke Konishi
2022-11-08 17:50               ` Ryusuke Konishi
2022-11-09  3:26               ` Liu Shixin
2022-11-09  3:26                 ` Liu Shixin
2022-11-09 14:00                 ` Ryusuke Konishi
2022-11-09 14:00                   ` Ryusuke Konishi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKFNMo=bbR+ZgJQosEoj=yfoY2y=PmYHVfz3CdLCvCWVK7igTQ@mail.gmail.com' \
    --to=konishi.ryusuke@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nilfs@vger.kernel.org \
    --cc=liushixin2@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.