Re: [PATCH] reiserfs: Expand i_mutex to enclose lookup_one_len

[Jeff Mahoney <jeffm@suse.com>]

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

Al Viro wrote:
> On Fri, May 01, 2009 at 12:11:12PM -0400, Jeff Mahoney wrote:
>>  2.6.30-rc3 introduced some sanity checks in the VFS code to avoid NFS
>>  bugs by ensuring that lookup_one_len is always called under i_mutex.
>>
>>  This patch expands the i_mutex locking to enclose lookup_one_len. This was
>>  always required, but not not enforced in the reiserfs code since it
>>  does locking around the xattr interactions with the xattr_sem.
>>
>>  This is obvious enough, but it survived an overnight 50 thread ACL test.
> 
> It's not enough, unfortunately ;-/  It deals with the warning, but it
> leaves an actual hole in there.
> 
> Look: what happens if we mount it r/o without that directory and then
> remount r/w?  We get dentry for privroot, hash it (negative at that point),
> then do actual mkdir, unlock root and modify the ->d_compare() of root
> to reject lookups on that sucker.  Too late - in the meanwhile lookups
> might very well come and find privroot in dcache.
> 
> BTW, the way ->d_compare() is done in there is rather dumb -
> 	if (q1 == &priv_root->d_name)
> 		return -ENOENT;
> 	...
> would do just as well.  Why don't we do that lookup *once* (on ->get_sb(),
> before anything can come and race with us), and then just keep negative
> dentry if the directory hadn't been around?  And set d_compare() for root
> immediately after that lookup...
> 
> I've applied your patch as-is, and unless you have objections to the
> variant above I'll do that as incremental.  Comments?

Of course you're right about the lookup hole.

The lookup during remount is from when the code didn't enable xattrs
unconditionally for security and trusted attributes. The privroot would
only be created when mounted read-write with -oacl and/or -ouser_xattr.
Now xattrs are enabled uncondtionally, so any read-write mount will
create that if xattrs are enabled in the kernel. It used to avoid
caching a negative lookup that would never get used.

I don't have any objections to any of your suggestions.

Thanks!

- -Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iEYEARECAAYFAkn+dp0ACgkQLPWxlyuTD7KuwQCgmrYQhuDy+HylXPL46Yb2R9Y+
Fr0AoIKRyL4mX1wCR+R9WN74zULTrbXT
=LSDi
-----END PGP SIGNATURE-----