On Sun, Sep 29, 2013 at 07:10:47PM +0100, Al Viro wrote: > FWIW, right now I'm reviewing the subset of fs code that can be hit in > RCU mode. Not a pretty sight, that... ;-/ First catch: in > fuse_dentry_revalidate() we have a case (reachable with LOOKUP_RCU) where > we do this: > } else if (inode) { > fc = get_fuse_conn(inode); > if (fc->readdirplus_auto) { > parent = dget_parent(entry); > fuse_advise_use_readdirplus(parent->d_inode); > dput(parent); > } > } > First of all, that'll lead to obvious nastiness if we get here when > ->s_fs_info has already been freed in process of fs shutdown; fc will > be pointing to kfreed object and no, freeing it isn't RCU-delayed. > That's not a problem with the current tree, of course, but this > dput(parent) very much is - doing that under rcu_read_lock() is > a Bloody Bad Idea(tm). Another one: int ll_revalidate_nd(struct dentry *dentry, unsigned int flags) { struct inode *parent = dentry->d_parent->d_inode; int unplug = 0; CDEBUG(D_VFSTRACE, "VFS Op:name=%s,flags=%u\n", dentry->d_name.name, flags); if (!(flags & (LOOKUP_PARENT|LOOKUP_OPEN|LOOKUP_CREATE)) && ll_need_statahead(parent, dentry) > 0) { if (flags & LOOKUP_RCU) return -ECHILD; ... and ll_need_statahead(NULL, ...) will oops. Doesn't even need LOOKUP_RCU to barf - ->d_revalidate() can be called without ->i_mutex on parent, so we can race with e.g. rename() followed by rmdir() of old parent. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html