From patchwork Wed Aug 8 08:29:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jose Pekkarinen X-Patchwork-Id: 954831 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41ll0k0DLBz9s3Z; Wed, 8 Aug 2018 18:29:10 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1fnJq2-0008Bg-MD; Wed, 08 Aug 2018 08:29:02 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1fnJq0-0008AU-KB for kernel-team@lists.canonical.com; Wed, 08 Aug 2018 08:29:00 +0000 Received: from 1.general.koalinux.uk.vpn ([10.172.192.202] helo=os1.maas) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1fnJq0-0005H6-9E; Wed, 08 Aug 2018 08:29:00 +0000 From: =?utf-8?q?Jos=C3=A9_Pekkarinen?= To: kernel-team@lists.canonical.com Subject: [X] [CVE-2015-1350] [PATCH] fs: Avoid premature clearing of capabilities Date: Wed, 8 Aug 2018 08:29:19 +0000 Message-Id: <1533716959-13802-1-git-send-email-jose.pekkarinen@canonical.com> X-Mailer: git-send-email 2.7.4 MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" CVE-2015-1350 Buglink: https://launchpad.net/bugs/1415636 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. Signed-off-by: José Pekkarinen Reviewed-by: Christoph Hellwig Signed-off-by: Jan Kara --- fs/attr.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/fs/attr.c b/fs/attr.c index df05bc1..870e4b6 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -68,7 +68,7 @@ int inode_change_ok(const struct inode *inode, 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) && !chown_ok(inode, attr->ia_uid)) @@ -95,6 +95,19 @@ int inode_change_ok(const struct inode *inode, struct iattr *attr) return -EPERM; } +kill_priv: + /* User has permission for the change */ + if (ia_valid & ATTR_KILL_PRIV) { + int error; + struct dentry * dentry = d_find_alias(inode); + + if (dentry) { + error = security_inode_killpriv(dentry); + if (error) + return error; + } + } + return 0; } EXPORT_SYMBOL(inode_change_ok); @@ -250,13 +263,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; } /*