If no hidden directory exists, the hfsplus_fill_super() function will
create it. A delayed work is then queued to sync the superblock, which
is never canceled in case of failure.
Also, if the filesystem is corrupted in such a way that the hidden
directory is not of type HFSPLUS_FOLDER, the mount will fail without
throwing an error code. The vfs layer is then forced to dereference a
NULL root dentry.
To fix these issues, return an error and allow ->put_super() to take
care of most of the cleanup if failure occurs after sb->s_root has been
set.
Before this patch the volume was simply flagged as inconsistent if the
mount failed while creating the hidden directory. Since ->put_super()
will now toggle those flags, we need to correct the folder count before
returning, with a call to hfsplus_delete_inode(). Simplify this by using
a new hfsplus_create_inode() function that can handle its own cleanup.
Reported-by: syzbot+4f2e5f086147d543ab03@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: 5bd9d99d107c ("hfsplus: add error checking for hfs_find_init()")
Fixes: 9e6c5829b07c ("hfsplus: get rid of write_super")
Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@xxxxxxxxx>
---
Changes in v3:
Move the code that creates the hidden dir into a separate function. I
think this is more reasonable. The new function can also be called by
hfsplus_mknod().
fs/hfsplus/dir.c | 28 +++-----------------------
fs/hfsplus/hfsplus_fs.h | 2 ++
fs/hfsplus/inode.c | 32 +++++++++++++++++++++++++++++
fs/hfsplus/super.c | 53 +++++++++++++------------------------------------
4 files changed, 51 insertions(+), 64 deletions(-)
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 15e06fb552da..2446e5db1dfb 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -483,37 +483,15 @@ static int hfsplus_mknod(struct inode *dir, struct dentry *dentry,
{
struct hfsplus_sb_info *sbi = HFSPLUS_SB(dir->i_sb);
struct inode *inode;
- int res = -ENOMEM;
+ int res;
mutex_lock(&sbi->vh_mutex);
- inode = hfsplus_new_inode(dir->i_sb, dir, mode);
- if (!inode)
- goto out;
-
- if (S_ISBLK(mode) || S_ISCHR(mode) || S_ISFIFO(mode) || S_ISSOCK(mode))
- init_special_inode(inode, mode, rdev);
-
- res = hfsplus_create_cat(inode->i_ino, dir, &dentry->d_name, inode);
+ res = hfsplus_create_inode(dir, &dentry->d_name, mode, rdev, &inode);
if (res)
- goto failed_mknod;
-
- res = hfsplus_init_inode_security(inode, dir, &dentry->d_name);
- if (res == -EOPNOTSUPP)
- res = 0; /* Operation is not supported. */
- else if (res) {
- /* Try to delete anyway without error analysis. */
- hfsplus_delete_cat(inode->i_ino, dir, &dentry->d_name);
- goto failed_mknod;
- }
-
+ goto out;
hfsplus_instantiate(dentry, inode, inode->i_ino);
mark_inode_dirty(inode);
- goto out;
-failed_mknod:
- clear_nlink(inode);
- hfsplus_delete_inode(inode);
- iput(inode);
out:
mutex_unlock(&sbi->vh_mutex);
return res;
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index d9255abafb81..05f30ae788f0 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -480,6 +480,8 @@ extern const struct dentry_operations hfsplus_dentry_operations;
struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
umode_t mode);
+int hfsplus_create_inode(struct inode *dir, struct qstr *name, umode_t mode,
+ dev_t rdev, struct inode **inode);
void hfsplus_delete_inode(struct inode *inode);
void hfsplus_inode_read_fork(struct inode *inode,
struct hfsplus_fork_raw *fork);
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index c0c8d433864f..b15bb83deda1 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -413,6 +413,38 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
return inode;
}
+int hfsplus_create_inode(struct inode *dir, struct qstr *name, umode_t mode,
+ dev_t rdev, struct inode **inode)
+{
+ int res;
+
+ *inode = hfsplus_new_inode(dir->i_sb, dir, mode);
+ if (!*inode)
+ return -ENOMEM;
+
+ if (S_ISBLK(mode) || S_ISCHR(mode) || S_ISFIFO(mode) || S_ISSOCK(mode))
+ init_special_inode(*inode, mode, rdev);
+
+ res = hfsplus_create_cat((*inode)->i_ino, dir, name, *inode);
+ if (res)
+ goto fail;
+
+ res = hfsplus_init_inode_security(*inode, dir, name);
+ if (res && res != -EOPNOTSUPP) {
+ /* Try to delete anyway without error analysis. */
+ hfsplus_delete_cat((*inode)->i_ino, dir, name);
+ goto fail;
+ }
+ return 0;
+
+fail:
+ clear_nlink(*inode);
+ hfsplus_delete_inode(*inode);
+ iput(*inode);
+ *inode = NULL;
+ return res;
+}
+
void hfsplus_delete_inode(struct inode *inode)
{
struct super_block *sb = inode->i_sb;
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 513c357c734b..3749d9381c56 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -514,22 +514,26 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
goto out_put_alloc_file;
}
+ /* From here on, most of the cleanup is handled by ->put_super() */
+
str.len = sizeof(HFSP_HIDDENDIR_NAME) - 1;
str.name = HFSP_HIDDENDIR_NAME;
err = hfs_find_init(sbi->cat_tree, &fd);
if (err)
- goto out_put_root;
+ goto out;
err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
if (unlikely(err < 0))
- goto out_put_root;
+ goto out;
if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
hfs_find_exit(&fd);
- if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
- goto out_put_root;
+ if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
+ err = -EINVAL;
+ goto out;
+ }
inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
- goto out_put_root;
+ goto out;
}
sbi->hidden_dir = inode;
} else
@@ -549,35 +553,11 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
if (!sbi->hidden_dir) {
mutex_lock(&sbi->vh_mutex);
- sbi->hidden_dir = hfsplus_new_inode(sb, root, S_IFDIR);
- if (!sbi->hidden_dir) {
- mutex_unlock(&sbi->vh_mutex);
- err = -ENOMEM;
- goto out_put_root;
- }
- err = hfsplus_create_cat(sbi->hidden_dir->i_ino, root,
- &str, sbi->hidden_dir);
- if (err) {
- mutex_unlock(&sbi->vh_mutex);
- goto out_put_hidden_dir;
- }
-
- err = hfsplus_init_inode_security(sbi->hidden_dir,
- root, &str);
- if (err == -EOPNOTSUPP)
- err = 0; /* Operation is not supported. */
- else if (err) {
- /*
- * Try to delete anyway without
- * error analysis.
- */
- hfsplus_delete_cat(sbi->hidden_dir->i_ino,
- root, &str);
- mutex_unlock(&sbi->vh_mutex);
- goto out_put_hidden_dir;
- }
-
+ err = hfsplus_create_inode(root, &str, S_IFDIR, 0,
+ &sbi->hidden_dir);
mutex_unlock(&sbi->vh_mutex);
+ if (err)
+ goto out;
hfsplus_mark_inode_dirty(sbi->hidden_dir,
HFSPLUS_I_CAT_DIRTY);
}
@@ -587,11 +567,6 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
sbi->nls = nls;
return 0;
-out_put_hidden_dir:
- iput(sbi->hidden_dir);
-out_put_root:
- dput(sb->s_root);
- sb->s_root = NULL;
out_put_alloc_file:
iput(sbi->alloc_file);
out_close_attr_tree:
@@ -605,9 +580,9 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
kfree(sbi->s_backup_vhdr_buf);
out_unload_nls:
unload_nls(sbi->nls);
- unload_nls(nls);
kfree(sbi);
out:
+ unload_nls(nls);
return err;
}
--
2.11.0
