Proper HFS+ mount failure fix from hch

5 files changed, 212 insertions, 86 deletions

diff --git a/hfsplus-02-fill-super-skip-cleanup.patch b/hfsplus-02-fill-super-skip-cleanup.patch
deleted file mode 100644
index d81d51dcf..000000000
--- a/hfsplus-02-fill-super-skip-cleanup.patch
+++ /dev/null
@@ -1,38 +0,0 @@
-hfsplus: Skip cleanup on early failures
-
-Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
-
---- vanilla-2.6.38-rc2-git9.orig/fs/hfsplus/super.c
-+++ vanilla-2.6.38-rc2-git9/fs/hfsplus/super.c
-@@ -344,14 +344,13 @@ static int hfsplus_fill_super(struct sup
- 	if (!sbi)
- 		return -ENOMEM;
- 
--	sb->s_fs_info = sbi;
- 	mutex_init(&sbi->alloc_mutex);
- 	mutex_init(&sbi->vh_mutex);
- 	hfsplus_fill_defaults(sbi);
- 	if (!hfsplus_parse_options(data, sbi)) {
- 		printk(KERN_ERR "hfs: unable to parse mount options\n");
--		err = -EINVAL;
--		goto cleanup;
-+		kfree(sbi);
-+		return -EINVAL;
- 	}
- 
- 	/* temporarily use utf8 to correctly find the hidden dir below */
-@@ -359,10 +358,12 @@ static int hfsplus_fill_super(struct sup
- 	sbi->nls = load_nls("utf8");
- 	if (!sbi->nls) {
- 		printk(KERN_ERR "hfs: unable to load nls for utf8\n");
--		err = -EINVAL;
--		goto cleanup;
-+		kfree(sbi);
-+		return -EINVAL;
- 	}
- 
-+	sb->s_fs_info = sbi;
-+
- 	/* Grab the volume header */
- 	if (hfsplus_read_wrapper(sb)) {
- 		if (!silent)
diff --git a/hfsplus-03-zero-vhdr-on-free.patch b/hfsplus-03-zero-vhdr-on-free.patch
index 6d3f8df22..020789ec5 100644
--- a/hfsplus-03-zero-vhdr-on-free.patch
+++ b/hfsplus-03-zero-vhdr-on-free.patch
@@ -1,8 +1,4 @@
-hfsplus: Clear volume header pointers on failure
-
-The next patch will use NULL volume header to determine whether
-to flush the superblock. Also fix two failure cases so they
-clear the headers before exiting.
+hfsplus: Fix two memory leaks in wrapper.c
 
 Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
 
@@ -26,14 +22,3 @@ Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
  		goto reread;
  	}
  
-@@ -230,8 +230,10 @@ reread:
- 
- out_free_backup_vhdr:
- 	kfree(sbi->s_backup_vhdr);
-+	sbi->s_backup_vhdr = NULL;
- out_free_vhdr:
- 	kfree(sbi->s_vhdr);
-+	sbi->s_vhdr = NULL;
- out:
- 	return error;
- }
diff --git a/hfsplus-04-check-for-vhdr.patch b/hfsplus-04-check-for-vhdr.patch
deleted file mode 100644
index b31e40d98..000000000
--- a/hfsplus-04-check-for-vhdr.patch
+++ /dev/null
@@ -1,28 +0,0 @@
-hfsplus: Check for NULL volume header
-
-If volume header is null there is not much to do in put_super().
-
-Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
-
---- vanilla-2.6.38-rc2-git9.orig/fs/hfsplus/super.c
-+++ vanilla-2.6.38-rc2-git9/fs/hfsplus/super.c
-@@ -237,7 +237,10 @@ static void hfsplus_put_super(struct sup
- 	if (!sb->s_fs_info)
- 		return;
- 
--	if (!(sb->s_flags & MS_RDONLY) && sbi->s_vhdr) {
-+	if (!sbi->s_vhdr)
-+		goto out_unload_nls;
-+
-+	if (!(sb->s_flags & MS_RDONLY)) {
- 		struct hfsplus_vh *vhdr = sbi->s_vhdr;
- 
- 		vhdr->modify_date = hfsp_now2mt();
-@@ -253,6 +256,7 @@ static void hfsplus_put_super(struct sup
- 	iput(sbi->hidden_dir);
- 	kfree(sbi->s_vhdr);
- 	kfree(sbi->s_backup_vhdr);
-+out_unload_nls:
- 	unload_nls(sbi->nls);
- 	kfree(sb->s_fs_info);
- 	sb->s_fs_info = NULL;
diff --git a/hfsplus-05-fix-failed-mount.patch b/hfsplus-05-fix-failed-mount.patch
new file mode 100644
index 000000000..10fdad4b1
--- /dev/null
+++ b/hfsplus-05-fix-failed-mount.patch
@@ -0,0 +1,209 @@
+From: Christoph Hellwig <hch@tuxera.com>
+Subject: 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>
+
+Index: linux-2.6/fs/hfsplus/super.c
+===================================================================
+--- linux-2.6.orig/fs/hfsplus/super.c	2011-02-01 12:48:09.984663687 -0700
++++ linux-2.6/fs/hfsplus/super.c	2011-02-01 13:17:41.803164619 -0700
+@@ -338,20 +338,28 @@ static int hfsplus_fill_super(struct sup
+ 	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;
++
++		/*
++		 * hfsplus_parse_options sets sbi->nls, but we are going
++		 * to free the localy cached nls in the cleanup path.
++		 */
++		nls = sbi->nls;
++		goto out_unload_nls;
+ 	}
+ 
+ 	/* temporarily use utf8 to correctly find the hidden dir below */
+@@ -359,16 +367,14 @@ static int hfsplus_fill_super(struct sup
+ 	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_free_vhdr;
+ 	}
+ 	vhdr = sbi->s_vhdr;
+ 
+@@ -377,7 +383,7 @@ static int hfsplus_fill_super(struct sup
+ 	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 +427,19 @@ static int hfsplus_fill_super(struct sup
+ 	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 +448,7 @@ static int hfsplus_fill_super(struct sup
+ 	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 +458,68 @@ static int hfsplus_fill_super(struct sup
+ 	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);
++
++		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);
+ 
+-	/* 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);
++		}
++	}
+ 
+-		hfsplus_mark_inode_dirty(sbi->hidden_dir, HFSPLUS_I_CAT_DIRTY);
++	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;
+ 	}
+-out:
++
+ 	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(nls);
++	kfree(sbi);
++out:
+ 	return err;
+ }
+ 
diff --git a/kernel.spec b/kernel.spec
index adff54e13..386d96609 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -735,9 +735,8 @@ Patch12430: can-softing-depend-on-iomem.patch
 
 # rhbz#673857
 Patch12432: hfsplus-01-dont-leak-buffer.patch
-Patch12433: hfsplus-02-fill-super-skip-cleanup.patch
 Patch12434: hfsplus-03-zero-vhdr-on-free.patch
-Patch12435: hfsplus-04-check-for-vhdr.patch
+Patch12436: hfsplus-05-fix-failed-mount.patch
 
 %endif
 
@@ -1357,9 +1356,8 @@ ApplyPatch can-softing-depend-on-iomem.patch
 
 # rhbz#673857
 ApplyPatch hfsplus-01-dont-leak-buffer.patch
-ApplyPatch hfsplus-02-fill-super-skip-cleanup.patch
 ApplyPatch hfsplus-03-zero-vhdr-on-free.patch
-ApplyPatch hfsplus-04-check-for-vhdr.patch
+ApplyPatch hfsplus-05-fix-failed-mount.patch
 
 # END OF PATCH APPLICATIONS