Fix nasty ncpfs symlink handling bug.

This bug could cause oopses and page state corruption, because ncpfs
used the generic page-cache symlink handlign functions.  But those
functions only work if the page cache is guaranteed to be "stable", ie a
page that was installed when the symlink walk was started has to still
be installed in the page cache at the end of the walk.

We could have fixed ncpfs to not use the generic helper routines, but it
is in many ways much cleaner to instead improve on the symlink walking
helper routines so that they don't require that absolute stability.

We do this by allowing "follow_link()" to return a error-pointer as a
cookie, which is fed back to the cleanup "put_link()" routine.  This
also simplifies NFS symlink handling.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/autofs/symlink.c b/fs/autofs/symlink.c
index f028396f138395a3b5f2958bbf858fc1c6d27e21..52e8772b066e33fece33d73ae5ddf30568c5b4ca 100644 (file)
--- a/fs/autofs/symlink.c
+++ b/fs/autofs/symlink.c
@@ -12,11 +12,12 @@
 
 #include "autofs_i.h"
 
-static int autofs_follow_link(struct dentry *dentry, struct nameidata *nd)
+/* Nothing to release.. */
+static void *autofs_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        char *s=((struct autofs_symlink *)dentry->d_inode->u.generic_ip)->data;
        nd_set_link(nd, s);
-       return 0;
+       return NULL;
 }
 
 struct inode_operations autofs_symlink_inode_operations = {

diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 78af5850c558127bde1a5c929e72f90cf7470403..1fd21f66f2435198e33eca7a2919ef42b08ef6ce 100644 (file)
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -83,8 +83,8 @@ extern int cifs_dir_notify(struct file *, unsigned long arg);
 extern struct dentry_operations cifs_dentry_ops;
 
 /* Functions related to symlinks */
-extern int cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
-extern void cifs_put_link(struct dentry *direntry, struct nameidata *nd);
+extern void *cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
+extern void cifs_put_link(struct dentry *direntry, struct nameidata *nd, void *);
 extern int cifs_readlink(struct dentry *direntry, char __user *buffer, 
                         int buflen);
 extern int cifs_symlink(struct inode *inode, struct dentry *direntry,

diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index bde0fabfece0aeeaecd9ab4b1c2cf733009975b1..ab925ef4f863c9696b9ed0a1832e9d3d6a0c0d07 100644 (file)
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -92,7 +92,7 @@ cifs_hl_exit:
        return rc;
 }
 
-int
+void *
 cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
 {
        struct inode *inode = direntry->d_inode;
@@ -148,7 +148,7 @@ out:
 out_no_free:
        FreeXid(xid);
        nd_set_link(nd, target_path);
-       return 0;
+       return NULL;    /* No cookie */
 }
 
 int
@@ -330,7 +330,7 @@ cifs_readlink(struct dentry *direntry, char __user *pBuffer, int buflen)
        return rc;
 }
 
