• Ryusuke Konishi's avatar
    nilfs2: fix use-after-free of nilfs_root in dirtying inodes via iput · f8654743
    Ryusuke Konishi authored
    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
    9b5a04ac ("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 0ae45f63 ("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 28a65b49 ("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 9b5a04ac ("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.comSigned-off-by: default avatarRyusuke Konishi <konishi.ryusuke@gmail.com>
    Reported-by: syzbot+74db8b3087f293d3a13a@syzkaller.appspotmail.com
    Closes: https://lkml.kernel.org/r/000000000000b4e906060113fd63@google.com
    Fixes: 0ae45f63 ("vfs: add support for a lazytime mount option")
    Tested-by: default avatarRyusuke Konishi <konishi.ryusuke@gmail.com>
    Cc: <stable@vger.kernel.org> # 4.0+
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    f8654743
segment.c 75.2 KB