hfsplus: fix failed mount handling

Currently the error handling in hfsplus_fill_super is a mess, and can
lead to accessing fields in the superblock that haven't been even set
up yet.  Fix this by making sure we do not set up sb->s_root until we
have the mount fully set up, and before that do proper step by step
unwinding instead of using hfsplus_put_super as a big hammer.

Reported-by: Dan Williams <dcbw@redhat.com>
Signed-off-by: Christoph Hellwig <hch@tuxera.com>

1 files changed, 61 insertions, 45 deletions

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 9a3b4795f43..b49b55584c8 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -338,20 +338,22 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
         struct inode *root, *inode;
         struct qstr str;
         struct nls_table *nls = NULL;
-        int err = -EINVAL;
+        int err;
 
+        err = -EINVAL;
         sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
         if (!sbi)
-                return -ENOMEM;
+                goto out;
 
         sb->s_fs_info = sbi;
         mutex_init(&sbi->alloc_mutex);
         mutex_init(&sbi->vh_mutex);
         hfsplus_fill_defaults(sbi);
+
+        err = -EINVAL;
         if (!hfsplus_parse_options(data, sbi)) {
                 printk(KERN_ERR "hfs: unable to parse mount options\n");
-                err = -EINVAL;
-                goto cleanup;
+                goto out_unload_nls;
         }
 
         /* temporarily use utf8 to correctly find the hidden dir below */
@@ -359,16 +361,14 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
         sbi->nls = load_nls("utf8");
         if (!sbi->nls) {
                 printk(KERN_ERR "hfs: unable to load nls for utf8\n");
-                err = -EINVAL;
-                goto cleanup;
+                goto out_unload_nls;
         }
 
         /* Grab the volume header */
         if (hfsplus_read_wrapper(sb)) {
                 if (!silent)
                         printk(KERN_WARNING "hfs: unable to find HFS+ superblock\n");
-                err = -EINVAL;
-                goto cleanup;
+                goto out_unload_nls;
         }
         vhdr = sbi->s_vhdr;
 
@@ -377,7 +377,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
         if (be16_to_cpu(vhdr->version) < HFSPLUS_MIN_VERSION ||
             be16_to_cpu(vhdr->version) > HFSPLUS_CURRENT_VERSION) {
                 printk(KERN_ERR "hfs: wrong filesystem version\n");
-                goto cleanup;
+                goto out_free_vhdr;
         }
         sbi->total_blocks = be32_to_cpu(vhdr->total_blocks);
         sbi->free_blocks = be32_to_cpu(vhdr->free_blocks);
@@ -421,19 +421,19 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
         sbi->ext_tree = hfs_btree_open(sb, HFSPLUS_EXT_CNID);
         if (!sbi->ext_tree) {
                 printk(KERN_ERR "hfs: failed to load extents file\n");
-                goto cleanup;
+                goto out_free_vhdr;
         }
         sbi->cat_tree = hfs_btree_open(sb, HFSPLUS_CAT_CNID);
         if (!sbi->cat_tree) {
                 printk(KERN_ERR "hfs: failed to load catalog file\n");
-                goto cleanup;
+                goto out_close_ext_tree;
         }
 
         inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID);
         if (IS_ERR(inode)) {
                 printk(KERN_ERR "hfs: failed to load allocation file\n");
                 err = PTR_ERR(inode);
-                goto cleanup;
+                goto out_close_cat_tree;
         }
         sbi->alloc_file = inode;
 
@@ -442,14 +442,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
         if (IS_ERR(root)) {
                 printk(KERN_ERR "hfs: failed to load root directory\n");
                 err = PTR_ERR(root);
-                goto cleanup;
-        }
-        sb->s_d_op = &hfsplus_dentry_operations;
-        sb->s_root = d_alloc_root(root);
-        if (!sb->s_root) {
-                iput(root);
-                err = -ENOMEM;
-                goto cleanup;
+                goto out_put_alloc_file;
         }
 
         str.len = sizeof(HFSP_HIDDENDIR_NAME) - 1;
@@ -459,46 +452,69 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
         if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
                 hfs_find_exit(&fd);
                 if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
-                        goto cleanup;
+                        goto out_put_root;
                 inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
                 if (IS_ERR(inode)) {
                         err = PTR_ERR(inode);
-                        goto cleanup;
+                        goto out_put_root;
                 }
                 sbi->hidden_dir = inode;
         } else
                 hfs_find_exit(&fd);
 
-        if (sb->s_flags & MS_RDONLY)
-                goto out;
+        if (!(sb->s_flags & MS_RDONLY)) {
+                /*
+                 * H+LX == hfsplusutils, H+Lx == this driver, H+lx is unused
+                 * all three are registered with Apple for our use
+                 */
+                vhdr->last_mount_vers = cpu_to_be32(HFSP_MOUNT_VERSION);
+                vhdr->modify_date = hfsp_now2mt();
+                be32_add_cpu(&vhdr->write_count, 1);
+                vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_UNMNT);
+                vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_INCNSTNT);
+                hfsplus_sync_fs(sb, 1);
 
-        /* H+LX == hfsplusutils, H+Lx == this driver, H+lx is unused
-         * all three are registered with Apple for our use
-         */
-        vhdr->last_mount_vers = cpu_to_be32(HFSP_MOUNT_VERSION);
-        vhdr->modify_date = hfsp_now2mt();
-        be32_add_cpu(&vhdr->write_count, 1);
-        vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_UNMNT);
-        vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_INCNSTNT);
-        hfsplus_sync_fs(sb, 1);
-
-        if (!sbi->hidden_dir) {
-                mutex_lock(&sbi->vh_mutex);
-                sbi->hidden_dir = hfsplus_new_inode(sb, S_IFDIR);
-                hfsplus_create_cat(sbi->hidden_dir->i_ino, sb->s_root->d_inode,
-                                   &str, sbi->hidden_dir);
-                mutex_unlock(&sbi->vh_mutex);
-
-                hfsplus_mark_inode_dirty(sbi->hidden_dir, HFSPLUS_I_CAT_DIRTY);
+                if (!sbi->hidden_dir) {
+                        mutex_lock(&sbi->vh_mutex);
+                        sbi->hidden_dir = hfsplus_new_inode(sb, S_IFDIR);
+                        hfsplus_create_cat(sbi->hidden_dir->i_ino, root, &str,
+                                           sbi->hidden_dir);
+                        mutex_unlock(&sbi->vh_mutex);
+
+                        hfsplus_mark_inode_dirty(sbi->hidden_dir,
+                                                 HFSPLUS_I_CAT_DIRTY);
+                }
         }
-out:
+
+        sb->s_d_op = &hfsplus_dentry_operations;
+        sb->s_root = d_alloc_root(root);
+        if (!sb->s_root) {
+                err = -ENOMEM;
+                goto out_put_hidden_dir;
+        }
+
         unload_nls(sbi->nls);
         sbi->nls = nls;
         return 0;
 
-cleanup:
-        hfsplus_put_super(sb);
+out_put_hidden_dir:
+        iput(sbi->hidden_dir);
+out_put_root:
+        iput(sbi->alloc_file);
+out_put_alloc_file:
+        iput(sbi->alloc_file);
+out_close_cat_tree:
+        hfs_btree_close(sbi->cat_tree);
+out_close_ext_tree:
+        hfs_btree_close(sbi->ext_tree);
+out_free_vhdr:
+        kfree(sbi->s_vhdr);
+        kfree(sbi->s_backup_vhdr);
+out_unload_nls:
+        unload_nls(sbi->nls);
         unload_nls(nls);
+        kfree(sbi);
+out:
         return err;
 }