Skip to content

Commit 8cccf05

Browse files
konisakpm00
authored andcommitted
nilfs2: fix use-after-free bug of ns_writer on remount
If a nilfs2 filesystem is downgraded to read-only due to metadata corruption on disk and is remounted read/write, or if emergency read-only remount is performed, detaching a log writer and synchronizing the filesystem can be done at the same time. In these cases, use-after-free of the log writer (hereinafter nilfs->ns_writer) can happen as shown in the scenario below: Task1 Task2 -------------------------------- ------------------------------ nilfs_construct_segment nilfs_segctor_sync init_wait init_waitqueue_entry add_wait_queue schedule nilfs_remount (R/W remount case) nilfs_attach_log_writer 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 scenario diagram is based on the Shigeru Yoshida's post [1]. This patch fixes the issue by not detaching nilfs->ns_writer on remount so that this UAF race doesn't happen. Along with this change, this patch also inserts a few necessary read-only checks with superblock instance where only the ns_writer pointer was used to check if the filesystem is read-only. Link: https://syzkaller.appspot.com/bug?id=79a4c002e960419ca173d55e863bd09e8112df8b Link: https://lkml.kernel.org/r/20221103141759.1836312-1-syoshida@redhat.com [1] Link: https://lkml.kernel.org/r/20221104142959.28296-1-konishi.ryusuke@gmail.com Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com> Reported-by: syzbot+f816fa82f8783f7a02bb@syzkaller.appspotmail.com Reported-by: Shigeru Yoshida <syoshida@redhat.com> Tested-by: Ryusuke Konishi <konishi.ryusuke@gmail.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent ba54d19 commit 8cccf05

File tree

2 files changed

+8
-9
lines changed

2 files changed

+8
-9
lines changed

fs/nilfs2/segment.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ void nilfs_relax_pressure_in_lock(struct super_block *sb)
317317
struct the_nilfs *nilfs = sb->s_fs_info;
318318
struct nilfs_sc_info *sci = nilfs->ns_writer;
319319

320-
if (!sci || !sci->sc_flush_request)
320+
if (sb_rdonly(sb) || unlikely(!sci) || !sci->sc_flush_request)
321321
return;
322322

323323
set_bit(NILFS_SC_PRIOR_FLUSH, &sci->sc_flags);
@@ -2242,7 +2242,7 @@ int nilfs_construct_segment(struct super_block *sb)
22422242
struct nilfs_sc_info *sci = nilfs->ns_writer;
22432243
struct nilfs_transaction_info *ti;
22442244

2245-
if (!sci)
2245+
if (sb_rdonly(sb) || unlikely(!sci))
22462246
return -EROFS;
22472247

22482248
/* A call inside transactions causes a deadlock. */
@@ -2280,7 +2280,7 @@ int nilfs_construct_dsync_segment(struct super_block *sb, struct inode *inode,
22802280
struct nilfs_transaction_info ti;
22812281
int err = 0;
22822282

2283-
if (!sci)
2283+
if (sb_rdonly(sb) || unlikely(!sci))
22842284
return -EROFS;
22852285

22862286
nilfs_transaction_lock(sb, &ti, 0);
@@ -2776,11 +2776,12 @@ int nilfs_attach_log_writer(struct super_block *sb, struct nilfs_root *root)
27762776

27772777
if (nilfs->ns_writer) {
27782778
/*
2779-
* This happens if the filesystem was remounted
2780-
* read/write after nilfs_error degenerated it into a
2781-
* read-only mount.
2779+
* This happens if the filesystem is made read-only by
2780+
* __nilfs_error or nilfs_remount and then remounted
2781+
* read/write. In these cases, reuse the existing
2782+
* writer.
27822783
*/
2783-
nilfs_detach_log_writer(sb);
2784+
return 0;
27842785
}
27852786

27862787
nilfs->ns_writer = nilfs_segctor_new(sb, root);

fs/nilfs2/super.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,8 +1133,6 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
11331133
if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
11341134
goto out;
11351135
if (*flags & SB_RDONLY) {
1136-
/* Shutting down log writer */
1137-
nilfs_detach_log_writer(sb);
11381136
sb->s_flags |= SB_RDONLY;
11391137

11401138
/*

0 commit comments

Comments
 (0)