All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ryusuke Konishi <konishi.ryusuke@gmail.com>
To: Chen Zhongjin <chenzhongjin@huawei.com>, akpm@linux-foundation.org
Cc: linux-nilfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v3] nilfs2: Fix nilfs_sufile_mark_dirty() not set segment usage as dirty
Date: Mon, 21 Nov 2022 18:48:49 +0900	[thread overview]
Message-ID: <CAKFNMonEeNWnz0OKdSRR-bzg-qPg0eL+OvWrUDfy1vP4QySquw@mail.gmail.com> (raw)
In-Reply-To: <20221121091141.214703-1-chenzhongjin@huawei.com>

On Mon, Nov 21, 2022 at 6:14 PM Chen Zhongjin wrote:
>
> When extending segments, nilfs_sufile_alloc() is called to get an
> unassigned segment, then mark it as dirty to avoid accidentally
> allocating the same segment in the future.
>
> But for some special cases such as a corrupted image it can be
> unreliable.
> If such corruption of the dirty state of the segment occurs, nilfs2 may
> reallocate a segment that is in use and pick the same segment for
> writing twice at the same time.
>
> This will cause the problem reported by syzkaller:
> https://syzkaller.appspot.com/bug?id=c7c4748e11ffcc367cef04f76e02e931833cbd24
>
> This case started with segbuf1.segnum = 3, nextnum = 4 when constructed.
> It supposed segment 4 has already been allocated and marked as dirty.
>
> However the dirty state was corrupted and segment 4 usage was not dirty.
> For the first time nilfs_segctor_extend_segments() segment 4 was
> allocated again, which made segbuf2 and next segbuf3 had same segment 4.
>
> sb_getblk() will get same bh for segbuf2 and segbuf3, and this bh is
> added to both buffer lists of two segbuf. It makes the lists broken
> which causes NULL pointer dereference.
>
> Fix the problem by setting usage as dirty every time in
> nilfs_sufile_mark_dirty(), which is called during constructing current
> segment to be written out and before allocating next segment.
>
> Fixes: 9ff05123e3bf ("nilfs2: segment constructor")
> Cc: stable@vger.kernel.org
> Reported-by: syzbot+77e4f005cb899d4268d1@syzkaller.appspotmail.com
> Reported-by: Liu Shixin <liushixin2@huawei.com>
> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
> Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> Tested-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> ---
> v1 -> v2:
> 1) Add lock protection as Ryusuke suggested and slightly fix commit
> message.
> 2) Fix and add tags.
>
> v2 -> v3:
> Fix commit message to make it clear.

Looks good to me.

Andrew, could you please apply this patch instead of the currently queued patch?

Thanks,
Ryusuke Konishi

> ---
>  fs/nilfs2/sufile.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/nilfs2/sufile.c b/fs/nilfs2/sufile.c
> index 77ff8e95421f..dc359b56fdfa 100644
> --- a/fs/nilfs2/sufile.c
> +++ b/fs/nilfs2/sufile.c
> @@ -495,14 +495,22 @@ void nilfs_sufile_do_free(struct inode *sufile, __u64 segnum,
>  int nilfs_sufile_mark_dirty(struct inode *sufile, __u64 segnum)
>  {
>         struct buffer_head *bh;
> +       void *kaddr;
> +       struct nilfs_segment_usage *su;
>         int ret;
>
> +       down_write(&NILFS_MDT(sufile)->mi_sem);
>         ret = nilfs_sufile_get_segment_usage_block(sufile, segnum, 0, &bh);
>         if (!ret) {
>                 mark_buffer_dirty(bh);
>                 nilfs_mdt_mark_dirty(sufile);
> +               kaddr = kmap_atomic(bh->b_page);
> +               su = nilfs_sufile_block_get_segment_usage(sufile, segnum, bh, kaddr);
> +               nilfs_segment_usage_set_dirty(su);
> +               kunmap_atomic(kaddr);
>                 brelse(bh);
>         }
> +       up_write(&NILFS_MDT(sufile)->mi_sem);
>         return ret;
>  }
>
> --
> 2.17.1
>

      reply	other threads:[~2022-11-21  9:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-21  9:11 [PATCH v3] nilfs2: Fix nilfs_sufile_mark_dirty() not set segment usage as dirty Chen Zhongjin
2022-11-21  9:11 ` Chen Zhongjin
2022-11-21  9:48 ` Ryusuke Konishi [this message]

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=CAKFNMonEeNWnz0OKdSRR-bzg-qPg0eL+OvWrUDfy1vP4QySquw@mail.gmail.com \
    --to=konishi.ryusuke@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=chenzhongjin@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nilfs@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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.