[PATCH] hfsplus: fix xattr entrylength OOB read and NULL hidden_dir on R/W remount

[PATCH] hfsplus: fix xattr entrylength OOB read and NULL hidden_dir on R/W remount

[Hui Peng]

Fix three issues in fs/hfsplus/:

1. In __hfsplus_getxattr() (fs/hfsplus/xattr.c), verify that
   record_length and attr_size fit within fd.entrylength before copying
   from the catalog or attributes btree entry so a malformed attribute
   length cannot trigger a slab-out-of-bounds read or leak uninitialized
   slab memory.
2. In hfsplus_delete_all_attrs() (fs/hfsplus/attributes.c), return early
   if HFSPLUS_SB(sb)->attr_tree is NULL.
3. In hfsplus_reconfigure() and hfsplus_unlink() (fs/hfsplus/super.c,
   fs/hfsplus/dir.c), allocate hidden_dir when remounting from read-only
   to read-write and guard against NULL hidden_dir when unlinking open
   files.

Fixes: 127e5f5ae51e ("hfsplus: rework functionality of getting, setting and deleting of extended attributes")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c
index 7c2e589d4553..a08a9d83ccda 100644
--- a/fs/hfsplus/attributes.c
+++ b/fs/hfsplus/attributes.c
@@ -83,7 +83,7 @@ int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
 
 hfsplus_attr_entry *hfsplus_alloc_attr_entry(void)
 {
-	return kmem_cache_alloc(hfsplus_attr_tree_cachep, GFP_KERNEL);
+	return kmem_cache_zalloc(hfsplus_attr_tree_cachep, GFP_KERNEL);
 }
 
 void hfsplus_destroy_attr_entry(hfsplus_attr_entry *entry)
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 51fcba2e6d40..2967a93433b9 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -386,6 +386,10 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
 	cnid = (u32)(unsigned long)dentry->d_fsdata;
 	if (inode->i_ino == cnid &&
 	    atomic_read(&HFSPLUS_I(inode)->opencnt)) {
+		if (!sbi->hidden_dir) {
+			res = -EIO;
+			goto out;
+		}
 		str.name = name;
 		str.len = sprintf(name, "temp%llu", inode->i_ino);
 		res = hfsplus_rename_cat(inode->i_ino,
@@ -409,6 +413,10 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
 		if (inode->i_ino != cnid) {
 			sbi->file_count--;
 			if (!atomic_read(&HFSPLUS_I(inode)->opencnt)) {
+				if (!sbi->hidden_dir) {
+					res = -EIO;
+					goto out;
+				}
 				res = hfsplus_delete_cat(inode->i_ino,
 							 sbi->hidden_dir,
 							 NULL);
@@ -425,11 +433,10 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
 out:
 	if (!res) {
 		res = hfsplus_cat_write_inode(dir);
-		if (!res) {
+		if (!res && sbi->hidden_dir)
 			res = hfsplus_cat_write_inode(sbi->hidden_dir);
-			if (!res)
-				res = hfsplus_cat_write_inode(inode);
-		}
+		if (!res)
+			res = hfsplus_cat_write_inode(inode);
 	}
 
 	mutex_unlock(&sbi->vh_mutex);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index ff7d6b3336a6..3e5adfe1b4cf 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -401,6 +401,31 @@ static int hfsplus_reconfigure(struct fs_context *fc)
 			sb->s_flags |= SB_RDONLY;
 			fc->sb_flags |= SB_RDONLY;
 		}
+
+		if (!(fc->sb_flags & SB_RDONLY) && !sbi->hidden_dir) {
+			struct inode *root = d_inode(sb->s_root);
+			struct qstr str = QSTR_INIT(HFSP_HIDDENDIR_NAME,
+						    sizeof(HFSP_HIDDENDIR_NAME) - 1);
+			int err;
+
+			mutex_lock(&sbi->vh_mutex);
+			sbi->hidden_dir = hfsplus_new_inode(sb, root, S_IFDIR);
+			if (!sbi->hidden_dir) {
+				mutex_unlock(&sbi->vh_mutex);
+				return -ENOMEM;
+			}
+			err = hfsplus_create_cat(sbi->hidden_dir->i_ino, root,
+						 &str, sbi->hidden_dir);
+			if (err) {
+				iput(sbi->hidden_dir);
+				sbi->hidden_dir = NULL;
+				mutex_unlock(&sbi->vh_mutex);
+				return err;
+			}
+			hfsplus_cat_write_inode(sbi->hidden_dir);
+			hfsplus_cat_write_inode(root);
+			mutex_unlock(&sbi->vh_mutex);
+		}
 	}
 	return 0;
 }
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 21a1c196c71f..7f9215387cbd 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -657,7 +657,9 @@ ssize_t __hfsplus_getxattr(struct inode *inode, const char *name,
 				fd.entryoffset +
 				offsetof(struct hfsplus_attr_inline_data,
 				length));
-		if (record_length > HFSPLUS_MAX_INLINE_DATA_SIZE) {
+		if (record_length > HFSPLUS_MAX_INLINE_DATA_SIZE ||
+		    offsetof(struct hfsplus_attr_inline_data, raw_bytes) +
+			    record_length > fd.entrylength) {
 			pr_err("invalid xattr record size\n");
 			res = -EIO;
 			goto out;