lkml.org 
[lkml]   [2016]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 337/346] fs: Avoid premature clearing of capabilities
    3.16.39-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Jan Kara <jack@suse.cz>

    commit 030b533c4fd4d2ec3402363323de4bb2983c9cee upstream.

    Currently, notify_change() clears capabilities or IMA attributes by
    calling security_inode_killpriv() before calling into ->setattr. Thus it
    happens before any other permission checks in inode_change_ok() and user
    is thus allowed to trigger clearing of capabilities or IMA attributes
    for any file he can look up e.g. by calling chown for that file. This is
    unexpected and can lead to user DoSing a system.

    Fix the problem by calling security_inode_killpriv() at the end of
    inode_change_ok() instead of from notify_change(). At that moment we are
    sure user has permissions to do the requested change.

    References: CVE-2015-1350
    Reviewed-by: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Jan Kara <jack@suse.cz>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    fs/attr.c | 20 ++++++++++++++------
    1 file changed, 14 insertions(+), 6 deletions(-)

    --- a/fs/attr.c
    +++ b/fs/attr.c
    @@ -47,7 +47,7 @@ int setattr_prepare(struct dentry *dentr

    /* If force is set do it anyway. */
    if (ia_valid & ATTR_FORCE)
    - return 0;
    + goto kill_priv;

    /* Make sure a caller can chown. */
    if ((ia_valid & ATTR_UID) &&
    @@ -80,6 +80,16 @@ int setattr_prepare(struct dentry *dentr
    return -EPERM;
    }

    +kill_priv:
    + /* User has permission for the change */
    + if (ia_valid & ATTR_KILL_PRIV) {
    + int error;
    +
    + error = security_inode_killpriv(dentry);
    + if (error)
    + return error;
    + }
    +
    return 0;
    }
    EXPORT_SYMBOL(setattr_prepare);
    @@ -220,13 +230,11 @@ int notify_change(struct dentry * dentry
    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)
    + ia_valid = attr->ia_valid &= ~ATTR_KILL_PRIV;
    }

    /*
    \
     
     \ /
      Last update: 2016-11-14 03:49    [W:4.737 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site