Re: inode_permission NULL pointer dereference in 3.13-rc1

[Al Viro <viro@ZenIV.linux.org.uk>]

On Fri, Nov 29, 2013 at 12:17:23PM -0800, Linus Torvalds wrote:

> Al - even in your scenario I don't see a NULL nd->inode, because when
> we do an rmdir we remove the dentry, we don't turn it into a negative
> one. Afaik, it would be a violation of all our dentry rules to change
> the dentry->d_inode field while the dentry is live. The only way to
> get a negative dentry (ie d_inode == NULL) should be from lookup (and
> from a rename that switches the dentries around, but even then the
> d_inode _stays_ NULL, it's just that we move the dentry itself
> around).

Look at the end of vfs_rmdir(); d_delete() in there will turn dentry
negative if nobody else hold references to it.  So yes, dentry of
directory *can* go negative under you, unless you've grabbed a reference.
Which we do not do in RCU mode, obviously.

What would be a violation of all rules is dentry held by somebody else
becoming negative.  And d_delete() avoids that, but the whole point of
RCU-mode pathwalk is to _not_ hold intermediates.  So nd->inode is
needed.  Sure, it'll get ->d_seq bumped, but that won't do you much good
when it comes to attempt to dereference nd->inode.  Sure, we can turn
all places that access nd->inode into
	struct dentry *dentry = nd->path.dentry;
	struct inode *inode = dentry->d_inode;
	if (read_seqcount_retry(&dentry->d_seq, nd->seq))
		/* too fucking bad, we'd lost the race */
	else
		/* use inode */
but it will be just as messy as maintaining nd->inode _and_ quite a bit
slower.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs