[PATCH for 6.19-rc1] fs: preserve file type in make_bad_inode() unless invalid

[Tetsuo Handa]

syzbot is hitting VFS_BUG_ON_INODE(!S_ISDIR(inode->i_mode)) check
introduced by commit e631df89cd5d ("fs: speed up path lookup with cheaper
handling of MAY_EXEC"), for make_bad_inode() is blindly changing file type
to S_IFREG. Since make_bad_inode() might be called after an inode is fully
constructed, make_bad_inode() should not needlessly change file type.

Reported-by: syzbot+d222f4b7129379c3d5bc@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=d222f4b7129379c3d5bc
Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
---
Should we implement all callbacks (except get_offset_ctx callback which is
currently used by only tmpfs which does not call make_bad_inode()) within
bad_inode_ops, for there might be a callback which is expected to be non-NULL
for !S_IFREG types? Implementing missing callbacks is good for eliminating
possibility of NULL function pointer call. Since VFS is using

    if (!inode->i_op->foo)
        return error;
    inode->i_op->foo();

pattern instead of

    pFoo = READ_ONCE(inode->i_op->foo)
    if (!pFoo)
        return error;
    pFoo();

pattern, suddenly replacing "one i_op with i_op->foo != NULL" with "another
i_op with i_op->foo == NULL" has possibility of NULL pointer function call
(e.g. https://lkml.kernel.org/r/18a58415-4aa9-4cba-97d2-b70384407313@xxxxxxxxxxxxxxxxxxx ).
If we implement missing callbacks, e.g. vfs_fileattr_get() will start
calling security_inode_file_getattr() on bad inode, but we can eliminate
possibility of inode->i_op->fileattr_get == NULL when make_bad_inode() is
called from security_inode_file_getattr() for some reason.

 fs/bad_inode.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 0ef9bcb744dd..ff6c2daecd1c 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -207,7 +207,19 @@ void make_bad_inode(struct inode *inode)
 {
 	remove_inode_hash(inode);
 
-	inode->i_mode = S_IFREG;
+	switch (inode->i_mode & S_IFMT) {
+	case S_IFREG:
+	case S_IFDIR:
+	case S_IFLNK:
+	case S_IFCHR:
+	case S_IFBLK:
+	case S_IFIFO:
+	case S_IFSOCK:
+		inode->i_mode &= S_IFMT;
+		break;
+	default:
+		inode->i_mode = S_IFREG;
+	}
 	simple_inode_init_ts(inode);
 	inode->i_op = &bad_inode_ops;	
 	inode->i_opflags &= ~IOP_XATTR;
-- 
2.47.3