Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Shigeru Yoshida <syoshida@redhat.com>
To:     konishi.ryusuke@gmail.com
Cc:     linux-nilfs@vger.kernel.org, linux-kernel@vger.kernel.org,
        syzkaller-bugs@googlegroups.com,
        Shigeru Yoshida <syoshida@redhat.com>,
        syzbot+f816fa82f8783f7a02bb@syzkaller.appspotmail.com
Subject: [PATCH] nilfs: Avoid use-after-free caused by nilfs->ns_writer
Date:   Thu,  3 Nov 2022 23:17:59 +0900
Message-Id: <20221103141759.1836312-1-syoshida@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

syzbot reported use-after-free in nilfs_segctor_sync() [1].

The use-after-free occurs with nilfs->ns_writer.  The scenario for the
issue is as follows:

Task1                                   Task2
----------------------------------------------------------------------
nilfs_construct_segment
  nilfs_segctor_sync
    init_wait
    init_waitqueue_entry
    add_wait_queue
    schedule
                                        nilfs_detach_log_writer
                                          nilfs_segctor_destroy
                                            kfree
    finish_wait
      _raw_spin_lock_irqsave
        __raw_spin_lock_irqsave
          do_raw_spin_lock
            debug_spin_lock_before  <-- use-after-free

While Task1 is sleeping, nilfs->ns_writer is freed by Task2.  After
Task1 waked up, Task1 accesses nilfs->ns_writer which is already
freed.

This patch fixes the issue by taking nilfs->ns_segctor_sem in
nilfs_construct_segment() so that nilfs->ns_segctor_sem cannot be
freed while nilfs_segctor_sync() is sleeping.

Link: https://syzkaller.appspot.com/bug?id=79a4c002e960419ca173d55e863bd09e8112df8b [1]
Reported-by: syzbot+f816fa82f8783f7a02bb@syzkaller.appspotmail.com
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
 fs/nilfs2/segment.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index b4cebad21b48..d4f10d82664d 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2239,16 +2239,24 @@ static void nilfs_segctor_wakeup(struct nilfs_sc_info *sci, int err)
 int nilfs_construct_segment(struct super_block *sb)
 {
 	struct the_nilfs *nilfs = sb->s_fs_info;
-	struct nilfs_sc_info *sci = nilfs->ns_writer;
+	struct nilfs_sc_info *sci;
 	struct nilfs_transaction_info *ti;
+	int ret;
 
-	if (!sci)
+	down_write(&nilfs->ns_segctor_sem);
+	sci = nilfs->ns_writer;
+	if (!sci) {
+		up_write(&nilfs->ns_segctor_sem);
 		return -EROFS;
+	}
 
 	/* A call inside transactions causes a deadlock. */
 	BUG_ON((ti = current->journal_info) && ti->ti_magic == NILFS_TI_MAGIC);
 
-	return nilfs_segctor_sync(sci);
+	ret = nilfs_segctor_sync(sci);
+	up_write(&nilfs->ns_segctor_sem);
+
+	return ret;
 }
 
 /**
-- 
2.37.3