From patchwork Mon Sep 19 15:30:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 9339715 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B64C8607D0 for ; Mon, 19 Sep 2016 15:31:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A81E0287F4 for ; Mon, 19 Sep 2016 15:31:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9CBB5293E7; Mon, 19 Sep 2016 15:31:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 12415287F4 for ; Mon, 19 Sep 2016 15:31:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752075AbcISPbR (ORCPT ); Mon, 19 Sep 2016 11:31:17 -0400 Received: from mx2.suse.de ([195.135.220.15]:34529 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751207AbcISPbQ (ORCPT ); Mon, 19 Sep 2016 11:31:16 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 69FCFACC5; Mon, 19 Sep 2016 15:31:14 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 46AF41E1132; Mon, 19 Sep 2016 17:31:12 +0200 (CEST) From: Jan Kara To: Al Viro Cc: Andrew Morton , linux-fsdevel@vger.kernel.org, xfs@oss.sgi.org, Dave Chinner , Ilya Dryomov , Sage Weil , "Yan, Zheng" , ceph-devel@vger.kernel.org, Miklos Szeredi , Jan Kara Subject: [PATCH 5/5] fs: Avoid premature clearing of capabilities Date: Mon, 19 Sep 2016 17:30:59 +0200 Message-Id: <1474299059-14806-6-git-send-email-jack@suse.cz> X-Mailer: git-send-email 2.6.6 In-Reply-To: <1474299059-14806-1-git-send-email-jack@suse.cz> References: <1474299059-14806-1-git-send-email-jack@suse.cz> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Jan Kara --- fs/attr.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/fs/attr.c b/fs/attr.c index 5c45909ea204..83c8430d908f 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -47,7 +47,7 @@ int setattr_prepare(struct dentry *dentry, struct iattr *attr) /* 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 *dentry, struct iattr *attr) 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, 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) + ia_valid = attr->ia_valid &= ~ATTR_KILL_PRIV; } /*