-void cifs_put_link(struct dentry *direntry, struct nameidata *nd)
+void cifs_put_link(struct dentry *direntry, struct nameidata *nd, void *cookie)
 {
        char *p = nd_get_link(nd);
        if (!IS_ERR(p))

diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c
index 9f7bac01d557631e5bcebde72220305dbb3dafcb..1e67d87cfa913b58c648a586235816350481fed5 100644 (file)
--- a/fs/ext2/symlink.c
+++ b/fs/ext2/symlink.c
@@ -21,11 +21,11 @@
 #include "xattr.h"
 #include <linux/namei.h>
 
-static int ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        struct ext2_inode_info *ei = EXT2_I(dentry->d_inode);
        nd_set_link(nd, (char *)ei->i_data);
-       return 0;
+       return NULL;
 }
 
 struct inode_operations ext2_symlink_inode_operations = {

diff --git a/fs/ext3/symlink.c b/fs/ext3/symlink.c
index 8c3e72818fb0893e0ddee74c33f46b7e09da57c4..4f79122cde670558c39a6886623c7522286b9854 100644 (file)
--- a/fs/ext3/symlink.c
+++ b/fs/ext3/symlink.c
@@ -23,11 +23,11 @@
 #include <linux/namei.h>
 #include "xattr.h"
 
-static int ext3_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void * ext3_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        struct ext3_inode_info *ei = EXT3_I(dentry->d_inode);
        nd_set_link(nd, (char*)ei->i_data);
-       return 0;
+       return NULL;
 }
 
 struct inode_operations ext3_symlink_inode_operations = {

diff --git a/fs/namei.c b/fs/namei.c
index b85f158aef0c8509867693a8c705e19f50fd6b1a..6ec1f0fefc5b017aa321738f7d3581370a91fad3 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -501,6 +501,7 @@ struct path {
 static inline int __do_follow_link(struct path *path, struct nameidata *nd)
 {
        int error;
+       void *cookie;
        struct dentry *dentry = path->dentry;
 
        touch_atime(path->mnt, dentry);
@@ -508,13 +509,15 @@ static inline int __do_follow_link(struct path *path, struct nameidata *nd)
 
        if (path->mnt == nd->mnt)
                mntget(path->mnt);
-       error = dentry->d_inode->i_op->follow_link(dentry, nd);
-       if (!error) {
+       cookie = dentry->d_inode->i_op->follow_link(dentry, nd);
+       error = PTR_ERR(cookie);
+       if (!IS_ERR(cookie)) {
                char *s = nd_get_link(nd);
+               error = 0;
                if (s)
                        error = __vfs_follow_link(nd, s);
                if (dentry->d_inode->i_op->put_link)
-                       dentry->d_inode->i_op->put_link(dentry, nd);
+                       dentry->d_inode->i_op->put_link(dentry, nd, cookie);
        }
        dput(dentry);
        mntput(path->mnt);
@@ -2344,15 +2347,17 @@ out:
 int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
 {
        struct nameidata nd;
-       int res;
+       void *cookie;
+
        nd.depth = 0;
-       res = dentry->d_inode->i_op->follow_link(dentry, &nd);
-       if (!res) {
-               res = vfs_readlink(dentry, buffer, buflen, nd_get_link(&nd));
+       cookie = dentry->d_inode->i_op->follow_link(dentry, &nd);
+       if (!IS_ERR(cookie)) {
+               int res = vfs_readlink(dentry, buffer, buflen, nd_get_link(&nd));
                if (dentry->d_inode->i_op->put_link)
-                       dentry->d_inode->i_op->put_link(dentry, &nd);
+                       dentry->d_inode->i_op->put_link(dentry, &nd, cookie);
+               cookie = ERR_PTR(res);
        }
-       return res;
+       return PTR_ERR(cookie);
 }
 
 int vfs_follow_link(struct nameidata *nd, const char *link)
@@ -2395,23 +2400,20 @@ int page_readlink(struct dentry *dentry, char __user *buffer, int buflen)
        return res;
 }
 
-int page_follow_link_light(struct dentry *dentry, struct nameidata *nd)
+void *page_follow_link_light(struct dentry *dentry, struct nameidata *nd)
 {
-       struct page *page;
+       struct page *page = NULL;
        nd_set_link(nd, page_getlink(dentry, &page));
-       return 0;
+       return page;
 }
 
-void page_put_link(struct dentry *dentry, struct nameidata *nd)
+void page_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
 {
-       if (!IS_ERR(nd_get_link(nd))) {
-               struct page *page;
-               page = find_get_page(dentry->d_inode->i_mapping, 0);
-               if (!page)
-                       BUG();
+       struct page *page = cookie;
+
+       if (page) {
                kunmap(page);
                page_cache_release(page);
-               page_cache_release(page);
        }
 }
 

diff --git a/fs/nfs/symlink.c b/fs/nfs/symlink.c
index 35f1065991441c3f3d02f6749692da739b8492b3..18dc95b0b64638695a9dfa9b6db1983875d9a757 100644 (file)
--- a/fs/nfs/symlink.c
+++ b/fs/nfs/symlink.c
@@ -27,26 +27,14 @@
 
 /* Symlink caching in the page cache is even more simplistic
  * and straight-forward than readdir caching.
- *
- * At the beginning of the page we store pointer to struct page in question,
- * simplifying nfs_put_link() (if inode got invalidated we can't find the page
- * to be freed via pagecache lookup).
- * The NUL-terminated string follows immediately thereafter.
  */
 
-struct nfs_symlink {
-       struct page *page;
-       char body[0];
-};
-
 static int nfs_symlink_filler(struct inode *inode, struct page *page)
 {
-       const unsigned int pgbase = offsetof(struct nfs_symlink, body);
-       const unsigned int pglen = PAGE_SIZE - pgbase;
        int error;
 
        lock_kernel();
-       error = NFS_PROTO(inode)->readlink(inode, page, pgbase, pglen);
+       error = NFS_PROTO(inode)->readlink(inode, page, 0, PAGE_SIZE);
        unlock_kernel();
        if (error < 0)
                goto error;
@@ -60,11 +48,10 @@ error:
        return -EIO;
 }
 
-static int nfs_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *nfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        struct inode *inode = dentry->d_inode;
        struct page *page;
-       struct nfs_symlink *p;
        void *err = ERR_PTR(nfs_revalidate_inode(NFS_SERVER(inode), inode));
        if (err)
                goto read_failed;
@@ -78,28 +65,20 @@ static int nfs_follow_link(struct dentry *dentry, struct nameidata *nd)
                err = ERR_PTR(-EIO);
                goto getlink_read_error;
        }
-       p = kmap(page);
-       p->page = page;
-       nd_set_link(nd, p->body);
-       return 0;
+       nd_set_link(nd, kmap(page));
+       return page;
 
 getlink_read_error:
        page_cache_release(page);
 read_failed:
        nd_set_link(nd, err);
-       return 0;
+       return NULL;
 }
 
-static void nfs_put_link(struct dentry *dentry, struct nameidata *nd)
+static void nfs_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
 {
-       char *s = nd_get_link(nd);
-       if (!IS_ERR(s)) {
-               struct nfs_symlink *p;
-               struct page *page;
-
-               p = container_of(s, struct nfs_symlink, body[0]);
-               page = p->page;
-
+       if (cookie) {
+               struct page *page = cookie;
                kunmap(page);
                page_cache_release(page);
        }

diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index fae57c83a722f4e782a2651e7dc0b74a66dfee76..de402fa915f2778e010d20bfeca2c6ebc05f6ddd 100644 (file)
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -151,17 +151,17 @@ static int sysfs_getlink(struct dentry *dentry, char * path)
 
 }
 
-static int sysfs_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *sysfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        int error = -ENOMEM;
        unsigned long page = get_zeroed_page(GFP_KERNEL);
        if (page)
                error = sysfs_getlink(dentry, (char *) page); 
        nd_set_link(nd, error ? ERR_PTR(error) : (char *)page);
-       return 0;
+       return NULL;
 }
 
-static void sysfs_put_link(struct dentry *dentry, struct nameidata *nd)
+static void sysfs_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
 {
        char *page = nd_get_link(nd);
        if (!IS_ERR(page))

diff --git a/include/linux/fs.h b/include/linux/fs.h
index f9adf75fd9b4badc81b1e5d61df8ba211615f579..67e6732d4fdc736ec39f4c57324e4d64064e063e 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -993,8 +993,8 @@ struct inode_operations {
        int (*rename) (struct inode *, struct dentry *,
                        struct inode *, struct dentry *);
        int (*readlink) (struct dentry *, char __user *,int);
-       int (*follow_link) (struct dentry *, struct nameidata *);
-       void (*put_link) (struct dentry *, struct nameidata *);
+       void * (*follow_link) (struct dentry *, struct nameidata *);
+       void (*put_link) (struct dentry *, struct nameidata *, void *);
        void (*truncate) (struct inode *);
        int (*permission) (struct inode *, int, struct nameidata *);
        int (*setattr) (struct dentry *, struct iattr *);
@@ -1602,8 +1602,8 @@ extern struct file_operations generic_ro_fops;
 extern int vfs_readlink(struct dentry *, char __user *, int, const char *);
 extern int vfs_follow_link(struct nameidata *, const char *);
 extern int page_readlink(struct dentry *, char __user *, int);
-extern int page_follow_link_light(struct dentry *, struct nameidata *);
-extern void page_put_link(struct dentry *, struct nameidata *);
+extern void *page_follow_link_light(struct dentry *, struct nameidata *);
+extern void page_put_link(struct dentry *, struct nameidata *, void *);
 extern int page_symlink(struct inode *inode, const char *symname, int len);
 extern struct inode_operations page_symlink_inode_operations;
 extern int generic_readlink(struct dentry *, char __user *, int);

diff --git a/mm/shmem.c b/mm/shmem.c
index e64fa726a790e0c4162fec88db0e1032dd8a5be0..5a81b1ee4f7a43fc972d7ec762f2d51d7ac8aa42 100644 (file)
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1773,32 +1773,27 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
        return 0;
 }
 
-static int shmem_follow_link_inline(struct dentry *dentry, struct nameidata *nd)
+static void *shmem_follow_link_inline(struct dentry *dentry, struct nameidata *nd)
 {
        nd_set_link(nd, (char *)SHMEM_I(dentry->d_inode));
-       return 0;
+       return NULL;
 }
 
-static int shmem_follow_link(struct dentry *dentry, struct nameidata *nd)
+static void *shmem_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        struct page *page = NULL;
        int res = shmem_getpage(dentry->d_inode, 0, &page, SGP_READ, NULL);
        nd_set_link(nd, res ? ERR_PTR(res) : kmap(page));
-       return 0;
+       return page;
 }
 
-static void shmem_put_link(struct dentry *dentry, struct nameidata *nd)
+static void shmem_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie)
 {
        if (!IS_ERR(nd_get_link(nd))) {
-               struct page *page;
-
-               page = find_get_page(dentry->d_inode->i_mapping, 0);
-               if (!page)
-                       BUG();
+               struct page *page = cookie;
                kunmap(page);
                mark_page_accessed(page);
                page_cache_release(page);
-               page_cache_release(page);
        }
 }