On Friday 26 January 2007 16:13, Jeff Dike wrote:
> On Fri, Jan 26, 2007 at 04:07:05AM -0500, William Stearns wrote:
> > After bisecting the difference between the two root filesystems,
> > the critical change was a line in /etc/fstab:
> >
> > none /mirrors hostfs defaults,/home/mirrors 0 0
> >
> > Woah.
Ok, I think I have found it from code analisys (no testing done). Please test
the attached patch, since the bug is repeatable for you.
Ok, we have a double kfree inside hostfs_fill_sb_common; since name is
assigned to host_filename, it is owned by the inode, and freed both by dput
and by kfree(name).
The path from dput to the first free is:
dput->iput->hostfs_destroy_inode->kfree(... ->host_filename)
The simplest fix is to clear 'name' after assigning it to ->host_filename.
Patch attached - the second hunk is totally optional, it avoids a
kfree(NULL);. The goal was maximum simplicity here.
Also, I discovered that in read_inode, failure from file_type (which calls
lstat64) is ignored. read_name is subsequently called, it calls lstat again,
so this should not be a problem; however, the other call to file_type (in
init_inode) is also unchecked, and that is less robust.
--
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Subject: uml - hostfs: fix double free
From: Paolo 'Blaisorblade' Giarrusso <[EMAIL PROTECTED]>
Fix double free in the error path - when name is assigned into root_inode we do
not own it any more and we must not kfree() it - see patch for details.
Thanks to William Stearns for the initial report.
CC: William Stearns <[EMAIL PROTECTED]>
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[EMAIL PROTECTED]>
Index: linux-2.6.git/fs/hostfs/hostfs_kern.c
===================================================================
--- linux-2.6.git.orig/fs/hostfs/hostfs_kern.c
+++ linux-2.6.git/fs/hostfs/hostfs_kern.c
@@ -966,6 +966,9 @@ static int hostfs_fill_sb_common(struct
goto out_put;
HOSTFS_I(root_inode)->host_filename = name;
+ /* Avoid that in the error path, iput(root_inode) frees again name through
+ * hostfs_destroy_inode! */
+ name = NULL;
err = -ENOMEM;
sb->s_root = d_alloc_root(root_inode);
@@ -977,7 +980,7 @@ static int hostfs_fill_sb_common(struct
/* No iput in this case because the dput does that for us */
dput(sb->s_root);
sb->s_root = NULL;
- goto out_free;
+ goto out;
}
return(0);
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
