Proper HFS+ mount failure fix from hch

This commit is contained in:
Chuck Ebbert 2011-02-01 19:27:02 -05:00
parent 580ae71e04
commit 64ca523865
5 changed files with 212 additions and 86 deletions

View File

@ -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)

View File

@ -1,8 +1,4 @@
hfsplus: Clear volume header pointers on failure hfsplus: Fix two memory leaks in wrapper.c
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.
Signed-Off-By: Chuck Ebbert <cebbert@redhat.com> Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
@ -26,14 +22,3 @@ Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
goto reread; 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;
}

View File

@ -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;

View File

@ -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;
}

View File

@ -735,9 +735,8 @@ Patch12430: can-softing-depend-on-iomem.patch
# rhbz#673857 # rhbz#673857
Patch12432: hfsplus-01-dont-leak-buffer.patch Patch12432: hfsplus-01-dont-leak-buffer.patch
Patch12433: hfsplus-02-fill-super-skip-cleanup.patch
Patch12434: hfsplus-03-zero-vhdr-on-free.patch Patch12434: hfsplus-03-zero-vhdr-on-free.patch
Patch12435: hfsplus-04-check-for-vhdr.patch Patch12436: hfsplus-05-fix-failed-mount.patch
%endif %endif
@ -1357,9 +1356,8 @@ ApplyPatch can-softing-depend-on-iomem.patch
# rhbz#673857 # rhbz#673857
ApplyPatch hfsplus-01-dont-leak-buffer.patch 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-03-zero-vhdr-on-free.patch
ApplyPatch hfsplus-04-check-for-vhdr.patch ApplyPatch hfsplus-05-fix-failed-mount.patch
# END OF PATCH APPLICATIONS # END OF PATCH APPLICATIONS