Re: inode_permission NULL pointer dereference in 3.13-rc1

Al Viro
2013-11-29T23:55:52Z

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.