Skip to content

Commit f865474

Browse files
konisakpm00
authored andcommitted
nilfs2: fix use-after-free of nilfs_root in dirtying inodes via iput
During unmount process of nilfs2, nothing holds nilfs_root structure after nilfs2 detaches its writer in nilfs_detach_log_writer(). Previously, nilfs_evict_inode() could cause use-after-free read for nilfs_root if inodes are left in "garbage_list" and released by nilfs_dispose_list at the end of nilfs_detach_log_writer(), and this bug was fixed by commit 9b5a04a ("nilfs2: fix use-after-free bug of nilfs_root in nilfs_evict_inode()"). However, it turned out that there is another possibility of UAF in the call path where mark_inode_dirty_sync() is called from iput(): nilfs_detach_log_writer() nilfs_dispose_list() iput() mark_inode_dirty_sync() __mark_inode_dirty() nilfs_dirty_inode() __nilfs_mark_inode_dirty() nilfs_load_inode_block() --> causes UAF of nilfs_root struct This can happen after commit 0ae45f6 ("vfs: add support for a lazytime mount option"), which changed iput() to call mark_inode_dirty_sync() on its final reference if i_state has I_DIRTY_TIME flag and i_nlink is non-zero. This issue appears after commit 28a65b4 ("nilfs2: do not write dirty data after degenerating to read-only") when using the syzbot reproducer, but the issue has potentially existed before. Fix this issue by adding a "purging flag" to the nilfs structure, setting that flag while disposing the "garbage_list" and checking it in __nilfs_mark_inode_dirty(). Unlike commit 9b5a04a ("nilfs2: fix use-after-free bug of nilfs_root in nilfs_evict_inode()"), this patch does not rely on ns_writer to determine whether to skip operations, so as not to break recovery on mount. The nilfs_salvage_orphan_logs routine dirties the buffer of salvaged data before attaching the log writer, so changing __nilfs_mark_inode_dirty() to skip the operation when ns_writer is NULL will cause recovery write to fail. The purpose of using the cleanup-only flag is to allow for narrowing of such conditions. Link: https://lkml.kernel.org/r/20230728191318.33047-1-konishi.ryusuke@gmail.com Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com> Reported-by: syzbot+74db8b3087f293d3a13a@syzkaller.appspotmail.com Closes: https://lkml.kernel.org/r/000000000000b4e906060113fd63@google.com Fixes: 0ae45f6 ("vfs: add support for a lazytime mount option") Tested-by: Ryusuke Konishi <konishi.ryusuke@gmail.com> Cc: <stable@vger.kernel.org> # 4.0+ Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent fac2650 commit f865474

File tree

3 files changed

+12
-0
lines changed

3 files changed

+12
-0
lines changed

fs/nilfs2/inode.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,9 +1101,17 @@ int nilfs_set_file_dirty(struct inode *inode, unsigned int nr_dirty)
11011101

11021102
int __nilfs_mark_inode_dirty(struct inode *inode, int flags)
11031103
{
1104+
struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
11041105
struct buffer_head *ibh;
11051106
int err;
11061107

1108+
/*
1109+
* Do not dirty inodes after the log writer has been detached
1110+
* and its nilfs_root struct has been freed.
1111+
*/
1112+
if (unlikely(nilfs_purging(nilfs)))
1113+
return 0;
1114+
11071115
err = nilfs_load_inode_block(inode, &ibh);
11081116
if (unlikely(err)) {
11091117
nilfs_warn(inode->i_sb,

fs/nilfs2/segment.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2845,6 +2845,7 @@ void nilfs_detach_log_writer(struct super_block *sb)
28452845
nilfs_segctor_destroy(nilfs->ns_writer);
28462846
nilfs->ns_writer = NULL;
28472847
}
2848+
set_nilfs_purging(nilfs);
28482849

28492850
/* Force to free the list of dirty files */
28502851
spin_lock(&nilfs->ns_inode_lock);
@@ -2857,4 +2858,5 @@ void nilfs_detach_log_writer(struct super_block *sb)
28572858
up_write(&nilfs->ns_segctor_sem);
28582859

28592860
nilfs_dispose_list(nilfs, &garbage_list, 1);
2861+
clear_nilfs_purging(nilfs);
28602862
}

fs/nilfs2/the_nilfs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ enum {
2929
THE_NILFS_DISCONTINUED, /* 'next' pointer chain has broken */
3030
THE_NILFS_GC_RUNNING, /* gc process is running */
3131
THE_NILFS_SB_DIRTY, /* super block is dirty */
32+
THE_NILFS_PURGING, /* disposing dirty files for cleanup */
3233
};
3334

3435
/**
@@ -208,6 +209,7 @@ THE_NILFS_FNS(INIT, init)
208209
THE_NILFS_FNS(DISCONTINUED, discontinued)
209210
THE_NILFS_FNS(GC_RUNNING, gc_running)
210211
THE_NILFS_FNS(SB_DIRTY, sb_dirty)
212+
THE_NILFS_FNS(PURGING, purging)
211213

212214
/*
213215
* Mount option operations

0 commit comments

Comments
 (0)