"For every complex and subtle problem,
There is a simple and obvious solution.
Which is wrong."
Sometimes, there is also one which is right.
I'll let you be the judge as to where the following should fit.
It seems to me that there are two sorts of symlinks.
1/ those for which "follow_link" is implemented by passing the result
of "readlink" to "vfs_follow_link" and
2/ those for which it isn't.
We could describe these two sorts in other ways:
1/ Those which can safely be exported via NFS (which asserts that the
client interprets symlinks) and
2/ Those that cannot.
or
1/ Those for which it makes some sort of sense for create to succeed
through a dangling symlink and
2/ Those for which it doesn't.
It seems to me that this distinction should be more obvious - raised
out of the individual filesystems and presented to the vfs (and hence
nfsd) layer.
This could easily be done by allowing that if follow_link is NULL, and
readlink is != NULL, then it is a type-1 symlink, but that if
follow_link != NULL, it is a type-2 symlink.
The follow patch against ac20 does this.
It allocates extra pages, and does a bit more copying (using set_fs
(yuk)) but seems to be essentially right.
The extra copying could be avoided by redefining "readlink" is some
why to, for example, take an actor which acts upon the link text.
This does not address mknod etc over a dangling link - but I agree
that it is quite reasonable for that to not work.
This does not address the fact that symlinks are in many ways an ugly
concept that we might be better off without, but I think we are stuck
with them.
It also does not modify any filesystem but ext2 (plus anything that
exclusively used page_symlink_inode_operations), but others are
trivial to do.
NeilBrown
--- ./fs/ext2/symlink.c 2000/06/18 21:59:09 1.1
+++ ./fs/ext2/symlink.c 2000/06/18 21:59:19
@@ -25,13 +25,6 @@
return vfs_readlink(dentry, buffer, buflen, s);
}
-static int ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
-{
- char *s = (char *)dentry->d_inode->u.ext2_i.i_data;
- return vfs_follow_link(nd, s);
-}
-
struct inode_operations ext2_fast_symlink_inode_operations = {
readlink: ext2_readlink,
- follow_link: ext2_follow_link,
};
--- ./fs/namei.c 2000/06/18 22:00:53 1.1
+++ ./fs/namei.c 2000/06/19 02:43:58
@@ -293,14 +293,32 @@
return result;
}
+static inline int
+__vfs_follow_link(struct nameidata *nd, const char *link);
+
static inline int do_follow_link(struct dentry *dentry, struct nameidata *nd)
{
int err;
+ char *tmp;
if (current->link_count >= 32)
goto loop;
current->link_count++;
UPDATE_ATIME(dentry->d_inode);
- err = dentry->d_inode->i_op->follow_link(dentry, nd);
+ if (dentry->d_inode->i_op->follow_link)
+ err = dentry->d_inode->i_op->follow_link(dentry, nd);
+ else if (!(tmp=__getname()))
+ err = -ENOMEM;
+ else {
+ mm_segment_t fs = get_fs();
+ set_fs(KERNEL_DS);
+ err = dentry->d_inode->i_op->readlink(dentry, tmp, PAGE_SIZE);
+ set_fs(fs);
+ if (err>=0) {
+ tmp[err]=0;
+ err = __vfs_follow_link(nd, tmp);
+ }
+ putname(tmp);
+ }
current->link_count--;
return err;
loop:
@@ -479,7 +497,7 @@
if (!inode->i_op)
goto out_dput;
- if (inode->i_op->follow_link) {
+ if (inode->i_op->follow_link || inode->i_op->readlink) {
err = do_follow_link(dentry, nd);
dput(dentry);
if (err)
@@ -534,7 +552,8 @@
;
inode = dentry->d_inode;
if ((lookup_flags & LOOKUP_FOLLOW)
- && inode && inode->i_op && inode->i_op->follow_link) {
+ && inode && inode->i_op
+ && (inode->i_op->follow_link || inode->i_op->readlink)) {
err = do_follow_link(dentry, nd);
dput(dentry);
if (err)
@@ -885,6 +904,7 @@
int acc_mode, error = 0;
struct inode *inode;
struct dentry *dentry;
+ int tail_link_count = 0;
acc_mode = ACC_MODE(flag);
if (!(flag & O_CREAT)) {
@@ -899,6 +919,7 @@
if (path_init(pathname, LOOKUP_PARENT, nd))
error = path_walk(pathname, nd);
+ again:
if (error)
return error;
/*
@@ -933,7 +954,8 @@
if (flag & O_NOFOLLOW) {
error = -ELOOP;
if (dentry->d_inode->i_op &&
- dentry->d_inode->i_op->follow_link)
+ (dentry->d_inode->i_op->follow_link ||
+ dentry->d_inode->i_op->readlink))
goto exit_dput;
if (d_mountpoint(dentry))
goto exit_dput;
@@ -958,6 +980,26 @@
if (error)
return error;
dentry = nd->dentry;
+ } else if (dentry->d_inode->i_op &&
+ dentry->d_inode->i_op->readlink) {
+ mm_segment_t fs = get_fs();
+
+ error = -ELOOP;
+ if (tail_link_count>= 32)
+ goto exit_dput;
+ tail_link_count++;
+ UPDATE_ATIME(dentry->d_inode);
+ set_fs(KERNEL_DS);
+ error = dentry->d_inode->i_op->readlink(
+ dentry, pathname, PAGE_SIZE);
+ set_fs(fs);
+ if (error>=0) {
+ pathname[error]=0;
+ error = __vfs_follow_link(nd, pathname);
+ }
+ dput(dentry);
+
+ goto again;
} else {
got_it:
dput(nd->dentry);
@@ -1842,19 +1884,6 @@
return res;
}
-int page_follow_link(struct dentry *dentry, struct nameidata *nd)
-{
- struct page *page = NULL;
- char *s = page_getlink(dentry, &page);
- int res = __vfs_follow_link(nd, s);
- if (page) {
- kunmap(page);
- page_cache_release(page);
- }
- return res;
-}
-
struct inode_operations page_symlink_inode_operations = {
readlink: page_readlink,
- follow_link: page_follow_link,
};
--- ./kernel/ksyms.c 2000/06/18 23:56:15 1.1
+++ ./kernel/ksyms.c 2000/06/18 23:56:18
@@ -243,7 +243,6 @@
EXPORT_SYMBOL(vfs_readlink);
EXPORT_SYMBOL(vfs_follow_link);
EXPORT_SYMBOL(page_readlink);
-EXPORT_SYMBOL(page_follow_link);
EXPORT_SYMBOL(page_symlink_inode_operations);
EXPORT_SYMBOL(block_symlink);
EXPORT_SYMBOL(vfs_readdir);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Jun 23 2000 - 21:00:16 EST