summaryrefslogtreecommitdiff
path: root/fs
diff options
context:
space:
mode:
authorChristoph Hellwig <hch@lst.de>2010-06-04 11:30:04 +0200
committerAl Viro <viro@zeniv.linux.org.uk>2010-08-09 16:47:39 -0400
commit2c27c65ed0696f0b5df2dad2cf6462d72164d547 (patch)
tree7d9036e3dea98938f7fd7074366ee73929e9b2e5 /fs
parentdb78b877f7744bec4a9d9f9e7d10da3931d7cd39 (diff)
check ATTR_SIZE contraints in inode_change_ok
Make sure we check the truncate constraints early on in ->setattr by adding those checks to inode_change_ok. Also clean up and document inode_change_ok to make this obvious. As a fallout we don't have to call inode_newsize_ok from simple_setsize and simplify it down to a truncate_setsize which doesn't return an error. This simplifies a lot of setattr implementations and means we use truncate_setsize almost everywhere. Get rid of fat_setsize now that it's trivial and mark ext2_setsize static to make the calling convention obvious. Keep the inode_newsize_ok in vmtruncate for now as all callers need an audit for its removal anyway. Note: setattr code in ecryptfs doesn't call inode_change_ok at all and needs a deeper audit, but that is left for later. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'fs')
-rw-r--r--fs/adfs/inode.c5
-rw-r--r--fs/attr.c44
-rw-r--r--fs/ecryptfs/inode.c18
-rw-r--r--fs/ext2/inode.c12
-rw-r--r--fs/fat/fat.h1
-rw-r--r--fs/fat/file.c17
-rw-r--r--fs/fuse/dir.c6
-rw-r--r--fs/gfs2/aops.c4
-rw-r--r--fs/gfs2/ops_inode.c6
-rw-r--r--fs/jffs2/fs.c4
-rw-r--r--fs/libfs.c51
-rw-r--r--fs/ocfs2/file.c6
-rw-r--r--fs/ramfs/file-nommu.c5
-rw-r--r--fs/smbfs/inode.c4
-rw-r--r--fs/ubifs/file.c23
-rw-r--r--fs/ubifs/ubifs.h2
-rw-r--r--fs/ufs/truncate.c11
17 files changed, 76 insertions, 143 deletions
diff --git a/fs/adfs/inode.c b/fs/adfs/inode.c
index b3dec193036..65794b8fe79 100644
--- a/fs/adfs/inode.c
+++ b/fs/adfs/inode.c
@@ -333,10 +333,7 @@ adfs_notify_change(struct dentry *dentry, struct iattr *attr)
/* XXX: this is missing some actual on-disk truncation.. */
if (ia_valid & ATTR_SIZE)
- error = simple_setsize(inode, attr->ia_size);
-
- if (error)
- goto out;
+ truncate_setsize(inode, attr->ia_size);
if (ia_valid & ATTR_MTIME) {
inode->i_mtime = attr->ia_mtime;
diff --git a/fs/attr.c b/fs/attr.c
index ed44d8ae8bf..7ca41811afa 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -14,35 +14,53 @@
#include <linux/fcntl.h>
#include <linux/security.h>
-/* Taken over from the old code... */
-
-/* POSIX UID/GID verification for setting inode attributes. */
+/**
+ * inode_change_ok - check if attribute changes to an inode are allowed
+ * @inode: inode to check
+ * @attr: attributes to change
+ *
+ * Check if we are allowed to change the attributes contained in @attr
+ * in the given inode. This includes the normal unix access permission
+ * checks, as well as checks for rlimits and others.
+ *
+ * Should be called as the first thing in ->setattr implementations,
+ * possibly after taking additional locks.
+ */
int inode_change_ok(const struct inode *inode, struct iattr *attr)
{
- int retval = -EPERM;
unsigned int ia_valid = attr->ia_valid;
+ /*
+ * First check size constraints. These can't be overriden using
+ * ATTR_FORCE.
+ */
+ if (ia_valid & ATTR_SIZE) {
+ int error = inode_newsize_ok(inode, attr->ia_size);
+ if (error)
+ return error;
+ }
+
/* If force is set do it anyway. */
if (ia_valid & ATTR_FORCE)
- goto fine;
+ return 0;
/* Make sure a caller can chown. */
if ((ia_valid & ATTR_UID) &&
(current_fsuid() != inode->i_uid ||
attr->ia_uid != inode->i_uid) && !capable(CAP_CHOWN))
- goto error;
+ return -EPERM;
/* Make sure caller can chgrp. */
if ((ia_valid & ATTR_GID) &&
(current_fsuid() != inode->i_uid ||
(!in_group_p(attr->ia_gid) && attr->ia_gid != inode->i_gid)) &&
!capable(CAP_CHOWN))
- goto error;
+ return -EPERM;
/* Make sure a caller can chmod. */
if (ia_valid & ATTR_MODE) {
if (!is_owner_or_cap(inode))
- goto error;
+ return -EPERM;
/* Also check the setgid bit! */
if (!in_group_p((ia_valid & ATTR_GID) ? attr->ia_gid :
inode->i_gid) && !capable(CAP_FSETID))
@@ -52,12 +70,10 @@ int inode_change_ok(const struct inode *inode, struct iattr *attr)
/* Check for setting the inode time. */
if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
if (!is_owner_or_cap(inode))
- goto error;
+ return -EPERM;
}
-fine:
- retval = 0;
-error:
- return retval;
+
+ return 0;
}
EXPORT_SYMBOL(inode_change_ok);
@@ -113,7 +129,7 @@ EXPORT_SYMBOL(inode_newsize_ok);
*
* setattr_copy updates the inode's metadata with that specified
* in attr. Noticably missing is inode size update, which is more complex
- * as it requires pagecache updates. See simple_setsize.
+ * as it requires pagecache updates.
*
* The inode is not marked as dirty after this operation. The rationale is
* that for "simple" filesystems, the struct inode is the inode storage.
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 31ef5252f0f..82900b063b1 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -804,10 +804,20 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia,
size_t num_zeros = (PAGE_CACHE_SIZE
- (ia->ia_size & ~PAGE_CACHE_MASK));
+
+ /*
+ * XXX(truncate) this should really happen at the begginning
+ * of ->setattr. But the code is too messy to that as part
+ * of a larger patch. ecryptfs is also totally missing out
+ * on the inode_change_ok check at the beginning of
+ * ->setattr while would include this.
+ */
+ rc = inode_newsize_ok(inode, ia->ia_size);
+ if (rc)
+ goto out;
+
if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
- rc = simple_setsize(inode, ia->ia_size);
- if (rc)
- goto out;
+ truncate_setsize(inode, ia->ia_size);
lower_ia->ia_size = ia->ia_size;
lower_ia->ia_valid |= ATTR_SIZE;
goto out;
@@ -830,7 +840,7 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia,
goto out;
}
}
- simple_setsize(inode, ia->ia_size);
+ truncate_setsize(inode, ia->ia_size);
rc = ecryptfs_write_inode_size_to_metadata(inode);
if (rc) {
printk(KERN_ERR "Problem with "
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 7dee7b3f368..069620b30d4 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1156,15 +1156,10 @@ static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
__ext2_truncate_blocks(inode, offset);
}
-int ext2_setsize(struct inode *inode, loff_t newsize)
+static int ext2_setsize(struct inode *inode, loff_t newsize)
{
- loff_t oldsize;
int error;
- error = inode_newsize_ok(inode, newsize);
- if (error)
- return error;
-
if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
S_ISLNK(inode->i_mode)))
return -EINVAL;
@@ -1184,10 +1179,7 @@ int ext2_setsize(struct inode *inode, loff_t newsize)
if (error)
return error;
- oldsize = inode->i_size;
- i_size_write(inode, newsize);
- truncate_pagecache(inode, oldsize, newsize);
-
+ truncate_setsize(inode, newsize);
__ext2_truncate_blocks(inode, newsize);
inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 27ac2572595..d75a77f85c2 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -306,7 +306,6 @@ extern long fat_generic_ioctl(struct file *filp, unsigned int cmd,
extern const struct file_operations fat_file_operations;
extern const struct inode_operations fat_file_inode_operations;
extern int fat_setattr(struct dentry * dentry, struct iattr * attr);
-extern int fat_setsize(struct inode *inode, loff_t offset);
extern void fat_truncate_blocks(struct inode *inode, loff_t offset);
extern int fat_getattr(struct vfsmount *mnt, struct dentry *dentry,
struct kstat *stat);
diff --git a/fs/fat/file.c b/fs/fat/file.c
index b2eedcee751..7257752b6d5 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -364,18 +364,6 @@ static int fat_allow_set_time(struct msdos_sb_info *sbi, struct inode *inode)
return 0;
}
-int fat_setsize(struct inode *inode, loff_t offset)
-{
- int error;
-
- error = simple_setsize(inode, offset);
- if (error)
- return error;
- fat_truncate_blocks(inode, offset);
-
- return error;
-}
-
#define TIMES_SET_FLAGS (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)
/* valid file mode bits */
#define FAT_VALID_MODE (S_IFREG | S_IFDIR | S_IRWXUGO)
@@ -441,9 +429,8 @@ int fat_setattr(struct dentry *dentry, struct iattr *attr)
}
if (attr->ia_valid & ATTR_SIZE) {
- error = fat_setsize(inode, attr->ia_size);
- if (error)
- goto out;
+ truncate_setsize(inode, attr->ia_size);
+ fat_truncate_blocks(inode, attr->ia_size);
}
setattr_copy(inode, attr);
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 43a9b3730a9..3978a42d4f0 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1280,12 +1280,8 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
if ((attr->ia_valid & ATTR_OPEN) && fc->atomic_o_trunc)
return 0;
- if (attr->ia_valid & ATTR_SIZE) {
- err = inode_newsize_ok(inode, attr->ia_size);
- if (err)
- return err;
+ if (attr->ia_valid & ATTR_SIZE)
is_truncate = true;
- }
req = fuse_get_req(fc);
if (IS_ERR(req))
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 703000d6e4d..54fe087bf54 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -702,12 +702,12 @@ out:
page_cache_release(page);
/*
- * XXX(hch): the call below should probably be replaced with
+ * XXX(truncate): the call below should probably be replaced with
* a call to the gfs2-specific truncate blocks helper to actually
* release disk blocks..
*/
if (pos + len > ip->i_inode.i_size)
- simple_setsize(&ip->i_inode, ip->i_inode.i_size);
+ truncate_setsize(&ip->i_inode, ip->i_inode.i_size);
out_endtrans:
gfs2_trans_end(sdp);
out_trans_fail:
diff --git a/fs/gfs2/ops_inode.c b/fs/gfs2/ops_inode.c
index d7d410a4ca4..1009be2c973 100644
--- a/fs/gfs2/ops_inode.c
+++ b/fs/gfs2/ops_inode.c
@@ -1072,7 +1072,7 @@ int gfs2_permission(struct inode *inode, int mask)
}
/*
- * XXX: should be changed to have proper ordering by opencoding simple_setsize
+ * XXX(truncate): the truncate_setsize calls should be moved to the end.
*/
static int setattr_size(struct inode *inode, struct iattr *attr)
{
@@ -1084,10 +1084,8 @@ static int setattr_size(struct inode *inode, struct iattr *attr)
error = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
if (error)
return error;
- error = simple_setsize(inode, attr->ia_size);
+ truncate_setsize(inode, attr->ia_size);
gfs2_trans_end(sdp);
- if (error)
- return error;
}
error = gfs2_truncatei(ip, attr->ia_size);
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 459d39d1ea0..1b2426604fe 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -169,13 +169,13 @@ int jffs2_do_setattr (struct inode *inode, struct iattr *iattr)
mutex_unlock(&f->sem);
jffs2_complete_reservation(c);
- /* We have to do the simple_setsize() without f->sem held, since
+ /* We have to do the truncate_setsize() without f->sem held, since
some pages may be locked and waiting for it in readpage().
We are protected from a simultaneous write() extending i_size
back past iattr->ia_size, because do_truncate() holds the
generic inode semaphore. */
if (ivalid & ATTR_SIZE && inode->i_size > iattr->ia_size) {
- simple_setsize(inode, iattr->ia_size);
+ truncate_setsize(inode, iattr->ia_size);
inode->i_blocks = (inode->i_size + 511) >> 9;
}
diff --git a/fs/libfs.c b/fs/libfs.c
index 40562224b71..0a9da95317f 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -327,49 +327,6 @@ int simple_rename(struct inode *old_dir, struct dentry *old_dentry,
}
/**
- * simple_setsize - handle core mm and vfs requirements for file size change
- * @inode: inode
- * @newsize: new file size
- *
- * Returns 0 on success, -error on failure.
- *
- * simple_setsize must be called with inode_mutex held.
- *
- * simple_setsize will check that the requested new size is OK (see
- * inode_newsize_ok), and then will perform the necessary i_size update
- * and pagecache truncation (if necessary). It will be typically be called
- * from the filesystem's setattr function when ATTR_SIZE is passed in.
- *
- * The inode itself must have correct permissions and attributes to allow
- * i_size to be changed, this function then just checks that the new size
- * requested is valid.
- *
- * In the case of simple in-memory filesystems with inodes stored solely
- * in the inode cache, and file data in the pagecache, nothing more needs
- * to be done to satisfy a truncate request. Filesystems with on-disk
- * blocks for example will need to free them in the case of truncate, in
- * that case it may be easier not to use simple_setsize (but each of its
- * components will likely be required at some point to update pagecache
- * and inode etc).
- */
-int simple_setsize(struct inode *inode, loff_t newsize)
-{
- loff_t oldsize;
- int error;
-
- error = inode_newsize_ok(inode, newsize);
- if (error)
- return error;
-
- oldsize = inode->i_size;
- i_size_write(inode, newsize);
- truncate_pagecache(inode, oldsize, newsize);
-
- return error;
-}
-EXPORT_SYMBOL(simple_setsize);
-
-/**
* simple_setattr - setattr for simple filesystem
* @dentry: dentry
* @iattr: iattr structure
@@ -394,12 +351,8 @@ int simple_setattr(struct dentry *dentry, struct iattr *iattr)
if (error)
return error;
- if (iattr->ia_valid & ATTR_SIZE) {
- error = simple_setsize(inode, iattr->ia_size);
- if (error)
- return error;
- }
-
+ if (iattr->ia_valid & ATTR_SIZE)
+ truncate_setsize(inode, iattr->ia_size);
setattr_copy(inode, iattr);
mark_inode_dirty(inode);
return 0;
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 584cf8ac167..81296b4e364 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1233,7 +1233,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
}
/*
- * This will intentionally not wind up calling simple_setsize(),
+ * This will intentionally not wind up calling truncate_setsize(),
* since all the work for a size change has been done above.
* Otherwise, we could get into problems with truncate as
* ip_alloc_sem is used there to protect against i_size
@@ -2308,12 +2308,12 @@ relock:
* blocks outside i_size. Trim these off again.
* Don't need i_size_read because we hold i_mutex.
*
- * XXX(hch): this looks buggy because ocfs2 did not
+ * XXX(truncate): this looks buggy because ocfs2 did not
* actually implement ->truncate. Take a look at
* the new truncate sequence and update this accordingly
*/
if (*ppos + count > inode->i_size)
- simple_setsize(inode, inode->i_size);
+ truncate_setsize(inode, inode->i_size);
ret = written;
goto out_dio;
}
diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index 8d44f0347b2..9eead2c796b 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -146,9 +146,8 @@ static int ramfs_nommu_resize(struct inode *inode, loff_t newsize, loff_t size)
return ret;
}
- ret = simple_setsize(inode, newsize);
-
- return ret;
+ truncate_setsize(inode, newsize);
+ return 0;
}
/*****************************************************************************/
diff --git a/fs/smbfs/inode.c b/fs/smbfs/inode.c
index 9551cb6f7fe..e338f0a5a70 100644
--- a/fs/smbfs/inode.c
+++ b/fs/smbfs/inode.c
@@ -714,9 +714,7 @@ smb_notify_change(struct dentry *dentry, struct iattr *attr)
error = server->ops->truncate(inode, attr->ia_size);
if (error)
goto out;
- error = simple_setsize(inode, attr->ia_size);
- if (error)
- goto out;
+ truncate_setsize(inode, attr->ia_size);
refresh = 1;
}
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 12f445cee9f..03ae894c45d 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -967,14 +967,15 @@ static int do_writepage(struct page *page, int len)
* the page locked, and it locks @ui_mutex. However, write-back does take inode
* @i_mutex, which means other VFS operations may be run on this inode at the
* same time. And the problematic one is truncation to smaller size, from where
- * we have to call 'simple_setsize()', which first changes @inode->i_size, then
+ * we have to call 'truncate_setsize()', which first changes @inode->i_size, then
* drops the truncated pages. And while dropping the pages, it takes the page
- * lock. This means that 'do_truncation()' cannot call 'simple_setsize()' with
+ * lock. This means that 'do_truncation()' cannot call 'truncate_setsize()' with
* @ui_mutex locked, because it would deadlock with 'ubifs_writepage()'. This
* means that @inode->i_size is changed while @ui_mutex is unlocked.
*
- * XXX: with the new truncate the above is not true anymore, the simple_setsize
- * calls can be replaced with the individual components.
+ * XXX(truncate): with the new truncate sequence this is not true anymore,
+ * and the calls to truncate_setsize can be move around freely. They should
+ * be moved to the very end of the truncate sequence.
*
* But in 'ubifs_writepage()' we have to guarantee that we do not write beyond
* inode size. How do we do this if @inode->i_size may became smaller while we
@@ -1128,9 +1129,7 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
budgeted = 0;
}
- err = simple_setsize(inode, new_size);
- if (err)
- goto out_budg;
+ truncate_setsize(inode, new_size);
if (offset) {
pgoff_t index = new_size >> PAGE_CACHE_SHIFT;
@@ -1217,16 +1216,14 @@ static int do_setattr(struct ubifs_info *c, struct inode *inode,
if (attr->ia_valid & ATTR_SIZE) {
dbg_gen("size %lld -> %lld", inode->i_size, new_size);
- err = simple_setsize(inode, new_size);
- if (err)
- goto out;
+ truncate_setsize(inode, new_size);
}
mutex_lock(&ui->ui_mutex);
if (attr->ia_valid & ATTR_SIZE) {
/* Truncation changes inode [mc]time */
inode->i_mtime = inode->i_ctime = ubifs_current_time(inode);
- /* 'simple_setsize()' changed @i_size, update @ui_size */
+ /* 'truncate_setsize()' changed @i_size, update @ui_size */
ui->ui_size = inode->i_size;
}
@@ -1248,10 +1245,6 @@ static int do_setattr(struct ubifs_info *c, struct inode *inode,
if (IS_SYNC(inode))
err = inode->i_sb->s_op->write_inode(inode, NULL);
return err;
-
-out:
- ubifs_release_budget(c, &req);
- return err;
}
int ubifs_setattr(struct dentry *dentry, struct iattr *attr)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 04310878f44..0c9876b396d 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -379,7 +379,7 @@ struct ubifs_gced_idx_leb {
* The @ui_size is a "shadow" variable for @inode->i_size and UBIFS uses
* @ui_size instead of @inode->i_size. The reason for this is that UBIFS cannot
* make sure @inode->i_size is always changed under @ui_mutex, because it
- * cannot call 'simple_setsize()' with @ui_mutex locked, because it would deadlock
+ * cannot call 'truncate_setsize()' with @ui_mutex locked, because it would deadlock
* with 'ubifs_writepage()' (see file.c). All the other inode fields are
* changed under @ui_mutex, so they do not need "shadow" fields. Note, one
* could consider to rework locking and base it on "shadow" fields.
diff --git a/fs/ufs/truncate.c b/fs/ufs/truncate.c
index 085e11623b7..34d5cb13532 100644
--- a/fs/ufs/truncate.c
+++ b/fs/ufs/truncate.c
@@ -500,11 +500,6 @@ out:
return err;
}
-/*
- * TODO:
- * - truncate case should use proper ordering instead of using
- * simple_setsize
- */
int ufs_setattr(struct dentry *dentry, struct iattr *attr)
{
struct inode *inode = dentry->d_inode;
@@ -518,9 +513,9 @@ int ufs_setattr(struct dentry *dentry, struct iattr *attr)
if (ia_valid & ATTR_SIZE && attr->ia_size != inode->i_size) {
loff_t old_i_size = inode->i_size;
- error = simple_setsize(inode, attr->ia_size);
- if (error)
- return error;
+ /* XXX(truncate): truncate_setsize should be called last */
+ truncate_setsize(inode, attr->ia_size);
+
error = ufs_truncate(inode, old_i_size);
if (error)
return error;