OK, here comes a patch series that hopefully should close all
too-early-mntput() races in fs/namei.c. Entire area is convoluted as
hell, so I'm splitting that series into _very_ small chunks.
Patches alread in the tree close only (very wide) races in following
symlinks (see "busy inodes after umount" thread some time ago). Unfortunately,
quite a few narrower races of the same nature were not closed. Hopefully
this should take care of all of them.
Please, review and test. It survives local beating and AFAICS
it's correct, but that's a hell of a critical area. Extra eyes would
be very welcome.
Patches will be sent in separate mails; they are also available on
ftp.linux.org.uk/pub/people/viro/NAMEI*. Series is based at 2.6.12-rc4
and yes, there are 19 patches in it.
I reviewed the patchset (not the individual patches, rather the end
result), and it seems OK to me.
It would be nice, if there was a comment about sharing the refcount
for path->mnt and nd->mnt, and how __follow_mount and __do_follow_link
deal with this.
Also, I think it would improve readability if the dealings with struct
path were extracted into separate functions. Something like the
attached patch.
Miklos
Index: linux/fs/namei.c
===================================================================
--- linux.orig/fs/namei.c 2005-05-23 13:36:05.000000000 +0200
+++ linux/fs/namei.c 2005-05-23 13:47:21.000000000 +0200
@@ -522,6 +522,22 @@ static inline int __do_follow_link(struc
return error;
}
+static inline void dput_path(struct path *path, struct nameidata *nd)
+{
+ dput(path->dentry);
+ if (path->mnt != nd->mnt)
+ mntput(path->mnt);
+}
+
+static inline void path_to_nameidata(struct path *path, struct nameidata *nd)
+{
+ dput(nd->dentry);
+ if (nd->mnt != path->mnt)
+ mntput(nd->mnt);
+ nd->mnt = path->mnt;
+ nd->dentry = path->dentry;
+}
+
/*
* This limits recursive symlink follows to 8, while
* limiting consecutive symlinks to 40.
@@ -549,9 +565,7 @@ static inline int do_follow_link(struct
nd->depth--;
return err;
loop:
- dput(path->dentry);
- if (path->mnt != nd->mnt)
- mntput(path->mnt);
+ dput_path(path, nd);
path_release(nd);
return err;
}
@@ -810,13 +824,8 @@ static fastcall int __link_path_walk(con
err = -ENOTDIR;
if (!inode->i_op)
break;
- } else {
- dput(nd->dentry);
- if (nd->mnt != next.mnt)
- mntput(nd->mnt);
- nd->mnt = next.mnt;
- nd->dentry = next.dentry;
- }
+ } else
+ path_to_nameidata(&next, nd);
err = -ENOTDIR;
if (!inode->i_op->lookup)
break;
@@ -856,13 +865,8 @@ last_component:
if (err)
goto return_err;
inode = nd->dentry->d_inode;
- } else {
- dput(nd->dentry);
- if (nd->mnt != next.mnt)
- mntput(nd->mnt);
- nd->mnt = next.mnt;
- nd->dentry = next.dentry;
- }
+ } else
+ path_to_nameidata(&next, nd);
err = -ENOENT;
if (!inode)
break;
@@ -898,9 +902,7 @@ return_reval:
return_base:
return 0;
out_dput:
- dput(next.dentry);
- if (nd->mnt != next.mnt)
- mntput(next.mnt);
+ dput_path(&next, nd);
break;
}
path_release(nd);
@@ -1504,11 +1506,7 @@ do_last:
if (path.dentry->d_inode->i_op && path.dentry->d_inode->i_op->follow_link)
goto do_link;
- dput(nd->dentry);
- nd->dentry = path.dentry;
- if (nd->mnt != path.mnt)
- mntput(nd->mnt);
- nd->mnt = path.mnt;
+ path_to_nameidata(&path, nd);
error = -EISDIR;
if (path.dentry->d_inode && S_ISDIR(path.dentry->d_inode->i_mode))
goto exit;
@@ -1519,9 +1517,7 @@ ok:
return 0;
exit_dput:
- dput(path.dentry);
- if (nd->mnt != path.mnt)
- mntput(path.mnt);
+ dput_path(&path, nd);
exit:
path_release(nd);
return error;