Re: [uml-devel] Possible hostfs uml kernel data corruption with missing host directory (fc6, 2.6.20-rc5-git3 on opteron)

[Blaisorblade <blaisorblade@yahoo.it>]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

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

[-- Attachment #2: hostfs-fix-double-free.diff --]
[-- Type: text/x-diff, Size: 1194 bytes --]

Subject: uml - hostfs: fix double free

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

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 <wstearns@pobox.com>
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
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);

[-- Attachment #3: Type: text/plain, Size: 345 bytes --]

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

[-- Attachment #4: Type: text/plain, Size: 194 bytes --]

_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel