Re: Linux 2.6.38-rc5

[Al Viro]

On Wed, Feb 16, 2011 at 04:06:43PM +0000, Al Viro wrote:

> Sigh...  I see what's going on.  We'd got inode from dentry that is getting
> crapped under us.  We will *not* survive dropping RCU - it's bad enough for
> full restart in normal mode.  So right after we'd seen that (already wrong)
> inode has ->follow_link(), we decide to drop RCU.  Originally this BUG_ON
> hadn't been reached in that case - we had already failed with -ECHILD before
> we got to it.  Now we don't...
> 
> _However_, I don't like passing inode to do_follow_link().  I'd rather set
> nd->inode to inode first and use it there.  Let me think a bit and see if
> it's feasible...

No, that won't do.  The damn thing uses previous value of nd->inode if
it walks into relative symlink...

	Let's shift that call of nameidata_dentry_drop_rcu_maybe() into both
callers of do_follow_link() instead.  Marginally less obvious that we won't
reach the guts of do_follow_link() in RCU mode, just as obvious that overall
structure is ugly as hell and avoids making it even uglier by passing inode
down there.  How about this:

diff --git a/fs/namei.c b/fs/namei.c
index 9e701e2..d7003cf 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -800,10 +800,6 @@ static inline int do_follow_link(struct path *path, struct nameidata *nd)
 	void *cookie;
 	int err = -ELOOP;
 
-	/* We drop rcu-walk here */
-	if (nameidata_dentry_drop_rcu_maybe(nd, path->dentry))
-		return -ECHILD;
-
 	if (current->link_count >= MAX_NESTED_LINKS)
 		goto loop;
 	if (current->total_link_count >= 40)
@@ -1413,6 +1409,9 @@ exec_again:
 			goto out_dput;
 
 		if (inode->i_op->follow_link) {
+			/* We drop rcu-walk here */
+			if (nameidata_dentry_drop_rcu_maybe(nd, next.dentry))
+				return -ECHILD;
 			BUG_ON(inode != next.dentry->d_inode);
 			err = do_follow_link(&next, nd);
 			if (err)
@@ -1458,6 +1457,8 @@ last_component:
 			break;
 		if (inode && unlikely(inode->i_op->follow_link) &&
 		    (lookup_flags & LOOKUP_FOLLOW)) {
+			if (nameidata_dentry_drop_rcu_maybe(nd, next.dentry))
+				return -ECHILD;
 			BUG_ON(inode != next.dentry->d_inode);
 			err = do_follow_link(&next, nd);
 			if (err)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/