Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