hfsplus can generate NULL pointer exceptions when a user attempts
to mount an invalid filesystem. In some paths it also leaks memory.
This series attempts to fix some of the most obvious problems.
hfsplus: Skip cleanup on early mount failures
If we don't assign s_fs_info until later we can skip cleanup code
when handling very early failures.
Signed-Off-By: Chuck Ebbert <[email protected]>
--- 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)
hfsplus: Don't leak buffer on error
Signed-Off-By: Chuck Ebbert <[email protected]>
--- vanilla-2.6.38-rc2-git9.orig/fs/hfsplus/part_tbl.c
+++ vanilla-2.6.38-rc2-git9/fs/hfsplus/part_tbl.c
@@ -134,7 +134,7 @@ int hfs_part_find(struct super_block *sb
res = hfsplus_submit_bio(sb->s_bdev, *part_start + HFS_PMAP_BLK,
data, READ);
if (res)
- return res;
+ goto out;
switch (be16_to_cpu(*((__be16 *)data))) {
case HFS_OLD_PMAP_MAGIC:
@@ -147,7 +147,7 @@ int hfs_part_find(struct super_block *sb
res = -ENOENT;
break;
}
-
+out:
kfree(data);
return res;
}
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.
Signed-Off-By: Chuck Ebbert <[email protected]>
--- vanilla-2.6.38-rc2-git9.orig/fs/hfsplus/wrapper.c
+++ vanilla-2.6.38-rc2-git9/fs/hfsplus/wrapper.c
@@ -167,7 +167,7 @@ reread:
break;
case cpu_to_be16(HFSP_WRAP_MAGIC):
if (!hfsplus_read_mdb(sbi->s_vhdr, &wd))
- goto out;
+ goto out_free_backup_vhdr;
wd.ablk_size >>= HFSPLUS_SECTOR_SHIFT;
part_start += wd.ablk_start + wd.embed_start * wd.ablk_size;
part_size = wd.embed_count * wd.ablk_size;
@@ -179,7 +179,7 @@ reread:
* (should do this only for cdrom/loop though)
*/
if (hfs_part_find(sb, &part_start, &part_size))
- goto out;
+ goto out_free_backup_vhdr;
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;
}
hfsplus: Check for NULL volume header in put_super()
If volume header is null there is not much to do in put_super().
Signed-Off-By: Chuck Ebbert <[email protected]>
--- 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;
On Tue, Feb 01, 2011 at 04:28:02PM -0500, Chuck Ebbert wrote:
> hfsplus can generate NULL pointer exceptions when a user attempts
> to mount an invalid filesystem. In some paths it also leaks memory.
>
> This series attempts to fix some of the most obvious problems.
Not in a very nice way, though. The real problem here is that we
a) can still return a failure after sb->s_root is set
b) abuse hfsplus_put_super for error handling, which is totally
ill-suited.
Take a look at the patch in
https://bugzilla.kernel.org/show_bug.cgi?id=27932 for a proper fix.
This one looks good though, I'll put it in.
On Tue, Feb 01, 2011 at 04:41:55PM -0500, Chuck Ebbert wrote:
> hfsplus: Don't leak buffer on error
That's already said in the subject, no need to repeat it in the mail
body.
On Tue, Feb 01, 2011 at 04:45:07PM -0500, Chuck Ebbert wrote:
> 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.
Can you resend it without the NULLing out, which is not required
with proper error handling?