From 1025774ce411f2bd4b059ad7b53f0003569b74fa Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 4 Jun 2010 11:30:02 +0200 Subject: remove inode_setattr Replace inode_setattr with opencoded variants of it in all callers. This moves the remaining call to vmtruncate into the filesystem methods where it can be replaced with the proper truncate sequence. In a few cases it was obvious that we would never end up calling vmtruncate so it was left out in the opencoded variant: spufs: explicitly checks for ATTR_SIZE earlier btrfs,hugetlbfs,logfs,dlmfs: explicitly clears ATTR_SIZE earlier ufs: contains an opencoded simple_seattr + truncate that sets the filesize just above In addition to that ncpfs called inode_setattr with handcrafted iattrs, which allowed to trim down the opencoded variant. Signed-off-by: Christoph Hellwig Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 87ac1891a185..7943ff11d489 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -849,13 +849,14 @@ int hostfs_permission(struct inode *ino, int desired) int hostfs_setattr(struct dentry *dentry, struct iattr *attr) { + struct inode *inode = dentry->d_inode; struct hostfs_iattr attrs; char *name; int err; - int fd = HOSTFS_I(dentry->d_inode)->fd; + int fd = HOSTFS_I(inode)->fd; - err = inode_change_ok(dentry->d_inode, attr); + err = inode_change_ok(inode, attr); if (err) return err; @@ -905,7 +906,18 @@ int hostfs_setattr(struct dentry *dentry, struct iattr *attr) if (err) return err; - return inode_setattr(dentry->d_inode, attr); + if ((attr->ia_valid & ATTR_SIZE) && + attr->ia_size != i_size_read(inode)) { + int error; + + error = vmtruncate(inode, attr->ia_size); + if (err) + return err; + } + + setattr_copy(inode, attr); + mark_inode_dirty(inode); + return 0; } static const struct inode_operations hostfs_iops = { -- cgit v1.2.2 From e971a6d7b9daebfe2c11c590377d3933410ab929 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 15:16:17 -0400 Subject: stop icache pollution in hostfs, switch to ->evict_inode() Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 7943ff11d489..fab5f5a1e6f0 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -241,16 +241,13 @@ static struct inode *hostfs_iget(struct super_block *sb) struct inode *inode; long ret; - inode = iget_locked(sb, 0); + inode = new_inode(sb); if (!inode) return ERR_PTR(-ENOMEM); - if (inode->i_state & I_NEW) { - ret = hostfs_read_inode(inode); - if (ret < 0) { - iget_failed(inode); - return ERR_PTR(ret); - } - unlock_new_inode(inode); + ret = hostfs_read_inode(inode); + if (ret < 0) { + iput(inode); + return ERR_PTR(ret); } return inode; } @@ -299,29 +296,19 @@ static struct inode *hostfs_alloc_inode(struct super_block *sb) return &hi->vfs_inode; } -static void hostfs_delete_inode(struct inode *inode) +static void hostfs_evict_inode(struct inode *inode) { truncate_inode_pages(&inode->i_data, 0); + end_writeback(inode); if (HOSTFS_I(inode)->fd != -1) { close_file(&HOSTFS_I(inode)->fd); HOSTFS_I(inode)->fd = -1; } - clear_inode(inode); } static void hostfs_destroy_inode(struct inode *inode) { kfree(HOSTFS_I(inode)->host_filename); - - /* - * XXX: This should not happen, probably. The check is here for - * additional safety. - */ - if (HOSTFS_I(inode)->fd != -1) { - close_file(&HOSTFS_I(inode)->fd); - printk(KERN_DEBUG "Closing host fd in .destroy_inode\n"); - } - kfree(HOSTFS_I(inode)); } @@ -339,9 +326,8 @@ static int hostfs_show_options(struct seq_file *seq, struct vfsmount *vfs) static const struct super_operations hostfs_sbops = { .alloc_inode = hostfs_alloc_inode, - .drop_inode = generic_delete_inode, - .delete_inode = hostfs_delete_inode, .destroy_inode = hostfs_destroy_inode, + .evict_inode = hostfs_evict_inode, .statfs = hostfs_statfs, .show_options = hostfs_show_options, }; -- cgit v1.2.2 From 601d2c38b93130d387091c28d13abea40924e518 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 17:53:01 -0400 Subject: hostfs: don't keep a field in each inode when we are using it only in root Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index fab5f5a1e6f0..7e6750499b8b 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -19,7 +19,6 @@ #include "kern.h" struct hostfs_inode_info { - char *host_filename; int fd; fmode_t mode; struct inode vfs_inode; @@ -103,7 +102,7 @@ static char *dentry_name(struct dentry *dentry, int extra) parent = parent->d_parent; } - root = HOSTFS_I(parent->d_inode)->host_filename; + root = parent->d_sb->s_fs_info; len += strlen(root); name = kmalloc(len + extra + 1, GFP_KERNEL); if (name == NULL) @@ -266,7 +265,7 @@ int hostfs_statfs(struct dentry *dentry, struct kstatfs *sf) long long f_files; long long f_ffree; - err = do_statfs(HOSTFS_I(dentry->d_sb->s_root->d_inode)->host_filename, + err = do_statfs(dentry->d_sb->s_fs_info, &sf->f_bsize, &f_blocks, &f_bfree, &f_bavail, &f_files, &f_ffree, &sf->f_fsid, sizeof(sf->f_fsid), &sf->f_namelen, sf->f_spare); @@ -285,13 +284,10 @@ static struct inode *hostfs_alloc_inode(struct super_block *sb) { struct hostfs_inode_info *hi; - hi = kmalloc(sizeof(*hi), GFP_KERNEL); + hi = kzalloc(sizeof(*hi), GFP_KERNEL); if (hi == NULL) return NULL; - - *hi = ((struct hostfs_inode_info) { .host_filename = NULL, - .fd = -1, - .mode = 0 }); + hi->fd = -1; inode_init_once(&hi->vfs_inode); return &hi->vfs_inode; } @@ -308,14 +304,12 @@ static void hostfs_evict_inode(struct inode *inode) static void hostfs_destroy_inode(struct inode *inode) { - kfree(HOSTFS_I(inode)->host_filename); kfree(HOSTFS_I(inode)); } static int hostfs_show_options(struct seq_file *seq, struct vfsmount *vfs) { - struct inode *root = vfs->mnt_sb->s_root->d_inode; - const char *root_path = HOSTFS_I(root)->host_filename; + const char *root_path = vfs->mnt_sb->s_fs_info; size_t offset = strlen(root_ino) + 1; if (strlen(root_path) > offset) @@ -978,8 +972,8 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) req_root = ""; err = -ENOMEM; - host_root_path = kmalloc(strlen(root_ino) + 1 - + strlen(req_root) + 1, GFP_KERNEL); + sb->s_fs_info = host_root_path = + kmalloc(strlen(root_ino) + strlen(req_root) + 2, GFP_KERNEL); if (host_root_path == NULL) goto out; @@ -988,20 +982,13 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) root_inode = hostfs_iget(sb); if (IS_ERR(root_inode)) { err = PTR_ERR(root_inode); - goto out_free; + goto out; } err = init_inode(root_inode, NULL); if (err) goto out_put; - HOSTFS_I(root_inode)->host_filename = host_root_path; - /* - * Avoid that in the error path, iput(root_inode) frees again - * host_root_path through hostfs_destroy_inode! - */ - host_root_path = NULL; - err = -ENOMEM; sb->s_root = d_alloc_root(root_inode); if (sb->s_root == NULL) @@ -1019,8 +1006,6 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) out_put: iput(root_inode); -out_free: - kfree(host_root_path); out: return err; } @@ -1032,11 +1017,17 @@ static int hostfs_read_sb(struct file_system_type *type, return get_sb_nodev(type, flags, data, hostfs_fill_sb_common, mnt); } +static void hostfs_kill_sb(struct super_block *s) +{ + kill_anon_super(s); + kfree(s->s_fs_info); +} + static struct file_system_type hostfs_type = { .owner = THIS_MODULE, .name = "hostfs", .get_sb = hostfs_read_sb, - .kill_sb = kill_anon_super, + .kill_sb = hostfs_kill_sb, .fs_flags = 0, }; -- cgit v1.2.2 From 52b209f7b848a28987ed133dc2b48f304b1dc6b8 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 18:43:19 -0400 Subject: get rid of hostfs_read_inode() There are only two call sites; in one (hostfs_iget()) it's actually a no-op and in another (fill_super()) it's easier to expand the damn thing and use what we know about its arguments to simplify it. Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 69 ++++++++++++------------------------------------- 1 file changed, 16 insertions(+), 53 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 7e6750499b8b..25d79298a98e 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -204,50 +204,11 @@ static char *follow_link(char *link) return ERR_PTR(n); } -static int hostfs_read_inode(struct inode *ino) -{ - char *name; - int err = 0; - - /* - * Unfortunately, we are called from iget() when we don't have a dentry - * allocated yet. - */ - if (list_empty(&ino->i_dentry)) - goto out; - - err = -ENOMEM; - name = inode_name(ino, 0); - if (name == NULL) - goto out; - - if (file_type(name, NULL, NULL) == OS_TYPE_SYMLINK) { - name = follow_link(name); - if (IS_ERR(name)) { - err = PTR_ERR(name); - goto out; - } - } - - err = read_name(ino, name); - kfree(name); - out: - return err; -} - static struct inode *hostfs_iget(struct super_block *sb) { - struct inode *inode; - long ret; - - inode = new_inode(sb); + struct inode *inode = new_inode(sb); if (!inode) return ERR_PTR(-ENOMEM); - ret = hostfs_read_inode(inode); - if (ret < 0) { - iput(inode); - return ERR_PTR(ret); - } return inode; } @@ -979,13 +940,23 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) sprintf(host_root_path, "%s/%s", root_ino, req_root); - root_inode = hostfs_iget(sb); - if (IS_ERR(root_inode)) { - err = PTR_ERR(root_inode); + root_inode = new_inode(sb); + if (!root_inode) goto out; - } - err = init_inode(root_inode, NULL); + root_inode->i_op = &hostfs_dir_iops; + root_inode->i_fop = &hostfs_dir_fops; + + if (file_type(host_root_path, NULL, NULL) == OS_TYPE_SYMLINK) { + char *name = follow_link(host_root_path); + if (IS_ERR(name)) + err = PTR_ERR(name); + else + err = read_name(root_inode, name); + kfree(name); + } else { + err = read_name(root_inode, host_root_path); + } if (err) goto out_put; @@ -994,14 +965,6 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) if (sb->s_root == NULL) goto out_put; - err = hostfs_read_inode(root_inode); - if (err) { - /* No iput in this case because the dput does that for us */ - dput(sb->s_root); - sb->s_root = NULL; - goto out; - } - return 0; out_put: -- cgit v1.2.2 From 5e2df28cc62fdc3f4900de23f4ec69e6312f78a4 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 19:38:18 -0400 Subject: hostfs: pass pathname to init_inode() We will calculate it in all callers anyway, so there's no need to duplicate that inside. Moreover, that way we lose all failure exits in init_inode(), so it doesn't need to return anything. Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 25d79298a98e..5a77ed3dfd7e 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -485,25 +485,16 @@ static const struct address_space_operations hostfs_aops = { .write_end = hostfs_write_end, }; -static int init_inode(struct inode *inode, struct dentry *dentry) +static void init_inode(struct inode *inode, char *path) { - char *name; - int type, err = -ENOMEM; + int type; int maj, min; dev_t rdev = 0; - if (dentry) { - name = dentry_name(dentry, 0); - if (name == NULL) - goto out; - type = file_type(name, &maj, &min); - /* Reencode maj and min with the kernel encoding.*/ - rdev = MKDEV(maj, min); - kfree(name); - } - else type = OS_TYPE_DIR; + type = file_type(path, &maj, &min); + /* Reencode maj and min with the kernel encoding.*/ + rdev = MKDEV(maj, min); - err = 0; if (type == OS_TYPE_SYMLINK) inode->i_op = &page_symlink_inode_operations; else if (type == OS_TYPE_DIR) @@ -531,8 +522,6 @@ static int init_inode(struct inode *inode, struct dentry *dentry) init_special_inode(inode, S_IFSOCK, 0); break; } - out: - return err; } int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, @@ -548,10 +537,6 @@ int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, goto out; } - error = init_inode(inode, dentry); - if (error) - goto out_put; - error = -ENOMEM; name = dentry_name(dentry, 0); if (name == NULL) @@ -561,9 +546,12 @@ int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, mode & S_IRUSR, mode & S_IWUSR, mode & S_IXUSR, mode & S_IRGRP, mode & S_IWGRP, mode & S_IXGRP, mode & S_IROTH, mode & S_IWOTH, mode & S_IXOTH); - if (fd < 0) + if (fd < 0) { error = fd; - else error = read_name(inode, name); + } else { + error = read_name(inode, name); + init_inode(inode, name); + } kfree(name); if (error) @@ -593,16 +581,14 @@ struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry, goto out; } - err = init_inode(inode, dentry); - if (err) - goto out_put; - err = -ENOMEM; name = dentry_name(dentry, 0); if (name == NULL) goto out_put; err = read_name(inode, name); + init_inode(inode, name); + kfree(name); if (err == -ENOENT) { iput(inode); @@ -717,10 +703,6 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev) goto out; } - err = init_inode(inode, dentry); - if (err) - goto out_put; - err = -ENOMEM; name = dentry_name(dentry, 0); if (name == NULL) @@ -732,6 +714,9 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev) goto out_free; err = read_name(inode, name); + init_inode(inode, name); + if (err) + goto out_put; kfree(name); if (err) goto out_put; -- cgit v1.2.2 From 39b743c6199a317ffac67fcae1dd05be3142633a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 20:08:56 -0400 Subject: switch stat_file() to passing a single struct rather than fsckloads of pointers Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 5a77ed3dfd7e..420a826ae0f2 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -131,28 +131,21 @@ static char *inode_name(struct inode *ino, int extra) static int read_name(struct inode *ino, char *name) { - /* - * The non-int inode fields are copied into ints by stat_file and - * then copied into the inode because passing the actual pointers - * in and having them treated as int * breaks on big-endian machines - */ - int err; - int i_mode, i_nlink, i_blksize; - unsigned long long i_size; - unsigned long long i_ino; - unsigned long long i_blocks; - - err = stat_file(name, &i_ino, &i_mode, &i_nlink, &ino->i_uid, - &ino->i_gid, &i_size, &ino->i_atime, &ino->i_mtime, - &ino->i_ctime, &i_blksize, &i_blocks, -1); + struct hostfs_stat st; + int err = stat_file(name, &st, -1); if (err) return err; - ino->i_ino = i_ino; - ino->i_mode = i_mode; - ino->i_nlink = i_nlink; - ino->i_size = i_size; - ino->i_blocks = i_blocks; + ino->i_ino = st.ino; + ino->i_mode = st.mode; + ino->i_nlink = st.nlink; + ino->i_uid = st.uid; + ino->i_gid = st.gid; + ino->i_atime = st.atime; + ino->i_mtime = st.mtime; + ino->i_ctime = st.ctime; + ino->i_size = st.size; + ino->i_blocks = st.blocks; return 0; } -- cgit v1.2.2 From 4754b825571a6f2f7655245e420e8e486c4458f6 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 20:33:12 -0400 Subject: hostfs: get rid of file_type(), fold init_inode() Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 107 ++++++++++++++++++++---------------------------- 1 file changed, 45 insertions(+), 62 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 420a826ae0f2..b29a2b878f46 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -129,26 +129,6 @@ static char *inode_name(struct inode *ino, int extra) return dentry_name(dentry, extra); } -static int read_name(struct inode *ino, char *name) -{ - struct hostfs_stat st; - int err = stat_file(name, &st, -1); - if (err) - return err; - - ino->i_ino = st.ino; - ino->i_mode = st.mode; - ino->i_nlink = st.nlink; - ino->i_uid = st.uid; - ino->i_gid = st.gid; - ino->i_atime = st.atime; - ino->i_mtime = st.mtime; - ino->i_ctime = st.ctime; - ino->i_size = st.size; - ino->i_blocks = st.blocks; - return 0; -} - static char *follow_link(char *link) { int len, n; @@ -478,43 +458,51 @@ static const struct address_space_operations hostfs_aops = { .write_end = hostfs_write_end, }; -static void init_inode(struct inode *inode, char *path) +static int read_name(struct inode *ino, char *name) { - int type; - int maj, min; - dev_t rdev = 0; + dev_t rdev; + struct hostfs_stat st; + int err = stat_file(name, &st, -1); + if (err) + return err; - type = file_type(path, &maj, &min); /* Reencode maj and min with the kernel encoding.*/ - rdev = MKDEV(maj, min); + rdev = MKDEV(st.maj, st.min); - if (type == OS_TYPE_SYMLINK) - inode->i_op = &page_symlink_inode_operations; - else if (type == OS_TYPE_DIR) - inode->i_op = &hostfs_dir_iops; - else inode->i_op = &hostfs_iops; - - if (type == OS_TYPE_DIR) inode->i_fop = &hostfs_dir_fops; - else inode->i_fop = &hostfs_file_fops; - - if (type == OS_TYPE_SYMLINK) - inode->i_mapping->a_ops = &hostfs_link_aops; - else inode->i_mapping->a_ops = &hostfs_aops; - - switch (type) { - case OS_TYPE_CHARDEV: - init_special_inode(inode, S_IFCHR, rdev); - break; - case OS_TYPE_BLOCKDEV: - init_special_inode(inode, S_IFBLK, rdev); + switch (st.mode & S_IFMT) { + case S_IFLNK: + ino->i_op = &page_symlink_inode_operations; + ino->i_mapping->a_ops = &hostfs_link_aops; break; - case OS_TYPE_FIFO: - init_special_inode(inode, S_IFIFO, 0); + case S_IFDIR: + ino->i_op = &hostfs_dir_iops; + ino->i_fop = &hostfs_dir_fops; break; - case OS_TYPE_SOCK: - init_special_inode(inode, S_IFSOCK, 0); + case S_IFCHR: + case S_IFBLK: + case S_IFIFO: + case S_IFSOCK: + init_special_inode(ino, st.mode & S_IFMT, rdev); + ino->i_op = &hostfs_iops; break; + + default: + ino->i_op = &hostfs_iops; + ino->i_fop = &hostfs_file_fops; + ino->i_mapping->a_ops = &hostfs_aops; } + + ino->i_ino = st.ino; + ino->i_mode = st.mode; + ino->i_nlink = st.nlink; + ino->i_uid = st.uid; + ino->i_gid = st.gid; + ino->i_atime = st.atime; + ino->i_mtime = st.mtime; + ino->i_ctime = st.ctime; + ino->i_size = st.size; + ino->i_blocks = st.blocks; + return 0; } int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, @@ -539,12 +527,10 @@ int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, mode & S_IRUSR, mode & S_IWUSR, mode & S_IXUSR, mode & S_IRGRP, mode & S_IWGRP, mode & S_IXGRP, mode & S_IROTH, mode & S_IWOTH, mode & S_IXOTH); - if (fd < 0) { + if (fd < 0) error = fd; - } else { + else error = read_name(inode, name); - init_inode(inode, name); - } kfree(name); if (error) @@ -580,7 +566,6 @@ struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry, goto out_put; err = read_name(inode, name); - init_inode(inode, name); kfree(name); if (err == -ENOENT) { @@ -707,7 +692,6 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev) goto out_free; err = read_name(inode, name); - init_inode(inode, name); if (err) goto out_put; kfree(name); @@ -922,21 +906,20 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) if (!root_inode) goto out; - root_inode->i_op = &hostfs_dir_iops; - root_inode->i_fop = &hostfs_dir_fops; + err = read_name(root_inode, host_root_path); + if (err) + goto out_put; - if (file_type(host_root_path, NULL, NULL) == OS_TYPE_SYMLINK) { + if (S_ISLNK(root_inode->i_mode)) { char *name = follow_link(host_root_path); if (IS_ERR(name)) err = PTR_ERR(name); else err = read_name(root_inode, name); kfree(name); - } else { - err = read_name(root_inode, host_root_path); + if (err) + goto out_put; } - if (err) - goto out_put; err = -ENOMEM; sb->s_root = d_alloc_root(root_inode); -- cgit v1.2.2 From c5322220eb91b9e56ac7b69eb690d9d20fac5725 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 20:42:10 -0400 Subject: hostfs: get rid of inode_dentry_name() it's equivalent to dentry_name() anyway Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 55 ++++++++++++++++++------------------------------- 1 file changed, 20 insertions(+), 35 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index b29a2b878f46..3841fb1ca5a2 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -89,7 +89,7 @@ __uml_setup("hostfs=", hostfs_args, ); #endif -static char *dentry_name(struct dentry *dentry, int extra) +static char *dentry_name(struct dentry *dentry) { struct dentry *parent; char *root, *name; @@ -104,7 +104,7 @@ static char *dentry_name(struct dentry *dentry, int extra) root = parent->d_sb->s_fs_info; len += strlen(root); - name = kmalloc(len + extra + 1, GFP_KERNEL); + name = kmalloc(len + 1, GFP_KERNEL); if (name == NULL) return NULL; @@ -121,12 +121,12 @@ static char *dentry_name(struct dentry *dentry, int extra) return name; } -static char *inode_name(struct inode *ino, int extra) +static char *inode_name(struct inode *ino) { struct dentry *dentry; dentry = list_entry(ino->i_dentry.next, struct dentry, d_alias); - return dentry_name(dentry, extra); + return dentry_name(dentry); } static char *follow_link(char *link) @@ -267,7 +267,7 @@ int hostfs_readdir(struct file *file, void *ent, filldir_t filldir) unsigned long long next, ino; int error, len; - name = dentry_name(file->f_path.dentry, 0); + name = dentry_name(file->f_path.dentry); if (name == NULL) return -ENOMEM; dir = open_dir(name, &error); @@ -312,7 +312,7 @@ int hostfs_file_open(struct inode *ino, struct file *file) if (w) r = 1; - name = dentry_name(file->f_path.dentry, 0); + name = dentry_name(file->f_path.dentry); if (name == NULL) return -ENOMEM; @@ -519,7 +519,7 @@ int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, } error = -ENOMEM; - name = dentry_name(dentry, 0); + name = dentry_name(dentry); if (name == NULL) goto out_put; @@ -561,7 +561,7 @@ struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry, } err = -ENOMEM; - name = dentry_name(dentry, 0); + name = dentry_name(dentry); if (name == NULL) goto out_put; @@ -585,29 +585,14 @@ struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry, return ERR_PTR(err); } -static char *inode_dentry_name(struct inode *ino, struct dentry *dentry) -{ - char *file; - int len; - - file = inode_name(ino, dentry->d_name.len + 1); - if (file == NULL) - return NULL; - strcat(file, "/"); - len = strlen(file); - strncat(file, dentry->d_name.name, dentry->d_name.len); - file[len + dentry->d_name.len] = '\0'; - return file; -} - int hostfs_link(struct dentry *to, struct inode *ino, struct dentry *from) { char *from_name, *to_name; int err; - if ((from_name = inode_dentry_name(ino, from)) == NULL) + if ((from_name = dentry_name(from)) == NULL) return -ENOMEM; - to_name = dentry_name(to, 0); + to_name = dentry_name(to); if (to_name == NULL) { kfree(from_name); return -ENOMEM; @@ -623,7 +608,7 @@ int hostfs_unlink(struct inode *ino, struct dentry *dentry) char *file; int err; - if ((file = inode_dentry_name(ino, dentry)) == NULL) + if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; if (append) return -EPERM; @@ -638,7 +623,7 @@ int hostfs_symlink(struct inode *ino, struct dentry *dentry, const char *to) char *file; int err; - if ((file = inode_dentry_name(ino, dentry)) == NULL) + if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; err = make_symlink(file, to); kfree(file); @@ -650,7 +635,7 @@ int hostfs_mkdir(struct inode *ino, struct dentry *dentry, int mode) char *file; int err; - if ((file = inode_dentry_name(ino, dentry)) == NULL) + if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; err = do_mkdir(file, mode); kfree(file); @@ -662,7 +647,7 @@ int hostfs_rmdir(struct inode *ino, struct dentry *dentry) char *file; int err; - if ((file = inode_dentry_name(ino, dentry)) == NULL) + if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; err = do_rmdir(file); kfree(file); @@ -682,7 +667,7 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev) } err = -ENOMEM; - name = dentry_name(dentry, 0); + name = dentry_name(dentry); if (name == NULL) goto out_put; @@ -715,9 +700,9 @@ int hostfs_rename(struct inode *from_ino, struct dentry *from, char *from_name, *to_name; int err; - if ((from_name = inode_dentry_name(from_ino, from)) == NULL) + if ((from_name = dentry_name(from)) == NULL) return -ENOMEM; - if ((to_name = inode_dentry_name(to_ino, to)) == NULL) { + if ((to_name = dentry_name(to)) == NULL) { kfree(from_name); return -ENOMEM; } @@ -735,7 +720,7 @@ int hostfs_permission(struct inode *ino, int desired) if (desired & MAY_READ) r = 1; if (desired & MAY_WRITE) w = 1; if (desired & MAY_EXEC) x = 1; - name = inode_name(ino, 0); + name = inode_name(ino); if (name == NULL) return -ENOMEM; @@ -801,7 +786,7 @@ int hostfs_setattr(struct dentry *dentry, struct iattr *attr) if (attr->ia_valid & ATTR_MTIME_SET) { attrs.ia_valid |= HOSTFS_ATTR_MTIME_SET; } - name = dentry_name(dentry, 0); + name = dentry_name(dentry); if (name == NULL) return -ENOMEM; err = set_attr(name, &attrs, fd); @@ -856,7 +841,7 @@ int hostfs_link_readpage(struct file *file, struct page *page) int err; buffer = kmap(page); - name = inode_name(page->mapping->host, 0); + name = inode_name(page->mapping->host); if (name == NULL) return -ENOMEM; err = hostfs_do_readlink(name, buffer, PAGE_CACHE_SIZE); -- cgit v1.2.2 From d0352d3ed722b134dacc21836c1763e7e3523662 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 21:51:16 -0400 Subject: hostfs: sanitize symlinks Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 61 ++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 26 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 3841fb1ca5a2..10bb71b1548b 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -14,6 +14,7 @@ #include #include #include +#include #include "hostfs.h" #include "init.h" #include "kern.h" @@ -48,7 +49,7 @@ static int append = 0; static const struct inode_operations hostfs_iops; static const struct inode_operations hostfs_dir_iops; -static const struct address_space_operations hostfs_link_aops; +static const struct inode_operations hostfs_link_iops; #ifndef MODULE static int __init hostfs_args(char *options, int *add) @@ -471,8 +472,7 @@ static int read_name(struct inode *ino, char *name) switch (st.mode & S_IFMT) { case S_IFLNK: - ino->i_op = &page_symlink_inode_operations; - ino->i_mapping->a_ops = &hostfs_link_aops; + ino->i_op = &hostfs_link_iops; break; case S_IFDIR: ino->i_op = &hostfs_dir_iops; @@ -835,32 +835,41 @@ static const struct inode_operations hostfs_dir_iops = { .setattr = hostfs_setattr, }; -int hostfs_link_readpage(struct file *file, struct page *page) -{ - char *buffer, *name; - int err; - - buffer = kmap(page); - name = inode_name(page->mapping->host); - if (name == NULL) - return -ENOMEM; - err = hostfs_do_readlink(name, buffer, PAGE_CACHE_SIZE); - kfree(name); - if (err == PAGE_CACHE_SIZE) - err = -E2BIG; - else if (err > 0) { - flush_dcache_page(page); - SetPageUptodate(page); - if (PageError(page)) ClearPageError(page); - err = 0; +static void *hostfs_follow_link(struct dentry *dentry, struct nameidata *nd) +{ + char *link = __getname(); + if (link) { + char *path = dentry_name(dentry); + int err = -ENOMEM; + if (path) { + int err = hostfs_do_readlink(path, link, PATH_MAX); + if (err == PATH_MAX) + err = -E2BIG; + kfree(path); + } + if (err < 0) { + __putname(link); + link = ERR_PTR(err); + } + } else { + link = ERR_PTR(-ENOMEM); } - kunmap(page); - unlock_page(page); - return err; + + nd_set_link(nd, link); + return NULL; +} + +static void hostfs_put_link(struct dentry *dentry, struct nameidata *nd, void *cookie) +{ + char *s = nd_get_link(nd); + if (!IS_ERR(s)) + __putname(s); } -static const struct address_space_operations hostfs_link_aops = { - .readpage = hostfs_link_readpage, +static const struct inode_operations hostfs_link_iops = { + .readlink = generic_readlink, + .follow_link = hostfs_follow_link, + .put_link = hostfs_put_link, }; static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) -- cgit v1.2.2 From e9193059b1b3733695d5b80e667778311695aa73 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 23:16:34 -0400 Subject: hostfs: fix races in dentry_name() and inode_name() calculating size, then doing allocation, then filling the path is a Bad Idea(tm), since the ancestors can be renamed, leading to buffer overrun. Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 106 +++++++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 46 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 10bb71b1548b..79783a0b2f4d 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -90,44 +90,58 @@ __uml_setup("hostfs=", hostfs_args, ); #endif -static char *dentry_name(struct dentry *dentry) +static char *__dentry_name(struct dentry *dentry, char *name) { - struct dentry *parent; - char *root, *name; - int len; + char *p = __dentry_path(dentry, name, PATH_MAX); + char *root; + size_t len; - len = 0; - parent = dentry; - while (parent->d_parent != parent) { - len += parent->d_name.len + 1; - parent = parent->d_parent; - } + spin_unlock(&dcache_lock); - root = parent->d_sb->s_fs_info; - len += strlen(root); - name = kmalloc(len + 1, GFP_KERNEL); - if (name == NULL) + root = dentry->d_sb->s_fs_info; + len = strlen(root); + if (IS_ERR(p)) { + __putname(name); return NULL; - - name[len] = '\0'; - parent = dentry; - while (parent->d_parent != parent) { - len -= parent->d_name.len + 1; - name[len] = '/'; - strncpy(&name[len + 1], parent->d_name.name, - parent->d_name.len); - parent = parent->d_parent; } - strncpy(name, root, strlen(root)); + strncpy(name, root, PATH_MAX); + if (len > p - name) { + __putname(name); + return NULL; + } + if (p > name + len) { + char *s = name + len; + while ((*s++ = *p++) != '\0') + ; + } return name; } +static char *dentry_name(struct dentry *dentry) +{ + char *name = __getname(); + if (!name) + return NULL; + + spin_lock(&dcache_lock); + return __dentry_name(dentry, name); /* will unlock */ +} + static char *inode_name(struct inode *ino) { struct dentry *dentry; + char *name = __getname(); + if (!name) + return NULL; - dentry = list_entry(ino->i_dentry.next, struct dentry, d_alias); - return dentry_name(dentry); + spin_lock(&dcache_lock); + if (list_empty(&ino->i_dentry)) { + spin_unlock(&dcache_lock); + __putname(name); + return NULL; + } + dentry = list_first_entry(&ino->i_dentry, struct dentry, d_alias); + return __dentry_name(dentry, name); /* will unlock */ } static char *follow_link(char *link) @@ -272,7 +286,7 @@ int hostfs_readdir(struct file *file, void *ent, filldir_t filldir) if (name == NULL) return -ENOMEM; dir = open_dir(name, &error); - kfree(name); + __putname(name); if (dir == NULL) return -error; next = file->f_pos; @@ -318,7 +332,7 @@ int hostfs_file_open(struct inode *ino, struct file *file) return -ENOMEM; fd = open_file(name, r, w, append); - kfree(name); + __putname(name); if (fd < 0) return fd; FILE_HOSTFS_I(file)->fd = fd; @@ -532,7 +546,7 @@ int hostfs_create(struct inode *dir, struct dentry *dentry, int mode, else error = read_name(inode, name); - kfree(name); + __putname(name); if (error) goto out_put; @@ -567,7 +581,7 @@ struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry, err = read_name(inode, name); - kfree(name); + __putname(name); if (err == -ENOENT) { iput(inode); inode = NULL; @@ -594,12 +608,12 @@ int hostfs_link(struct dentry *to, struct inode *ino, struct dentry *from) return -ENOMEM; to_name = dentry_name(to); if (to_name == NULL) { - kfree(from_name); + __putname(from_name); return -ENOMEM; } err = link_file(to_name, from_name); - kfree(from_name); - kfree(to_name); + __putname(from_name); + __putname(to_name); return err; } @@ -614,7 +628,7 @@ int hostfs_unlink(struct inode *ino, struct dentry *dentry) return -EPERM; err = unlink_file(file); - kfree(file); + __putname(file); return err; } @@ -626,7 +640,7 @@ int hostfs_symlink(struct inode *ino, struct dentry *dentry, const char *to) if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; err = make_symlink(file, to); - kfree(file); + __putname(file); return err; } @@ -638,7 +652,7 @@ int hostfs_mkdir(struct inode *ino, struct dentry *dentry, int mode) if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; err = do_mkdir(file, mode); - kfree(file); + __putname(file); return err; } @@ -650,7 +664,7 @@ int hostfs_rmdir(struct inode *ino, struct dentry *dentry) if ((file = dentry_name(dentry)) == NULL) return -ENOMEM; err = do_rmdir(file); - kfree(file); + __putname(file); return err; } @@ -673,13 +687,13 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev) init_special_inode(inode, mode, dev); err = do_mknod(name, mode, MAJOR(dev), MINOR(dev)); - if (err) + if (!err) goto out_free; err = read_name(inode, name); + __putname(name); if (err) goto out_put; - kfree(name); if (err) goto out_put; @@ -687,7 +701,7 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev) return 0; out_free: - kfree(name); + __putname(name); out_put: iput(inode); out: @@ -703,12 +717,12 @@ int hostfs_rename(struct inode *from_ino, struct dentry *from, if ((from_name = dentry_name(from)) == NULL) return -ENOMEM; if ((to_name = dentry_name(to)) == NULL) { - kfree(from_name); + __putname(from_name); return -ENOMEM; } err = rename_file(from_name, to_name); - kfree(from_name); - kfree(to_name); + __putname(from_name); + __putname(to_name); return err; } @@ -729,7 +743,7 @@ int hostfs_permission(struct inode *ino, int desired) err = 0; else err = access_file(name, r, w, x); - kfree(name); + __putname(name); if (!err) err = generic_permission(ino, desired, NULL); return err; @@ -790,7 +804,7 @@ int hostfs_setattr(struct dentry *dentry, struct iattr *attr) if (name == NULL) return -ENOMEM; err = set_attr(name, &attrs, fd); - kfree(name); + __putname(name); if (err) return err; @@ -845,7 +859,7 @@ static void *hostfs_follow_link(struct dentry *dentry, struct nameidata *nd) int err = hostfs_do_readlink(path, link, PATH_MAX); if (err == PATH_MAX) err = -E2BIG; - kfree(path); + __putname(path); } if (err < 0) { __putname(link); -- cgit v1.2.2 From f8d7e1877e5121841bc9a4d284a04dbc13f45bea Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 23:19:04 -0400 Subject: leak in hostfs_unlink() Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 79783a0b2f4d..8130ce93a06a 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -622,11 +622,12 @@ int hostfs_unlink(struct inode *ino, struct dentry *dentry) char *file; int err; - if ((file = dentry_name(dentry)) == NULL) - return -ENOMEM; if (append) return -EPERM; + if ((file = dentry_name(dentry)) == NULL) + return -ENOMEM; + err = unlink_file(file); __putname(file); return err; -- cgit v1.2.2 From f8ad850f11e11d10e7de1a16ca53cb193afc9313 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 6 Jun 2010 23:49:18 -0400 Subject: try to get rid of races in hostfs open() In case of mode mismatch, do *not* blindly close the descriptor another openers might be using right now. Open the underlying file with currently sufficient mode, then * if current mode has grown so that it's sufficient for us now, just close our new fd * if current mode has grown and our fd is *not* enough to cover it, close and repeat. * otherwise, install our fd if the file hadn't been opened at all or dup2() our fd over the current one (and close our fd). Critical section is protected by mutex; yes, system-wide. All we do under it is a bunch of comparison and maybe an overwriting dup2() on host. Signed-off-by: Al Viro --- fs/hostfs/hostfs_kern.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) (limited to 'fs/hostfs/hostfs_kern.c') diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 8130ce93a06a..dd1e55535a4e 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -302,27 +302,22 @@ int hostfs_readdir(struct file *file, void *ent, filldir_t filldir) int hostfs_file_open(struct inode *ino, struct file *file) { + static DEFINE_MUTEX(open_mutex); char *name; fmode_t mode = 0; + int err; int r = 0, w = 0, fd; mode = file->f_mode & (FMODE_READ | FMODE_WRITE); if ((mode & HOSTFS_I(ino)->mode) == mode) return 0; - /* - * The file may already have been opened, but with the wrong access, - * so this resets things and reopens the file with the new access. - */ - if (HOSTFS_I(ino)->fd != -1) { - close_file(&HOSTFS_I(ino)->fd); - HOSTFS_I(ino)->fd = -1; - } + mode |= HOSTFS_I(ino)->mode; - HOSTFS_I(ino)->mode |= mode; - if (HOSTFS_I(ino)->mode & FMODE_READ) +retry: + if (mode & FMODE_READ) r = 1; - if (HOSTFS_I(ino)->mode & FMODE_WRITE) + if (mode & FMODE_WRITE) w = 1; if (w) r = 1; @@ -335,7 +330,31 @@ int hostfs_file_open(struct inode *ino, struct file *file) __putname(name); if (fd < 0) return fd; - FILE_HOSTFS_I(file)->fd = fd; + + mutex_lock(&open_mutex); + /* somebody else had handled it first? */ + if ((mode & HOSTFS_I(ino)->mode) == mode) { + mutex_unlock(&open_mutex); + return 0; + } + if ((mode | HOSTFS_I(ino)->mode) != mode) { + mode |= HOSTFS_I(ino)->mode; + mutex_unlock(&open_mutex); + close_file(&fd); + goto retry; + } + if (HOSTFS_I(ino)->fd == -1) { + HOSTFS_I(ino)->fd = fd; + } else { + err = replace_file(fd, HOSTFS_I(ino)->fd); + close_file(&fd); + if (err < 0) { + mutex_unlock(&open_mutex); + return err; + } + } + HOSTFS_I(ino)->mode = mode; + mutex_unlock(&open_mutex); return 0; } -- cgit v1.2.2