From patchwork Wed Jul 31 16:57:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mark Salyzyn X-Patchwork-Id: 1139918 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.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=reject dis=none) header.from=android.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="QLM8sYrE"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=android.com header.i=@android.com header.b="fmiQSh9y"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 45zKS06yhfz9sML for ; Thu, 1 Aug 2019 03:00:32 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HM+N/vPMGzqD89SKpdo8LWLiSN1llxYKL4/g9Ff9sCI=; b=QLM8sYrEg3Z9zE rwQpnalShRAh5zjESuhyzSVA6Ojw8SN8R/ksA18TZPnXd3LzUuWjv9cArF6tyLONsYLF7AinM5fMt /KnMxo/j8VGSLRRJcmbKmgMeWFg8lm7GM9hwuEu1INbJ/hOqPbvj0OBBZ0jTH4bFZIh5fL09CMNBf wbYPcOAPNAfNGt6cXB3tkzUJYMQJi3FiIiITBKd9grDcqMyredVvL0t5p/F7Ez3rMQpf2by/puwRZ R48R4X30rwvpTTolWnX1iqM8EAYP1xGZGuohEVqeyYkhVvmtra+7o/lU53DaRGb9vSl3CUe/4UfsF jW8hfedsu/kZlFYfSX1A==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hsrxk-00028B-1n; Wed, 31 Jul 2019 17:00:28 +0000 Received: from mail-pl1-x641.google.com ([2607:f8b0:4864:20::641]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1hsrwF-0007w1-1t for linux-mtd@lists.infradead.org; Wed, 31 Jul 2019 16:59:12 +0000 Received: by mail-pl1-x641.google.com with SMTP id ay6so30791829plb.9 for ; Wed, 31 Jul 2019 09:58:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=android.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=p6RSnHZej7dMZQfosYc1rQOAXMnG8HF3vzAcSS3MG98=; b=fmiQSh9yv929qF/IP6Uym2pEsxsKnDznkX0VjJ8cIh+7NzZrkbqvPCG3bssMygeAbK NmkNsix4uTqi91gMmHKksLpNQD7b0D1A9bda6aSayMw9sPfvhUAJO63BySMzz39Ednc6 +iWQAfEEJ8NQ6QfBslJvC3DyBQUS6s3R1R0XZUZMZgKOLtN+EIac6ejHjwc9qbdb5uvz eiCuyG0A6yFWZLw5lHrPP3O5DsRBGnqxHkq8vNdP6QJnUoh2+fIqC3yhPbsH+qEpXwib mUbx7N0k3Aa9YqKq/FSCQYABcvQoWcmd3n7QaLFSg3+9u/w9YuhZ+4mMF/9YdnWbfIFZ JhFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=p6RSnHZej7dMZQfosYc1rQOAXMnG8HF3vzAcSS3MG98=; b=btZTjQPXf9SV87iSaaCkfBnjvAnne25S53vMVpN0J+1GO5L1Co7PjI53m0E4Ol3jYO YMyiWLobSZndGvYbJIQMuXOJZYUUaxEHSC1Sud4rwIIqS6qsxetW9x0x7SWsE5YF5QNr iTZGFC99M1xZulYABpgmexZdGMPYq1R6558jH0crXonZ903may/AIZJezC7EdnbNmPHF HprJP0R7kBgJ5M5UR4bitFU6PT8MZYFuhuYMYaq+bI6frgV1+mfzZiREOHvgtxjU+BdX fqVxtbzleaAgAPSFrSv/zwxGWLyDimDnRNCzDmzXnAs2pqb4W7FNfYH/+v1jLAZtBpU1 clTA== X-Gm-Message-State: APjAAAX4q9aI9mf4BWfLi7QbWOZc1EmarWp2+LgVQ9pgx50blmicg2dN 7katw5u+vopt+pnWbJxGihw= X-Google-Smtp-Source: APXvYqyB1157MtzfYeWJ1dBcWuMDKOUJgOnzq0VC5i1SowoHtyq/iVzFaALo7AjUpN0O6A6XZS+NfQ== X-Received: by 2002:a17:902:9a85:: with SMTP id w5mr121452633plp.221.1564592333814; Wed, 31 Jul 2019 09:58:53 -0700 (PDT) Received: from nebulus.mtv.corp.google.com ([2620:15c:211:200:5404:91ba:59dc:9400]) by smtp.gmail.com with ESMTPSA id f72sm2245954pjg.10.2019.07.31.09.58.51 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 31 Jul 2019 09:58:53 -0700 (PDT) From: Mark Salyzyn To: linux-kernel@vger.kernel.org Subject: [PATCH v13 4/5] overlayfs: internal getxattr operations without sepolicy checking Date: Wed, 31 Jul 2019 09:57:59 -0700 Message-Id: <20190731165803.4755-5-salyzyn@android.com> X-Mailer: git-send-email 2.22.0.770.g0f2c4a37fd-goog In-Reply-To: <20190731165803.4755-1-salyzyn@android.com> References: <20190731165803.4755-1-salyzyn@android.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190731_095855_231389_75CE3BBA X-CRM114-Status: GOOD ( 19.39 ) X-Spam-Score: -0.2 (/) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (-0.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2607:f8b0:4864:20:0:0:0:641 listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.0 DKIMWL_WL_HIGH DKIMwl.org - Whitelisted High sender X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Latchesar Ionkov , Dave Kleikamp , jfs-discussion@lists.sourceforge.net, Randy Dunlap , linux-doc@vger.kernel.org, Martin Brandenburg , samba-technical@lists.samba.org, Dominique Martinet , Amir Goldstein , David Howells , Chris Mason , "David S . Miller" , Andreas Dilger , ocfs2-devel@oss.oracle.com, netdev@vger.kernel.org, Tyler Hicks , linux-afs@lists.infradead.org, Mike Marshall , linux-xfs@vger.kernel.org, Andreas Gruenbacher , Sage Weil , Jonathan Corbet , Richard Weinberger , Mark Fasheh , devel@lists.orangefs.org, Hugh Dickins , linux-security-module@vger.kernel.org, cluster-devel@redhat.com, Vyacheslav Dubeyko , v9fs-developer@lists.sourceforge.net, Ilya Dryomov , linux-ext4@vger.kernel.org, Stephen Smalley , linux-mm@kvack.org, Vivek Goyal , Chao Yu , linux-cifs@vger.kernel.org, Eric Van Hensbergen , ecryptfs@vger.kernel.org, Josef Bacik , "Darrick J . Wong" , reiserfs-devel@vger.kernel.org, Tejun Heo , Greg Kroah-Hartman , Joel Becker , linux-mtd@lists.infradead.org, David Sterba , Jaegeuk Kim , ceph-devel@vger.kernel.org, Trond Myklebust , linux-nfs@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Theodore Ts'o , linux-fsdevel@vger.kernel.org, Joseph Qi , Mathieu Malaterre , kernel-team@android.com, Miklos Szeredi , Jeff Layton , linux-unionfs@vger.kernel.org, stable@vger.kernel.org, Mark Salyzyn , Steve French , =?utf-8?q?Ernesto_A_=2E_Fern=C3=A1n?= =?utf-8?q?dez?= , "Eric W . Biederman" , Jan Kara , Bob Peterson , Phillip Lougher , Andrew Morton , David Woodhouse , Anna Schumaker , linux-btrfs@vger.kernel.org, Alexander Viro Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org Check impure, opaque, origin & meta xattr with no sepolicy audit (using __vfs_getxattr) since these operations are internal to overlayfs operations and do not disclose any data. This became an issue for credential override off since sys_admin would have been required by the caller; whereas would have been inherently present for the creator since it performed the mount. This is a change in operations since we do not check in the new ovl_do_vfs_getxattr function if the credential override is off or not. Reasoning is that the sepolicy check is unnecessary overhead, especially since the check can be expensive. Because for override credentials off, this affects _everyone_ that underneath performs private xattr calls without the appropriate sepolicy permissions and sys_admin capability. Providing blanket support for sys_admin would be bad for all possible callers. For the override credentials on, this will affect only the mounter, should it lack sepolicy permissions. Not considered a security problem since mounting by definition has sys_admin capabilities, but sepolicy contexts would still need to be crafted. It should be noted that there is precedence, __vfs_getxattr is used in other filesystems for their own internal trusted xattr management. Signed-off-by: Mark Salyzyn Cc: Miklos Szeredi Cc: Jonathan Corbet Cc: Vivek Goyal Cc: Eric W. Biederman Cc: Amir Goldstein Cc: Randy Dunlap Cc: Stephen Smalley Cc: linux-unionfs@vger.kernel.org Cc: linux-doc@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: kernel-team@android.com Cc: Eric Van Hensbergen Cc: Latchesar Ionkov Cc: Dominique Martinet Cc: David Howells Cc: Chris Mason Cc: Josef Bacik Cc: David Sterba Cc: Jeff Layton Cc: Sage Weil Cc: Ilya Dryomov Cc: Steve French Cc: Tyler Hicks Cc: Jan Kara Cc: Theodore Ts'o Cc: Andreas Dilger Cc: Jaegeuk Kim Cc: Chao Yu Cc: Bob Peterson Cc: Andreas Gruenbacher Cc: David Woodhouse Cc: Richard Weinberger Cc: Dave Kleikamp Cc: Greg Kroah-Hartman Cc: Tejun Heo Cc: Trond Myklebust Cc: Anna Schumaker Cc: Mark Fasheh Cc: Joel Becker Cc: Joseph Qi Cc: Mike Marshall Cc: Martin Brandenburg Cc: Alexander Viro Cc: Phillip Lougher Cc: Darrick J. Wong Cc: linux-xfs@vger.kernel.org Cc: Hugh Dickins Cc: David S. Miller Cc: Andrew Morton Cc: Mathieu Malaterre Cc: Ernesto A. Fernández Cc: Vyacheslav Dubeyko Cc: v9fs-developer@lists.sourceforge.net Cc: linux-afs@lists.infradead.org Cc: linux-btrfs@vger.kernel.org Cc: ceph-devel@vger.kernel.org Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Cc: ecryptfs@vger.kernel.org Cc: linux-ext4@vger.kernel.org Cc: linux-f2fs-devel@lists.sourceforge.net Cc: linux-fsdevel@vger.kernel.org Cc: cluster-devel@redhat.com Cc: linux-mtd@lists.infradead.org Cc: jfs-discussion@lists.sourceforge.net Cc: linux-nfs@vger.kernel.org Cc: ocfs2-devel@oss.oracle.com Cc: devel@lists.orangefs.org Cc: reiserfs-devel@vger.kernel.org Cc: linux-mm@kvack.org Cc: netdev@vger.kernel.org Cc: linux-security-module@vger.kernel.org Cc: stable@vger.kernel.org # 4.4, 4.9, 4.14 & 4.19 --- v13 - rebase to use __vfs_getxattr flags option v12 - rebase v11 - switch name to ovl_do_vfs_getxattr, fortify comment v10 - added to patch series --- fs/overlayfs/namei.c | 12 +++++++----- fs/overlayfs/overlayfs.h | 2 ++ fs/overlayfs/util.c | 25 ++++++++++++++++--------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 9702f0d5309d..a4a452c489fa 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -106,10 +106,11 @@ int ovl_check_fh_len(struct ovl_fh *fh, int fh_len) static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name) { - int res, err; + ssize_t res; + int err; struct ovl_fh *fh = NULL; - res = vfs_getxattr(dentry, name, NULL, 0); + res = ovl_do_vfs_getxattr(dentry, name, NULL, 0); if (res < 0) { if (res == -ENODATA || res == -EOPNOTSUPP) return NULL; @@ -123,7 +124,7 @@ static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name) if (!fh) return ERR_PTR(-ENOMEM); - res = vfs_getxattr(dentry, name, fh, res); + res = ovl_do_vfs_getxattr(dentry, name, fh, res); if (res < 0) goto fail; @@ -141,10 +142,11 @@ static struct ovl_fh *ovl_get_fh(struct dentry *dentry, const char *name) return NULL; fail: - pr_warn_ratelimited("overlayfs: failed to get origin (%i)\n", res); + pr_warn_ratelimited("overlayfs: failed to get origin (%zi)\n", res); goto out; invalid: - pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n", res, fh); + pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n", + (int)res, fh); goto out; } diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index ab3d031c422b..9d26d8758513 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -205,6 +205,8 @@ int ovl_want_write(struct dentry *dentry); void ovl_drop_write(struct dentry *dentry); struct dentry *ovl_workdir(struct dentry *dentry); const struct cred *ovl_override_creds(struct super_block *sb); +ssize_t ovl_do_vfs_getxattr(struct dentry *dentry, const char *name, void *buf, + size_t size); struct super_block *ovl_same_sb(struct super_block *sb); int ovl_can_decode_fh(struct super_block *sb); struct dentry *ovl_indexdir(struct super_block *sb); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index f5678a3f8350..c588c0d66d8c 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -40,6 +40,13 @@ const struct cred *ovl_override_creds(struct super_block *sb) return override_creds(ofs->creator_cred); } +ssize_t ovl_do_vfs_getxattr(struct dentry *dentry, const char *name, void *buf, + size_t size) +{ + return __vfs_getxattr(dentry, d_inode(dentry), name, buf, size, + XATTR_NOSECURITY); +} + struct super_block *ovl_same_sb(struct super_block *sb) { struct ovl_fs *ofs = sb->s_fs_info; @@ -537,9 +544,9 @@ void ovl_copy_up_end(struct dentry *dentry) bool ovl_check_origin_xattr(struct dentry *dentry) { - int res; + ssize_t res; - res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0); + res = ovl_do_vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0); /* Zero size value means "copied up but origin unknown" */ if (res >= 0) @@ -550,13 +557,13 @@ bool ovl_check_origin_xattr(struct dentry *dentry) bool ovl_check_dir_xattr(struct dentry *dentry, const char *name) { - int res; + ssize_t res; char val; if (!d_is_dir(dentry)) return false; - res = vfs_getxattr(dentry, name, &val, 1); + res = ovl_do_vfs_getxattr(dentry, name, &val, 1); if (res == 1 && val == 'y') return true; @@ -837,13 +844,13 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) /* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */ int ovl_check_metacopy_xattr(struct dentry *dentry) { - int res; + ssize_t res; /* Only regular files can have metacopy xattr */ if (!S_ISREG(d_inode(dentry)->i_mode)) return 0; - res = vfs_getxattr(dentry, OVL_XATTR_METACOPY, NULL, 0); + res = ovl_do_vfs_getxattr(dentry, OVL_XATTR_METACOPY, NULL, 0); if (res < 0) { if (res == -ENODATA || res == -EOPNOTSUPP) return 0; @@ -852,7 +859,7 @@ int ovl_check_metacopy_xattr(struct dentry *dentry) return 1; out: - pr_warn_ratelimited("overlayfs: failed to get metacopy (%i)\n", res); + pr_warn_ratelimited("overlayfs: failed to get metacopy (%zi)\n", res); return res; } @@ -878,7 +885,7 @@ ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value, ssize_t res; char *buf = NULL; - res = vfs_getxattr(dentry, name, NULL, 0); + res = ovl_do_vfs_getxattr(dentry, name, NULL, 0); if (res < 0) { if (res == -ENODATA || res == -EOPNOTSUPP) return -ENODATA; @@ -890,7 +897,7 @@ ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value, if (!buf) return -ENOMEM; - res = vfs_getxattr(dentry, name, buf, res); + res = ovl_do_vfs_getxattr(dentry, name, buf, res); if (res < 0) goto fail; }