[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+5PVA4zWLBW7fqe3S4vXxqvO8CSvxZu3bXqRoq+fC_CuxNZ_Q@mail.gmail.com>
Date: Mon, 16 Feb 2015 14:50:28 -0500
From: Josh Boyer <jwboyer@...oraproject.org>
To: Ben Hutchings <ben@...adent.org.uk>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org,
linux-security-module <linux-security-module@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
770492@...s.debian.org, Ben Harris <bjh21@....ac.uk>, oss-security@...ts.openwall.com
Subject: Re: [RFC PATCH RESEND] vfs: Move security_inode_killpriv() after
permission checks
On Sat, Jan 17, 2015 at 6:26 PM, Ben Hutchings <ben@...adent.org.uk> wrote:
> chown() and write() should clear all privilege attributes on
> a file - setuid, setgid, setcap and any other extended
> privilege attributes.
>
> However, any attributes beyond setuid and setgid are managed by the
> LSM and not directly by the filesystem, so they cannot be set along
> with the other attributes.
>
> Currently we call security_inode_killpriv() in notify_change(),
> but in case of a chown() this is too early - we have not called
> inode_change_ok() or made any filesystem-specific permission/sanity
> checks.
>
> Add a new function setattr_killpriv() which calls
> security_inode_killpriv() if necessary, and change the setattr()
> implementation to call this in each filesystem that supports xattrs.
> This assumes that extended privilege attributes are always stored in
> xattrs.
>
> Compile-tested only.
>
> XXX This is a silent change to the VFS API, but we should probably
> change something so OOT filesystems fail to compile if they aren't
> updated to call setattr_killpriv().
>
> Reported-by: Ben Harris <bjh21@....ac.uk>
> References: https://bugs.debian.org/770492
This seems to have stalled. I don't see it in linux-next or anywhere
else I can find. The issue has a shiny CVE now, so it makes people
that follow those nervous. Is there any further feedback or follow-up
here?
josh
> ---
> drivers/staging/lustre/lustre/llite/llite_lib.c | 4 ++++
> fs/9p/vfs_inode.c | 4 ++++
> fs/9p/vfs_inode_dotl.c | 4 ++++
> fs/attr.c | 32 +++++++++++++++++++++----
> fs/btrfs/inode.c | 4 ++++
> fs/ceph/inode.c | 4 ++++
> fs/cifs/inode.c | 11 ++++++++-
> fs/ext2/inode.c | 4 ++++
> fs/ext3/inode.c | 4 ++++
> fs/ext4/inode.c | 4 ++++
> fs/f2fs/file.c | 4 ++++
> fs/fuse/dir.c | 15 +++++++-----
> fs/fuse/file.c | 3 ++-
> fs/fuse/fuse_i.h | 2 +-
> fs/gfs2/inode.c | 3 +++
> fs/hfs/inode.c | 4 ++++
> fs/hfsplus/inode.c | 4 ++++
> fs/jffs2/fs.c | 4 ++++
> fs/jfs/file.c | 4 ++++
> fs/kernfs/inode.c | 17 +++++++++++++
> fs/libfs.c | 3 +++
> fs/nfs/inode.c | 11 +++++++--
> fs/ocfs2/file.c | 6 ++++-
> fs/reiserfs/inode.c | 4 ++++
> fs/ubifs/file.c | 4 ++++
> fs/xfs/xfs_acl.c | 3 ++-
> fs/xfs/xfs_file.c | 2 +-
> fs/xfs/xfs_ioctl.c | 2 +-
> fs/xfs/xfs_iops.c | 16 ++++++++++---
> fs/xfs/xfs_iops.h | 10 ++++++--
> include/linux/fs.h | 1 +
> mm/shmem.c | 4 ++++
> 32 files changed, 176 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
> index a8bcc51..2a714b2 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_lib.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
> @@ -1434,6 +1434,10 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr, bool hsm_import)
> spin_unlock(&lli->lli_lock);
> }
>
> + rc = setattr_killpriv(dentry, attr);
> + if (rc)
> + return rc;
> +
> /* We always do an MDS RPC, even if we're only changing the size;
> * only the MDS knows whether truncate() should fail with -ETXTBUSY */
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 296482f..735cbf84 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -1130,6 +1130,10 @@ static int v9fs_vfs_setattr(struct dentry *dentry, struct iattr *iattr)
> if (S_ISREG(dentry->d_inode->i_mode))
> filemap_write_and_wait(dentry->d_inode->i_mapping);
>
> + retval = setattr_killpriv(dentry, iattr);
> + if (retval)
> + return retval;
> +
> retval = p9_client_wstat(fid, &wstat);
> if (retval < 0)
> return retval;
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 02b64f4..f3ca76d 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -583,6 +583,10 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
> if (S_ISREG(inode->i_mode))
> filemap_write_and_wait(inode->i_mapping);
>
> + retval = setattr_killpriv(dentry, iattr);
> + if (retval)
> + return retval;
> +
> retval = p9_client_setattr(fid, &p9attr);
> if (retval < 0)
> return retval;
> diff --git a/fs/attr.c b/fs/attr.c
> index 6530ced..184f3bf 100644
> --- a/fs/attr.c
> +++ b/fs/attr.c
> @@ -168,6 +168,28 @@ void setattr_copy(struct inode *inode, const struct iattr *attr)
> EXPORT_SYMBOL(setattr_copy);
>
> /**
> + * setattr_killpriv - remove extended privilege attributes from a file
> + * @dentry: Directory entry passed to the setattr operation
> + * @iattr: New attributes pased to the setattr operation
> + *
> + * All filesystems that can carry extended privilege attributes
> + * should call this from their setattr operation *after* validating
> + * the attribute changes.
> + *
> + * It does nothing if !(iattr->ia_valid & ATTR_KILL_PRIV), so
> + * it is not necessary to call it in that case.
> + */
> +int setattr_killpriv(struct dentry *dentry, struct iattr *iattr)
> +{
> + if (!(iattr->ia_valid & ATTR_KILL_PRIV))
> + return 0;
> +
> + iattr->ia_valid &= ~ATTR_KILL_PRIV;
> + return security_inode_killpriv(dentry);
> +}
> +EXPORT_SYMBOL(setattr_killpriv);
> +
> +/**
> * notify_change - modify attributes of a filesytem object
> * @dentry: object affected
> * @iattr: new attributes
> @@ -217,13 +239,13 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
> if (!(ia_valid & ATTR_MTIME_SET))
> attr->ia_mtime = now;
> if (ia_valid & ATTR_KILL_PRIV) {
> - attr->ia_valid &= ~ATTR_KILL_PRIV;
> - ia_valid &= ~ATTR_KILL_PRIV;
> error = security_inode_need_killpriv(dentry);
> - if (error > 0)
> - error = security_inode_killpriv(dentry);
> - if (error)
> + if (error < 0)
> return error;
> + if (error == 0) {
> + attr->ia_valid &= ~ATTR_KILL_PRIV;
> + ia_valid &= ~ATTR_KILL_PRIV;
> + }
> }
>
> /*
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d23362f..71e3fb8 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4697,6 +4697,10 @@ static int btrfs_setattr(struct dentry *dentry, struct iattr *attr)
> if (err)
> return err;
>
> + err = setattr_killpriv(dentry, attr);
> + if (err)
> + return err;
> +
> if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
> err = btrfs_setsize(inode, attr);
> if (err)
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 7b61390..9ba5556 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1712,6 +1712,10 @@ int ceph_setattr(struct dentry *dentry, struct iattr *attr)
> if (err != 0)
> return err;
>
> + err = setattr_killpriv(dentry, attr);
> + if (err != 0)
> + return err;
> +
> req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_SETATTR,
> USE_AUTH_MDS);
> if (IS_ERR(req))
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 197cb50..0e971f9 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -2149,7 +2149,10 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
> */
> rc = filemap_write_and_wait(inode->i_mapping);
> mapping_set_error(inode->i_mapping, rc);
> - rc = 0;
> +
> + rc = setattr_killpriv(direntry, attrs);
> + if (rc)
> + goto out;
>
> if (attrs->ia_valid & ATTR_SIZE) {
> rc = cifs_set_file_size(inode, attrs, xid, full_path);
> @@ -2273,6 +2276,12 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
> return rc;
> }
>
> + rc = setattr_killpriv(direntry, attrs);
> + if (rc) {
> + free_xid(xid);
> + return rc;
> + }
> +
> full_path = build_path_from_dentry(direntry);
> if (full_path == NULL) {
> rc = -ENOMEM;
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 36d35c3..9e245af 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1551,6 +1551,10 @@ int ext2_setattr(struct dentry *dentry, struct iattr *iattr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, iattr);
> + if (error)
> + return error;
> +
> if (is_quota_modification(inode, iattr))
> dquot_initialize(inode);
> if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) ||
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 2c6ccc4..ec4dffa 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -3248,6 +3248,10 @@ int ext3_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + return error;
> +
> if (is_quota_modification(inode, attr))
> dquot_initialize(inode);
> if ((ia_valid & ATTR_UID && !uid_eq(attr->ia_uid, inode->i_uid)) ||
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 3356ab5..80877a48 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4455,6 +4455,10 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + return error;
> +
> if (is_quota_modification(inode, attr))
> dquot_initialize(inode);
> if ((ia_valid & ATTR_UID && !uid_eq(attr->ia_uid, inode->i_uid)) ||
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 8e68bb6..c9371d2 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -560,6 +560,10 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
> if (err)
> return err;
>
> + err = setattr_killpriv(dentry, attr);
> + if (err)
> + return err;
> +
> if (attr->ia_valid & ATTR_SIZE) {
> err = f2fs_convert_inline_data(inode, attr->ia_size, NULL);
> if (err)
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index dbab798..f750848 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1693,9 +1693,10 @@ int fuse_flush_times(struct inode *inode, struct fuse_file *ff)
> * vmtruncate() doesn't allow for this case, so do the rlimit checking
> * and the actual truncation by hand.
> */
> -int fuse_do_setattr(struct inode *inode, struct iattr *attr,
> +int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
> struct file *file)
> {
> + struct inode *inode = entry->d_inode;
> struct fuse_conn *fc = get_fuse_conn(inode);
> struct fuse_inode *fi = get_fuse_inode(inode);
> struct fuse_req *req;
> @@ -1714,6 +1715,10 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
> if (err)
> return err;
>
> + err = setattr_killpriv(entry, attr);
> + if (err)
> + return err;
> +
> if (attr->ia_valid & ATTR_OPEN) {
> if (fc->atomic_o_trunc)
> return 0;
> @@ -1809,15 +1814,13 @@ error:
>
> static int fuse_setattr(struct dentry *entry, struct iattr *attr)
> {
> - struct inode *inode = entry->d_inode;
> -
> - if (!fuse_allow_current_process(get_fuse_conn(inode)))
> + if (!fuse_allow_current_process(get_fuse_conn(entry->d_inode)))
> return -EACCES;
>
> if (attr->ia_valid & ATTR_FILE)
> - return fuse_do_setattr(inode, attr, attr->ia_file);
> + return fuse_do_setattr(entry, attr, attr->ia_file);
> else
> - return fuse_do_setattr(inode, attr, NULL);
> + return fuse_do_setattr(entry, attr, NULL);
> }
>
> static int fuse_getattr(struct vfsmount *mnt, struct dentry *entry,
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index caa8d95..ffdc363 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2855,7 +2855,8 @@ static void fuse_do_truncate(struct file *file)
> attr.ia_file = file;
> attr.ia_valid |= ATTR_FILE;
>
> - fuse_do_setattr(inode, &attr, file);
> + /* XXX Is file->f_dentry->d_inode always the same as inode? */
> + fuse_do_setattr(file->f_dentry, &attr, file);
> }
>
> static inline loff_t fuse_round_up(loff_t off)
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index e8e47a6..163de1f 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -894,7 +894,7 @@ bool fuse_write_update_size(struct inode *inode, loff_t pos);
> int fuse_flush_times(struct inode *inode, struct fuse_file *ff);
> int fuse_write_inode(struct inode *inode, struct writeback_control *wbc);
>
> -int fuse_do_setattr(struct inode *inode, struct iattr *attr,
> +int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
> struct file *file);
>
> #endif /* _FS_FUSE_I_H */
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index c4ed823..b39d81a 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -1786,6 +1786,9 @@ static int gfs2_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> goto out;
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + goto out;
> if (attr->ia_valid & ATTR_SIZE)
> error = gfs2_setattr_size(inode, attr->ia_size);
> else if (attr->ia_valid & (ATTR_UID | ATTR_GID))
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index d0929bc..817f7a5 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -620,6 +620,10 @@ int hfs_inode_setattr(struct dentry *dentry, struct iattr * attr)
> return hsb->s_quiet ? 0 : error;
> }
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + return error;
> +
> if (attr->ia_valid & ATTR_MODE) {
> /* Only the 'w' bits can ever change and only all together. */
> if (attr->ia_mode & S_IWUSR)
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index 0cf786f..12549bc 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -251,6 +251,10 @@ static int hfsplus_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + return error;
> +
> if ((attr->ia_valid & ATTR_SIZE) &&
> attr->ia_size != i_size_read(inode)) {
> inode_dio_wait(inode);
> diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
> index 601afd1..b260789 100644
> --- a/fs/jffs2/fs.c
> +++ b/fs/jffs2/fs.c
> @@ -197,6 +197,10 @@ int jffs2_setattr(struct dentry *dentry, struct iattr *iattr)
> if (rc)
> return rc;
>
> + rc = setattr_killpriv(dentry, iattr);
> + if (rc)
> + return rc;
> +
> rc = jffs2_do_setattr(inode, iattr);
> if (!rc && (iattr->ia_valid & ATTR_MODE))
> rc = posix_acl_chmod(inode, inode->i_mode);
> diff --git a/fs/jfs/file.c b/fs/jfs/file.c
> index 33aa0cc..4008313 100644
> --- a/fs/jfs/file.c
> +++ b/fs/jfs/file.c
> @@ -107,6 +107,10 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr)
> if (rc)
> return rc;
>
> + rc = setattr_killpriv(dentry, iattr);
> + if (rc)
> + return rc;
> +
> if (is_quota_modification(inode, iattr))
> dquot_initialize(inode);
> if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) ||
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 9852176..6a70fc5 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -135,6 +135,23 @@ int kernfs_iop_setattr(struct dentry *dentry, struct iattr *iattr)
> if (error)
> goto out;
>
> + /*
> + * If we need to remove privileges, drop the mutex to do that
> + * first and then re-validate the remaining changes.
> + */
> + if (iattr->ia_valid & ATTR_KILL_PRIV) {
> + mutex_unlock(&kernfs_mutex);
> +
> + error = setattr_killpriv(dentry, iattr);
> + if (error)
> + return error;
> +
> + mutex_lock(&kernfs_mutex);
> + error = inode_change_ok(inode, iattr);
> + if (error)
> + goto out;
> + }
> +
> error = __kernfs_setattr(kn, iattr);
> if (error)
> goto out;
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 171d284..9a00049 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -375,6 +375,9 @@ int simple_setattr(struct dentry *dentry, struct iattr *iattr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, iattr);
> + if (error)
> + return error;
> if (iattr->ia_valid & ATTR_SIZE)
> truncate_setsize(inode, iattr->ia_size);
> setattr_copy(inode, iattr);
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 00689a8..94dd6ac 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -496,7 +496,7 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr)
> {
> struct inode *inode = dentry->d_inode;
> struct nfs_fattr *fattr;
> - int error = -ENOMEM;
> + int error;
>
> nfs_inc_stats(inode, NFSIOS_VFSSETATTR);
>
> @@ -524,9 +524,16 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr)
> nfs_wb_all(inode);
> }
>
> + /* XXX Can we assume the server's permission checks are sufficient? */
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + goto out;
> +
> fattr = nfs_alloc_fattr();
> - if (fattr == NULL)
> + if (fattr == NULL) {
> + error = -ENOMEM;
> goto out;
> + }
> /*
> * Return any delegations if we're going to change ACLs
> */
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 324dc93..ed93d74 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1139,7 +1139,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> attr->ia_valid &= ~ATTR_SIZE;
>
> #define OCFS2_VALID_ATTRS (ATTR_ATIME | ATTR_MTIME | ATTR_CTIME | ATTR_SIZE \
> - | ATTR_GID | ATTR_UID | ATTR_MODE)
> + | ATTR_GID | ATTR_UID | ATTR_MODE | ATTR_KILL_PRIV)
> if (!(attr->ia_valid & OCFS2_VALID_ATTRS))
> return 0;
>
> @@ -1147,6 +1147,10 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> if (status)
> return status;
>
> + status = setattr_killpriv(dentry, attr);
> + if (status)
> + return status;
> +
> if (is_quota_modification(inode, attr))
> dquot_initialize(inode);
> size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE;
> diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> index a7eec98..a458c12 100644
> --- a/fs/reiserfs/inode.c
> +++ b/fs/reiserfs/inode.c
> @@ -3316,6 +3316,10 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + return error;
> +
> /* must be turned off for recursive notify_change calls */
> ia_valid = attr->ia_valid &= ~(ATTR_KILL_SUID|ATTR_KILL_SGID);
>
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index b5b593c..73d2e87 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1269,6 +1269,10 @@ int ubifs_setattr(struct dentry *dentry, struct iattr *attr)
> if (err)
> return err;
>
> + err = setattr_killpriv(dentry, attr);
> + if (err)
> + return err;
> +
> if ((attr->ia_valid & ATTR_SIZE) && attr->ia_size < inode->i_size)
> /* Truncation to a smaller size */
> err = do_truncation(c, inode, attr);
> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
> index a65fa5d..22b7482 100644
> --- a/fs/xfs/xfs_acl.c
> +++ b/fs/xfs/xfs_acl.c
> @@ -244,7 +244,8 @@ xfs_set_mode(struct inode *inode, umode_t mode)
> iattr.ia_mode = mode;
> iattr.ia_ctime = current_fs_time(inode->i_sb);
>
> - error = xfs_setattr_nonsize(XFS_I(inode), &iattr, XFS_ATTR_NOACL);
> + error = xfs_setattr_nonsize(NULL, XFS_I(inode), &iattr,
> + XFS_ATTR_NOACL);
> }
>
> return error;
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index eb596b4..c9b9019 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -873,7 +873,7 @@ xfs_file_fallocate(
>
> iattr.ia_valid = ATTR_SIZE;
> iattr.ia_size = new_size;
> - error = xfs_setattr_size(ip, &iattr);
> + error = xfs_setattr_size(NULL, ip, &iattr);
> }
>
> out_unlock:
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 24c926b6..3e0dc56 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -714,7 +714,7 @@ xfs_ioc_space(
> iattr.ia_valid = ATTR_SIZE;
> iattr.ia_size = bf->l_start;
>
> - error = xfs_setattr_size(ip, &iattr);
> + error = xfs_setattr_size(NULL, ip, &iattr);
> if (!error)
> clrprealloc = true;
> break;
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index ec6dcdc..669150b 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -527,6 +527,7 @@ xfs_setattr_time(
>
> int
> xfs_setattr_nonsize(
> + struct dentry *dentry,
> struct xfs_inode *ip,
> struct iattr *iattr,
> int flags)
> @@ -554,6 +555,10 @@ xfs_setattr_nonsize(
> error = inode_change_ok(inode, iattr);
> if (error)
> return error;
> +
> + error = setattr_killpriv(dentry, iattr);
> + if (error)
> + return error;
> }
>
> ASSERT((mask & ATTR_SIZE) == 0);
> @@ -734,6 +739,7 @@ out_dqrele:
> */
> int
> xfs_setattr_size(
> + struct dentry *dentry,
> struct xfs_inode *ip,
> struct iattr *iattr)
> {
> @@ -776,9 +782,13 @@ xfs_setattr_size(
> * Use the regular setattr path to update the timestamps.
> */
> iattr->ia_valid &= ~ATTR_SIZE;
> - return xfs_setattr_nonsize(ip, iattr, 0);
> + return xfs_setattr_nonsize(dentry, ip, iattr, 0);
> }
>
> + error = setattr_killpriv(dentry, iattr);
> + if (error)
> + return error;
> +
> /*
> * Make sure that the dquots are attached to the inode.
> */
> @@ -974,10 +984,10 @@ xfs_vn_setattr(
>
> if (iattr->ia_valid & ATTR_SIZE) {
> xfs_ilock(ip, XFS_IOLOCK_EXCL);
> - error = xfs_setattr_size(ip, iattr);
> + error = xfs_setattr_size(dentry, ip, iattr);
> xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> } else {
> - error = xfs_setattr_nonsize(ip, iattr, 0);
> + error = xfs_setattr_nonsize(dentry, ip, iattr, 0);
> }
>
> return error;
> diff --git a/fs/xfs/xfs_iops.h b/fs/xfs/xfs_iops.h
> index 1c34e43..6994d3e 100644
> --- a/fs/xfs/xfs_iops.h
> +++ b/fs/xfs/xfs_iops.h
> @@ -32,8 +32,14 @@ extern void xfs_setup_inode(struct xfs_inode *);
> */
> #define XFS_ATTR_NOACL 0x01 /* Don't call posix_acl_chmod */
>
> -extern int xfs_setattr_nonsize(struct xfs_inode *ip, struct iattr *vap,
> +/*
> + * XXX Several callers have to pass dentry = NULL and this should
> + * work but it's really ugly.
> + */
> +extern int xfs_setattr_nonsize(struct dentry *dentry,
> + struct xfs_inode *ip, struct iattr *vap,
> int flags);
> -extern int xfs_setattr_size(struct xfs_inode *ip, struct iattr *vap);
> +extern int xfs_setattr_size(struct dentry *dentry,
> + struct xfs_inode *ip, struct iattr *vap);
>
> #endif /* __XFS_IOPS_H__ */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9ab779e..7cad5d1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2663,6 +2663,7 @@ extern int buffer_migrate_page(struct address_space *,
> extern int inode_change_ok(const struct inode *, struct iattr *);
> extern int inode_newsize_ok(const struct inode *, loff_t offset);
> extern void setattr_copy(struct inode *inode, const struct iattr *attr);
> +extern int setattr_killpriv(struct dentry *dentry, struct iattr *attr);
>
> extern int file_update_time(struct file *file);
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 185836b..d1d4b9b 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -557,6 +557,10 @@ static int shmem_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> + error = setattr_killpriv(dentry, attr);
> + if (error)
> + return error;
> +
> if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
> loff_t oldsize = inode->i_size;
> loff_t newsize = attr->ia_size;
>
>
> --
> Ben Hutchings
> The first rule of tautology club is the first rule of tautology club.
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
