From: Andreas Gruenbacher <[email protected]>
The path that __d_path() computes can become slightly inconsistent when it
races with mount operations: it grabs the vfsmount_lock when traversing mount
points but immediately drops it again, only to re-grab it when it reaches the
next mount point. The result is that the filename computed is not always
consisent, and the file may never have had that name. (This is unlikely, but
still possible.)
Fix this by grabbing the vfsmount_lock for the whole duration of
__d_path().
Signed-off-by: Andreas Gruenbacher <[email protected]>
Signed-off-by: John Johansen <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
Acked-by: Christoph Hellwig <[email protected]>
---
fs/dcache.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2008-06-13 13:13:01.000000000 +0200
+++ linux-2.6/fs/dcache.c 2008-06-13 13:13:04.000000000 +0200
@@ -1784,6 +1784,7 @@ char *__d_path(const struct path *path,
char *retval;
struct qstr *name;
+ spin_lock(&vfsmount_lock);
prepend(&end, &buflen, "\0", 1);
if (!IS_ROOT(dentry) && d_unhashed(dentry) &&
(prepend(&end, &buflen, " (deleted)", 10) != 0))
@@ -1802,14 +1803,11 @@ char *__d_path(const struct path *path,
break;
if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
/* Global root? */
- spin_lock(&vfsmount_lock);
if (vfsmnt->mnt_parent == vfsmnt) {
- spin_unlock(&vfsmount_lock);
goto global_root;
}
dentry = vfsmnt->mnt_mountpoint;
vfsmnt = vfsmnt->mnt_parent;
- spin_unlock(&vfsmount_lock);
continue;
}
parent = dentry->d_parent;
@@ -1822,6 +1820,8 @@ char *__d_path(const struct path *path,
dentry = parent;
}
+out:
+ spin_unlock(&vfsmount_lock);
return retval;
global_root:
@@ -1841,9 +1841,11 @@ global_root:
}
root->mnt = vfsmnt;
root->dentry = dentry;
- return retval;
+ goto out;
+
Elong:
- return ERR_PTR(-ENAMETOOLONG);
+ retval = ERR_PTR(-ENAMETOOLONG);
+ goto out;
}
/**
--
On Mon, Jun 16, 2008 at 01:28:07PM +0200, Miklos Szeredi wrote:
> From: Andreas Gruenbacher <[email protected]>
>
> The path that __d_path() computes can become slightly inconsistent when it
> races with mount operations: it grabs the vfsmount_lock when traversing mount
> points but immediately drops it again, only to re-grab it when it reaches the
> next mount point. The result is that the filename computed is not always
> consisent, and the file may never have had that name. (This is unlikely, but
> still possible.)
>
> Fix this by grabbing the vfsmount_lock for the whole duration of
> __d_path().
Umm... I don't see problems with doing that, but I hope you realize that
the notion of "ever having that name" is not the same as "pathnam resolution
on that name ever leading to that file" - path_walk() is *NOT* atomic
wrt rename() (or mount --move, indeed) and it's quite possible to walk
into subdirectory, have it moved under you, then see .. as the next pathname
component and step out into new parent.
Said that, it makes sense to avoid dropping/regaining the lock in that
case - it's less work and I don't believe that it would increase contention
on vfsmount_lock. So I'm applying that one, just be careful with assumptions
about consistency, etc. in the general area.
On Monday 23 June 2008 16:01:44 Al Viro wrote:
> Umm... I don't see problems with doing that, but I hope you realize that
> the notion of "ever having that name" is not the same as "pathnam
> resolution on that name ever leading to that file" - path_walk() is *NOT*
> atomic wrt rename() (or mount --move, indeed) and it's quite possible to
> walk into subdirectory, have it moved under you, then see .. as the next
> pathname component and step out into new parent.
Yes, that's understood. Relative lookups don't visit all
directories up to the root (unless via ".."), so there can be
infinite time between walking down a directory and computing
a pathname for it.
> Said that, it makes sense to avoid dropping/regaining the lock in that
> case - it's less work and I don't believe that it would increase contention
> on vfsmount_lock. So I'm applying that one, just be careful with
> assumptions about consistency, etc. in the general area.
Thanks.
Andreas