Skip to content

Commit

Permalink
check ATTR_SIZE contraints in inode_change_ok
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Christoph Hellwig authored and Al Viro committed Aug 9, 2010
1 parent db78b87 commit 2c27c65
Show file tree
Hide file tree
Showing 21 changed files with 108 additions and 156 deletions.
5 changes: 1 addition & 4 deletions fs/adfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
44 changes: 30 additions & 14 deletions fs/attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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);

Expand Down Expand Up @@ -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.
Expand Down
18 changes: 14 additions & 4 deletions fs/ecryptfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 "
Expand Down
12 changes: 2 additions & 10 deletions fs/ext2/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
1 change: 0 additions & 1 deletion fs/fat/fat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 2 additions & 15 deletions fs/fat/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 1 addition & 5 deletions fs/fuse/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions fs/gfs2/aops.c
Original file line number Diff line number Diff line change
Expand Up @@ -702,12 +702,12 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
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:
Expand Down
6 changes: 2 additions & 4 deletions fs/gfs2/ops_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions fs/jffs2/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
51 changes: 2 additions & 49 deletions fs/libfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,49 +326,6 @@ int simple_rename(struct inode *old_dir, struct dentry *old_dentry,
return 0;
}

/**
* 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
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions fs/ocfs2/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -2308,12 +2308,12 @@ static ssize_t ocfs2_file_aio_write(struct kiocb *iocb,
* 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;
}
Expand Down
5 changes: 2 additions & 3 deletions fs/ramfs/file-nommu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/*****************************************************************************/
Expand Down
4 changes: 1 addition & 3 deletions fs/smbfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit 2c27c65

Please sign in to comment.