[PATCH] incfs: fix use-after-eviction in incfs_kill_sb()

[PATCH] incfs: fix use-after-eviction in incfs_kill_sb()

[Tamil Mathi]

incfs_kill_sb() called kill_anon_super() before vfs_rmdir() on the
special .index and .incomplete backing directories. kill_anon_super()
calls generic_shutdown_super(), which evicts all inodes. When
vfs_rmdir() is subsequently called on the already-evicted directory
inodes, drop_nlink() is invoked on an inode with i_nlink already 0,
triggering a WARNING in shmem_rmdir() and a null-ptr-deref in ihold().

Fix this by moving the vfs_rmdir() cleanup calls to before
kill_anon_super(), so the directory inodes are still valid when
removed. The existing constraint — that kill_anon_super() must precede
incfs_free_mount_info() — is preserved.

Reported-by: syzbot+64a5c9ae57720239f5c6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=64a5c9ae57720239f5c6
Signed-off-by: Tamil Mathi <9tamilmathi@gmail.com>
---
 fs/incfs/vfs.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/incfs/vfs.c b/fs/incfs/vfs.c
index 4faf2c2..b45533b 100644
--- a/fs/incfs/vfs.c
+++ b/fs/incfs/vfs.c
@@ -1953,15 +1953,14 @@ void incfs_kill_sb(struct super_block *sb)
 	pr_debug("incfs: unmount\n");
 
 	/*
-	 * We must kill the super before freeing mi, since killing the super
-	 * triggers inode eviction, which triggers the final update of the
-	 * backing file, which uses certain information for mi
+	 * Clean up the special backing directories before kill_anon_super(),
+	 * since kill_anon_super() evicts all inodes via generic_shutdown_super().
+	 * Calling vfs_rmdir() after inode eviction causes drop_nlink() to be
+	 * called on an inode with i_nlink already 0, triggering a WARNING in
+	 * shmem_rmdir() and a subsequent null-ptr-deref in ihold().
 	 */
-	kill_anon_super(sb);
-
-	if (mi) {
-		if (mi->mi_backing_dir_path.dentry)
-			dinode = d_inode(mi->mi_backing_dir_path.dentry);
+	if (mi && mi->mi_backing_dir_path.dentry) {
+		dinode = d_inode(mi->mi_backing_dir_path.dentry);
 
 		if (dinode) {
 			if (mi->mi_index_dir && mi->mi_index_free)
@@ -1972,7 +1971,16 @@ void incfs_kill_sb(struct super_block *sb)
 				vfs_rmdir(&nop_mnt_idmap, dinode,
 					  mi->mi_incomplete_dir);
 		}
+	}
 
+	/*
+	 * We must kill the super before freeing mi, since killing the super
+	 * triggers inode eviction, which triggers the final update of the
+	 * backing file, which uses certain information for mi
+	 */
+	kill_anon_super(sb);
+
+	if (mi) {
 		incfs_free_mount_info(mi);
 		sb->s_fs_info = NULL;
 	}
-- 
2.39.5 (Apple Git-154)