Re: [PATCH] VFS: Add back check for !inode in walk_component()

From: Al Viro
Date: Thu May 07 2015 - 14:43:54 EST


On Thu, May 07, 2015 at 07:13:35PM +0100, Al Viro wrote:
> On Thu, May 07, 2015 at 01:39:35PM -0400, Steven Rostedt wrote:
> > I had them printed in my previous traces. The flags were 0x200088, and
> > they were 0 just before the call.
>
> Not dentry->d_flags, nd->flags. Most interesting part is bit 6 in those
> (LOOKUP_RCU, 0x40).
>
> As for creation... I think I see what might be going on:
>
> A: finds a negative dentry, picks NULL ->d_inode from it and whatever
> ->d_seq it had.
> B: d_instantiate(): sets ->d_inode non-NULL, ->d_flags accordingly and
> bumps ->d_seq.
> A: fetches ->d_flags, sees non-negative, assumes ->d_inode is non-NULL.
>
> In reality, the last assumption should've been "->d_inode is non-NULL or
> we have a stale ->d_seq and will end up discarding that fscker anyway".
>
> Hmm... Smells like we ought to
[snip]

Actually, could you try the following on top of -rc2?

diff --git a/fs/namei.c b/fs/namei.c
index 4a8d998b..421e597 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1415,6 +1415,7 @@ static int lookup_fast(struct nameidata *nd,
*/
if (nd->flags & LOOKUP_RCU) {
unsigned seq;
+ bool negative;
dentry = __d_lookup_rcu(parent, &nd->last, &seq);
if (!dentry)
goto unlazy;
@@ -1424,8 +1425,11 @@ static int lookup_fast(struct nameidata *nd,
* the dentry name information from lookup.
*/
*inode = dentry->d_inode;
+ negative = d_is_negative(dentry);
if (read_seqcount_retry(&dentry->d_seq, seq))
return -ECHILD;
+ if (negative)
+ return -ENOENT;

/*
* This sequence count validates that the parent had no
@@ -1472,9 +1476,13 @@ unlazy:
goto need_lookup;
}

- path->mnt = mnt;
- path->dentry = dentry;
- err = follow_managed(path, nd->flags);
+ if (unlikely(d_is_negative(dentry))) {
+ err = -ENOENT;
+ } else {
+ path->mnt = mnt;
+ path->dentry = dentry;
+ err = follow_managed(path, nd->flags);
+ }
if (unlikely(err < 0)) {
path_put_conditional(path, nd);
return err;
@@ -1583,10 +1591,10 @@ static inline int walk_component(struct nameidata *nd, struct path *path,
goto out_err;

inode = path->dentry->d_inode;
+ err = -ENOENT;
+ if (d_is_negative(path->dentry))
+ goto out_path_put;
}
- err = -ENOENT;
- if (d_is_negative(path->dentry))
- goto out_path_put;

if (should_follow_link(path->dentry, follow)) {
if (nd->flags & LOOKUP_RCU) {
@@ -3036,14 +3044,13 @@ retry_lookup:

BUG_ON(nd->flags & LOOKUP_RCU);
inode = path->dentry->d_inode;
-finish_lookup:
- /* we _can_ be in RCU mode here */
error = -ENOENT;
if (d_is_negative(path->dentry)) {
path_to_nameidata(path, nd);
goto out;
}
-
+finish_lookup:
+ /* we _can_ be in RCU mode here */
if (should_follow_link(path->dentry, !symlink_ok)) {
if (nd->flags & LOOKUP_RCU) {
if (unlikely(nd->path.mnt != path->mnt ||
--
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/