From patchwork Thu Apr 15 23:23:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1466793 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FLwQR39ZJz9sW4; Fri, 16 Apr 2021 09:23:59 +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 1lXBL2-0003v5-Fq; Thu, 15 Apr 2021 23:23:56 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lXBL1-0003uI-0a for kernel-team@lists.ubuntu.com; Thu, 15 Apr 2021 23:23:55 +0000 Received: from [177.103.101.177] (helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lXBL0-0001gp-8F for kernel-team@lists.ubuntu.com; Thu, 15 Apr 2021 23:23:54 +0000 From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU Xenial] UBUNTU: SAUCE: vfs_setxattr: free converted value if xattr_permission returns error Date: Thu, 15 Apr 2021 20:23:36 -0300 Message-Id: <20210415232336.199462-3-cascardo@canonical.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20210415232336.199462-1-cascardo@canonical.com> References: <20210415232336.199462-1-cascardo@canonical.com> 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" BugLink: https://bugs.launchpad.net/bugs/1924611 The backport of commit 7c03e2cda4a5 ("vfs: move cap_convert_nscap() call into vfs_setxattr()") did not consider that vfs_setxattr had other exit paths that would require a converted value to be freed. If xattr_permission returns a failure, it would cause a memory leak. In the case of security.capability attribute, which is the only that can allocate a new value, xattr_permission will return a failure in case of HAS_UNMAPPED_ID(inode), which would already be caught by cap_convert_nscap, at !capable_wrt_inode_uidgid(inode, CAP_SETFCAP). However, if the file IS_IMMUTABLE or IS_APPEND, the failure will be returned and the leak will happen. Though setting a file as immutable or append is restricted to CAP_FILE_IMMUTABLE, the leak was still shown to happen when trying to setcap on an immutable file after doing a mount unshare. Signed-off-by: Thadeu Lima de Souza Cascardo Reviewed-by: Krzysztof Kozlowski --- fs/xattr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/xattr.c b/fs/xattr.c index 6f38106d08dc..53086977db5c 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -142,7 +142,7 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value, error = xattr_permission(inode, name, MAY_WRITE); if (error) - return error; + goto out_free; mutex_lock(&inode->i_mutex); error = security_inode_setxattr(dentry, name, value, size, flags); @@ -153,6 +153,7 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value, out: mutex_unlock(&inode->i_mutex); +out_free: if (value != orig_value) kfree(value);