Commit cc314eef authored by Linus Torvalds's avatar Linus Torvalds

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: default avatarLinus Torvalds <torvalds@osdl.org>
parent 2fb1e308
...@@ -12,11 +12,12 @@ ...@@ -12,11 +12,12 @@
#include "autofs_i.h" #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; char *s=((struct autofs_symlink *)dentry->d_inode->u.generic_ip)->data;
nd_set_link(nd, s); nd_set_link(nd, s);
return 0; return NULL;
} }
struct inode_operations autofs_symlink_inode_operations = { struct inode_operations autofs_symlink_inode_operations = {
......
...@@ -83,8 +83,8 @@ extern int cifs_dir_notify(struct file *, unsigned long arg); ...@@ -83,8 +83,8 @@ extern int cifs_dir_notify(struct file *, unsigned long arg);
extern struct dentry_operations cifs_dentry_ops; extern struct dentry_operations cifs_dentry_ops;
/* Functions related to symlinks */ /* Functions related to symlinks */
extern int cifs_follow_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); extern void cifs_put_link(struct dentry *direntry, struct nameidata *nd, void *);
extern int cifs_readlink(struct dentry *direntry, char __user *buffer, extern int cifs_readlink(struct dentry *direntry, char __user *buffer,
int buflen); int buflen);
extern int cifs_symlink(struct inode *inode, struct dentry *direntry, extern int cifs_symlink(struct inode *inode, struct dentry *direntry,
......
...@@ -92,7 +92,7 @@ cifs_hardlink(struct dentry *old_file, struct inode *inode, ...@@ -92,7 +92,7 @@ cifs_hardlink(struct dentry *old_file, struct inode *inode,
return rc; return rc;
} }
int void *
cifs_follow_link(struct dentry *direntry, struct nameidata *nd) cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
{ {
struct inode *inode = direntry->d_inode; struct inode *inode = direntry->d_inode;
...@@ -148,7 +148,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) ...@@ -148,7 +148,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
out_no_free: out_no_free:
FreeXid(xid); FreeXid(xid);
nd_set_link(nd, target_path); nd_set_link(nd, target_path);
return 0; return NULL; /* No cookie */
} }
int int
...@@ -330,7 +330,7 @@ cifs_readlink(struct dentry *direntry, char __user *pBuffer, int buflen) ...@@ -330,7 +330,7 @@ cifs_readlink(struct dentry *direntry, char __user *pBuffer, int buflen)
return rc; 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); char *p = nd_get_link(nd);
if (!IS_ERR(p)) if (!IS_ERR(p))
......
...@@ -21,11 +21,11 @@ ...@@ -21,11 +21,11 @@
#include "xattr.h" #include "xattr.h"
#include <linux/namei.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); struct ext2_inode_info *ei = EXT2_I(dentry->d_inode);
nd_set_link(nd, (char *)ei->i_data); nd_set_link(nd, (char *)ei->i_data);
return 0; return NULL;
} }
struct inode_operations ext2_symlink_inode_operations = { struct inode_operations ext2_symlink_inode_operations = {
......
...@@ -23,11 +23,11 @@ ...@@ -23,11 +23,11 @@
#include <linux/namei.h> #include <linux/namei.h>
#include "xattr.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); struct ext3_inode_info *ei = EXT3_I(dentry->d_inode);
nd_set_link(nd, (char*)ei->i_data); nd_set_link(nd, (char*)ei->i_data);
return 0; return NULL;
} }
struct inode_operations ext3_symlink_inode_operations = { struct inode_operations ext3_symlink_inode_operations = {
......
...@@ -501,6 +501,7 @@ struct path { ...@@ -501,6 +501,7 @@ struct path {
static inline int __do_follow_link(struct path *path, struct nameidata *nd) static inline int __do_follow_link(struct path *path, struct nameidata *nd)
{ {
int error; int error;
void *cookie;
struct dentry *dentry = path->dentry; struct dentry *dentry = path->dentry;
touch_atime(path->mnt, dentry); touch_atime(path->mnt, dentry);
...@@ -508,13 +509,15 @@ static inline int __do_follow_link(struct path *path, struct nameidata *nd) ...@@ -508,13 +509,15 @@ static inline int __do_follow_link(struct path *path, struct nameidata *nd)
if (path->mnt == nd->mnt) if (path->mnt == nd->mnt)
mntget(path->mnt); mntget(path->mnt);
error = dentry->d_inode->i_op->follow_link(dentry, nd); cookie = dentry->d_inode->i_op->follow_link(dentry, nd);
if (!error) { error = PTR_ERR(cookie);
if (!IS_ERR(cookie)) {
char *s = nd_get_link(nd); char *s = nd_get_link(nd);
error = 0;
if (s) if (s)
error = __vfs_follow_link(nd, s); error = __vfs_follow_link(nd, s);
if (dentry->d_inode->i_op->put_link) 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); dput(dentry);
mntput(path->mnt); mntput(path->mnt);
...@@ -2344,15 +2347,17 @@ int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen, const c ...@@ -2344,15 +2347,17 @@ int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen, const c
int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen) int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
{ {
struct nameidata nd; struct nameidata nd;
int res; void *cookie;
nd.depth = 0; nd.depth = 0;
res = dentry->d_inode->i_op->follow_link(dentry, &nd); cookie = dentry->d_inode->i_op->follow_link(dentry, &nd);
if (!res) { if (!IS_ERR(cookie)) {
res = vfs_readlink(dentry, buffer, buflen, nd_get_link(&nd)); int res = vfs_readlink(dentry, buffer, buflen, nd_get_link(&nd));
if (dentry->d_inode->i_op->put_link) 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) 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) ...@@ -2395,23 +2400,20 @@ int page_readlink(struct dentry *dentry, char __user *buffer, int buflen)
return res; 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)); 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 = cookie;
struct page *page;
page = find_get_page(dentry->d_inode->i_mapping, 0); if (page) {
if (!page)
BUG();
kunmap(page); kunmap(page);
page_cache_release(page); page_cache_release(page);
page_cache_release(page);
} }
} }
......
...@@ -27,26 +27,14 @@ ...@@ -27,26 +27,14 @@
/* Symlink caching in the page cache is even more simplistic /* Symlink caching in the page cache is even more simplistic
* and straight-forward than readdir caching. * 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) 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; int error;
lock_kernel(); lock_kernel();
error = NFS_PROTO(inode)->readlink(inode, page, pgbase, pglen); error = NFS_PROTO(inode)->readlink(inode, page, 0, PAGE_SIZE);
unlock_kernel(); unlock_kernel();
if (error < 0) if (error < 0)
goto error; goto error;
...@@ -60,11 +48,10 @@ static int nfs_symlink_filler(struct inode *inode, struct page *page) ...@@ -60,11 +48,10 @@ static int nfs_symlink_filler(struct inode *inode, struct page *page)
return -EIO; 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 inode *inode = dentry->d_inode;
struct page *page; struct page *page;
struct nfs_symlink *p;
void *err = ERR_PTR(nfs_revalidate_inode(NFS_SERVER(inode), inode)); void *err = ERR_PTR(nfs_revalidate_inode(NFS_SERVER(inode), inode));
if (err) if (err)
goto read_failed; goto read_failed;
...@@ -78,28 +65,20 @@ static int nfs_follow_link(struct dentry *dentry, struct nameidata *nd) ...@@ -78,28 +65,20 @@ static int nfs_follow_link(struct dentry *dentry, struct nameidata *nd)
err = ERR_PTR(-EIO); err = ERR_PTR(-EIO);
goto getlink_read_error; goto getlink_read_error;
} }
p = kmap(page); nd_set_link(nd, kmap(page));
p->page = page; return page;
nd_set_link(nd, p->body);
return 0;
getlink_read_error: getlink_read_error:
page_cache_release(page); page_cache_release(page);
read_failed: read_failed:
nd_set_link(nd, err); 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 (cookie) {
if (!IS_ERR(s)) { struct page *page = cookie;
struct nfs_symlink *p;
struct page *page;
p = container_of(s, struct nfs_symlink, body[0]);
page = p->page;
kunmap(page); kunmap(page);
page_cache_release(page); page_cache_release(page);
} }
......
...@@ -151,17 +151,17 @@ static int sysfs_getlink(struct dentry *dentry, char * path) ...@@ -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; int error = -ENOMEM;
unsigned long page = get_zeroed_page(GFP_KERNEL); unsigned long page = get_zeroed_page(GFP_KERNEL);
if (page) if (page)
error = sysfs_getlink(dentry, (char *) page); error = sysfs_getlink(dentry, (char *) page);
nd_set_link(nd, error ? ERR_PTR(error) : (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); char *page = nd_get_link(nd);
if (!IS_ERR(page)) if (!IS_ERR(page))
......
...@@ -993,8 +993,8 @@ struct inode_operations { ...@@ -993,8 +993,8 @@ struct inode_operations {
int (*rename) (struct inode *, struct dentry *, int (*rename) (struct inode *, struct dentry *,
struct inode *, struct dentry *); struct inode *, struct dentry *);
int (*readlink) (struct dentry *, char __user *,int); int (*readlink) (struct dentry *, char __user *,int);
int (*follow_link) (struct dentry *, struct nameidata *); void * (*follow_link) (struct dentry *, struct nameidata *);
void (*put_link) (struct dentry *, struct nameidata *); void (*put_link) (struct dentry *, struct nameidata *, void *);
void (*truncate) (struct inode *); void (*truncate) (struct inode *);
int (*permission) (struct inode *, int, struct nameidata *); int (*permission) (struct inode *, int, struct nameidata *);
int (*setattr) (struct dentry *, struct iattr *); int (*setattr) (struct dentry *, struct iattr *);
...@@ -1602,8 +1602,8 @@ extern struct file_operations generic_ro_fops; ...@@ -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_readlink(struct dentry *, char __user *, int, const char *);
extern int vfs_follow_link(struct nameidata *, const char *); extern int vfs_follow_link(struct nameidata *, const char *);
extern int page_readlink(struct dentry *, char __user *, int); extern int page_readlink(struct dentry *, char __user *, int);
extern int page_follow_link_light(struct dentry *, struct nameidata *); extern void *page_follow_link_light(struct dentry *, struct nameidata *);
extern void page_put_link(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 int page_symlink(struct inode *inode, const char *symname, int len);
extern struct inode_operations page_symlink_inode_operations; extern struct inode_operations page_symlink_inode_operations;
extern int generic_readlink(struct dentry *, char __user *, int); extern int generic_readlink(struct dentry *, char __user *, int);
......
...@@ -1773,32 +1773,27 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s ...@@ -1773,32 +1773,27 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
return 0; 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)); 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; struct page *page = NULL;
int res = shmem_getpage(dentry->d_inode, 0, &page, SGP_READ, NULL); int res = shmem_getpage(dentry->d_inode, 0, &page, SGP_READ, NULL);
nd_set_link(nd, res ? ERR_PTR(res) : kmap(page)); 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))) { if (!IS_ERR(nd_get_link(nd))) {
struct page *page; struct page *page = cookie;
page = find_get_page(dentry->d_inode->i_mapping, 0);
if (!page)
BUG();
kunmap(page); kunmap(page);
mark_page_accessed(page); mark_page_accessed(page);
page_cache_release(page); page_cache_release(page);
page_cache_release(page);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment