* [Patch 0/4] Fix error handling in hfsplus mount failure paths
@ 2011-02-01 21:28 Chuck Ebbert
2011-02-01 21:41 ` [Patch 2/4] hfsplus: Skip cleanup on early mount failures Chuck Ebbert
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Chuck Ebbert @ 2011-02-01 21:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch 2/4] hfsplus: Skip cleanup on early mount failures
2011-02-01 21:28 [Patch 0/4] Fix error handling in hfsplus mount failure paths Chuck Ebbert
@ 2011-02-01 21:41 ` Chuck Ebbert
2011-02-01 21:41 ` [Patch 1/4] hfsplus: Don't leak buffer on error Chuck Ebbert
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Chuck Ebbert @ 2011-02-01 21:41 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel
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 <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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch 1/4] hfsplus: Don't leak buffer on error
2011-02-01 21:28 [Patch 0/4] Fix error handling in hfsplus mount failure paths Chuck Ebbert
2011-02-01 21:41 ` [Patch 2/4] hfsplus: Skip cleanup on early mount failures Chuck Ebbert
@ 2011-02-01 21:41 ` Chuck Ebbert
2011-02-01 22:08 ` Christoph Hellwig
2011-02-01 21:45 ` [Patch 3/4] hfsplus: Clear volume header pointers on failure Chuck Ebbert
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Chuck Ebbert @ 2011-02-01 21:41 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel
hfsplus: Don't leak buffer on error
Signed-Off-By: Chuck Ebbert <cebbert@redhat.com>
--- 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;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch 3/4] hfsplus: Clear volume header pointers on failure
2011-02-01 21:28 [Patch 0/4] Fix error handling in hfsplus mount failure paths Chuck Ebbert
2011-02-01 21:41 ` [Patch 2/4] hfsplus: Skip cleanup on early mount failures Chuck Ebbert
2011-02-01 21:41 ` [Patch 1/4] hfsplus: Don't leak buffer on error Chuck Ebbert
@ 2011-02-01 21:45 ` Chuck Ebbert
2011-02-01 22:13 ` Christoph Hellwig
2011-02-01 21:56 ` [Patch 4/4] hfsplus: Check for NULL volume header in put_super() Chuck Ebbert
2011-02-01 22:07 ` [Patch 0/4] Fix error handling in hfsplus mount failure paths Christoph Hellwig
4 siblings, 1 reply; 8+ messages in thread
From: Chuck Ebbert @ 2011-02-01 21:45 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel
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 <cebbert@redhat.com>
--- 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;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch 4/4] hfsplus: Check for NULL volume header in put_super()
2011-02-01 21:28 [Patch 0/4] Fix error handling in hfsplus mount failure paths Chuck Ebbert
` (2 preceding siblings ...)
2011-02-01 21:45 ` [Patch 3/4] hfsplus: Clear volume header pointers on failure Chuck Ebbert
@ 2011-02-01 21:56 ` Chuck Ebbert
2011-02-01 22:07 ` [Patch 0/4] Fix error handling in hfsplus mount failure paths Christoph Hellwig
4 siblings, 0 replies; 8+ messages in thread
From: Chuck Ebbert @ 2011-02-01 21:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel
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 <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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 0/4] Fix error handling in hfsplus mount failure paths
2011-02-01 21:28 [Patch 0/4] Fix error handling in hfsplus mount failure paths Chuck Ebbert
` (3 preceding siblings ...)
2011-02-01 21:56 ` [Patch 4/4] hfsplus: Check for NULL volume header in put_super() Chuck Ebbert
@ 2011-02-01 22:07 ` Christoph Hellwig
4 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2011-02-01 22:07 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Christoph Hellwig, linux-kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 1/4] hfsplus: Don't leak buffer on error
2011-02-01 21:41 ` [Patch 1/4] hfsplus: Don't leak buffer on error Chuck Ebbert
@ 2011-02-01 22:08 ` Christoph Hellwig
0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2011-02-01 22:08 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Christoph Hellwig, linux-kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 3/4] hfsplus: Clear volume header pointers on failure
2011-02-01 21:45 ` [Patch 3/4] hfsplus: Clear volume header pointers on failure Chuck Ebbert
@ 2011-02-01 22:13 ` Christoph Hellwig
0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2011-02-01 22:13 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Christoph Hellwig, linux-kernel
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-02-01 22:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-01 21:28 [Patch 0/4] Fix error handling in hfsplus mount failure paths Chuck Ebbert
2011-02-01 21:41 ` [Patch 2/4] hfsplus: Skip cleanup on early mount failures Chuck Ebbert
2011-02-01 21:41 ` [Patch 1/4] hfsplus: Don't leak buffer on error Chuck Ebbert
2011-02-01 22:08 ` Christoph Hellwig
2011-02-01 21:45 ` [Patch 3/4] hfsplus: Clear volume header pointers on failure Chuck Ebbert
2011-02-01 22:13 ` Christoph Hellwig
2011-02-01 21:56 ` [Patch 4/4] hfsplus: Check for NULL volume header in put_super() Chuck Ebbert
2011-02-01 22:07 ` [Patch 0/4] Fix error handling in hfsplus mount failure paths Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.