Re: [BUG] Race with iput and umount

[Jeff Mahoney <jeffm@novell.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

viro@parcelfarce.linux.theplanet.co.uk wrote:
| On Sat, Oct 02, 2004 at 03:55:18AM -0400, Jeff Mahoney wrote:
|
|>generic_shutdown_super() will happily call the ->put_super fs method,
|>destroying data structures still in use by the iput (->delete_inode) in
|>progress.  That's where Oopsen come into play.
|>
|>The unlink path will call the ->unlink fs method, release the path (thus
|>dropping the reference to the vfsmount, and then call iput. Since the
|>vfsmount reference is dropped back to 1, a umount will succeed, causing
|>the superblock to be cleaned up.
|
|
| Arrgh...
|
| Bug is in the ->i_count hacks in sys_unlink().  1001st proof that VFS has
| no fscking business playing with inode refcount directly...
|
| OK, quick and dirty fix follows.  Note: all places that go to exit1:
or exit:
| will have NULL inode, so we are not leaking anything here and it is OK
do that
| iput() early; indeed, the goal of that kludge was to postpone the final
| iput() past the unlocking the parent for the sake of contention if a wunch
| of bankers is doing parallel unlink() on files in the same directory and
| normally it would happen on dput() after vfs_unlink())
|
| --- linux/fs/namei.c	Mon Sep 13 01:32:00 2004
| +++ linux/fs/namei.c.fix	Sat Oct  2 05:48:21 2004
| @@ -1825,13 +1825,12 @@
|  		dput(dentry);
|  	}
|  	up(&nd.dentry->d_inode->i_sem);
| +	if (inode)
| +		iput(inode);	/* truncate the inode here */
|  exit1:
|  	path_release(&nd);
|  exit:
|  	putname(name);
| -
| -	if (inode)
| -		iput(inode);	/* truncate the inode here */
|  	return error;
|
|  slashes:


Works as expected. Thanks!

- -Jeff

- --
Jeff Mahoney
SuSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFBXs0lLPWxlyuTD7IRAivHAJ4l3GsDxWgQTrTHrMeR7C0CqpomiACgljzB
njBzAn2tP+P5pya1egf9TmU=
=JmTS
-----END PGP SIGNATURE-----