6.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp
[ Upstream commit 005d4b0d33f6b4a23d382b7930f7a96b95b01f39 ]
syzbot is reporting that S_IFMT bits of inode->i_mode can become bogus when the S_IFMT bits of the 16bits "mode" field loaded from disk are corrupted.
According to [1], the permissions field was treated as reserved in Mac OS 8 and 9. According to [2], the reserved field was explicitly initialized with 0, and that field must remain 0 as long as reserved. Therefore, when the "mode" field is not 0 (i.e. no longer reserved), the file must be S_IFDIR if dir == 1, and the file must be one of S_IFREG/S_IFLNK/S_IFCHR/ S_IFBLK/S_IFIFO/S_IFSOCK if dir == 0.
Reported-by: syzbot syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d Link: https://developer.apple.com/library/archive/technotes/tn/tn1150.html#HFSPlus... [1] Link: https://developer.apple.com/library/archive/technotes/tn/tn1150.html#Reserve... [2] Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp Reviewed-by: Viacheslav Dubeyko slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko slava@dubeyko.com Link: https://lore.kernel.org/r/04ded9f9-73fb-496c-bfa5-89c4f5d1d7bb@I-love.SAKURA... Signed-off-by: Viacheslav Dubeyko slava@dubeyko.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/hfsplus/inode.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index b51a411ecd237..e290e417ed3a7 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -180,13 +180,29 @@ const struct dentry_operations hfsplus_dentry_operations = { .d_compare = hfsplus_compare_dentry, };
-static void hfsplus_get_perms(struct inode *inode, - struct hfsplus_perm *perms, int dir) +static int hfsplus_get_perms(struct inode *inode, + struct hfsplus_perm *perms, int dir) { struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb); u16 mode;
mode = be16_to_cpu(perms->mode); + if (dir) { + if (mode && !S_ISDIR(mode)) + goto bad_type; + } else if (mode) { + switch (mode & S_IFMT) { + case S_IFREG: + case S_IFLNK: + case S_IFCHR: + case S_IFBLK: + case S_IFIFO: + case S_IFSOCK: + break; + default: + goto bad_type; + } + }
i_uid_write(inode, be32_to_cpu(perms->owner)); if ((test_bit(HFSPLUS_SB_UID, &sbi->flags)) || (!i_uid_read(inode) && !mode)) @@ -212,6 +228,10 @@ static void hfsplus_get_perms(struct inode *inode, inode->i_flags |= S_APPEND; else inode->i_flags &= ~S_APPEND; + return 0; +bad_type: + pr_err("invalid file type 0%04o for inode %lu\n", mode, inode->i_ino); + return -EIO; }
static int hfsplus_file_open(struct inode *inode, struct file *file) @@ -516,7 +536,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd) } hfs_bnode_read(fd->bnode, &entry, fd->entryoffset, sizeof(struct hfsplus_cat_folder)); - hfsplus_get_perms(inode, &folder->permissions, 1); + res = hfsplus_get_perms(inode, &folder->permissions, 1); + if (res) + goto out; set_nlink(inode, 1); inode->i_size = 2 + be32_to_cpu(folder->valence); inode_set_atime_to_ts(inode, hfsp_mt2ut(folder->access_date)); @@ -545,7 +567,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
hfsplus_inode_read_fork(inode, HFSPLUS_IS_RSRC(inode) ? &file->rsrc_fork : &file->data_fork); - hfsplus_get_perms(inode, &file->permissions, 0); + res = hfsplus_get_perms(inode, &file->permissions, 0); + if (res) + goto out; set_nlink(inode, 1); if (S_ISREG(inode->i_mode)) { if (file->permissions.dev)