From patchwork Wed Apr 8 23:34:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 6182931 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 954129F2EC for ; Wed, 8 Apr 2015 23:38:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8E15C20395 for ; Wed, 8 Apr 2015 23:38:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 419A42038F for ; Wed, 8 Apr 2015 23:38:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932445AbbDHXiR (ORCPT ); Wed, 8 Apr 2015 19:38:17 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:60429 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932267AbbDHXiR (ORCPT ); Wed, 8 Apr 2015 19:38:17 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1YfzY0-0005EP-Jj; Wed, 08 Apr 2015 17:38:16 -0600 Received: from 70-59-163-10.omah.qwest.net ([70.59.163.10] helo=x220.int.ebiederm.org.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1YfzXz-0004KW-M2; Wed, 08 Apr 2015 17:38:16 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Linux Containers Cc: , Al Viro , Andy Lutomirski , "Serge E. Hallyn" , Richard Weinberger , Andrey Vagin , Jann Horn , Willy Tarreau , Omar Sandoval References: <871tncuaf6.fsf@x220.int.ebiederm.org> <87mw5xq7lt.fsf@x220.int.ebiederm.org> <87a8yqou41.fsf_-_@x220.int.ebiederm.org> <874moq9oyb.fsf_-_@x220.int.ebiederm.org> Date: Wed, 08 Apr 2015 18:34:12 -0500 In-Reply-To: <874moq9oyb.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Wed, 08 Apr 2015 18:31:56 -0500") Message-ID: <87iod68aa3.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-XM-AID: U2FsdGVkX1/NuyTk1V+uIiTTSiGn8Nkdt498+BhOdBs= X-SA-Exim-Connect-IP: 70.59.163.10 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: *;Linux Containers X-Spam-Relay-Country: X-Spam-Timing: total 409 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 4.3 (1.0%), b_tie_ro: 3.0 (0.7%), parse: 1.22 (0.3%), extract_message_metadata: 16 (3.8%), get_uri_detail_list: 4.2 (1.0%), tests_pri_-1000: 6 (1.4%), tests_pri_-950: 1.31 (0.3%), tests_pri_-900: 1.11 (0.3%), tests_pri_-400: 29 (7.0%), check_bayes: 27 (6.7%), b_tokenize: 9 (2.3%), b_tok_get_all: 9 (2.3%), b_comp_prob: 3.0 (0.7%), b_tok_touch_all: 3.2 (0.8%), b_finish: 0.80 (0.2%), tests_pri_0: 342 (83.6%), tests_pri_500: 5 (1.3%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH review 4/4] vfs: Do not allow escaping from bind mounts. X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Rename can move a file or directory outside of a bind mount. This has allowed programs with paths below the renamed directory to traverse up their directory tree to the real root of the filesystem instead of just the root of their bind mount. In the presence of such renames limit applications to what the bind mount intended to reveal by marking mounts that have had dentries renamed out of them with MNT_VIOLATED, marking mounts that can no longer walk up to their parent mounts with MNT_UMOUNT_PENDING and then lazily unmounting such mounts. All moves go through __d_move so __d_move has been modified to mark all mounts whose dentries have been moved outside of them. Once the root dentry of a violated mount has been found a new function mnt_set_violated is called to mark all mounts that have that dentry as their root as violated. The worst case performance of the changes to __d_move is two extra trip from dentry and target to the root of their respective dentry trees. This closes a hole where it was possible in some circumstances to follow .. past the root of a bind mount. Cc: stable@vger.kernel.org Signed-off-by: "Eric W. Biederman" --- fs/dcache.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- fs/internal.h | 1 + fs/namespace.c | 17 +++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index 6e68312494ed..7baecba354dd 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2535,6 +2535,56 @@ static void dentry_unlock_for_move(struct dentry *dentry, struct dentry *target) spin_unlock(&dentry->d_lock); } +static unsigned d_depth(const struct dentry *dentry) +{ + unsigned depth = 0; + + while (!IS_ROOT(dentry)) { + dentry = dentry->d_parent; + depth++; + } + return depth; +} + +static const struct dentry *d_common_ancestor(const struct dentry *left, + const struct dentry *right) +{ + unsigned ldepth = d_depth(left); + unsigned rdepth = d_depth(right); + + if (ldepth > rdepth) { + swap(left, right); + swap(ldepth, rdepth); + } + + while (rdepth > ldepth) { + right = right->d_parent; + rdepth--; + } + + while (right != left) { + if (IS_ROOT(right)) + return NULL; + right = right->d_parent; + left = left->d_parent; + } + + return right; +} + +static void mark_violated_mounts(struct dentry *dentry, + const struct dentry *ancestor) +{ + /* Mark all mountroots that are children of the common + * ancestor and ancestors of dentry. + */ + struct dentry *p; + for (p = dentry->d_parent; p != ancestor; p = p->d_parent) { + if (d_mountroot(p)) + mnt_set_violated(p); + } +} + /* * When switching names, the actual string doesn't strictly have to * be preserved in the target - because we're dropping the target @@ -2563,11 +2613,22 @@ static void dentry_unlock_for_move(struct dentry *dentry, struct dentry *target) static void __d_move(struct dentry *dentry, struct dentry *target, bool exchange) { + const struct dentry *ancestor = d_common_ancestor(dentry, target); + if (!dentry->d_inode) printk(KERN_WARNING "VFS: moving negative dcache entry\n"); - BUG_ON(d_ancestor(dentry, target)); - BUG_ON(d_ancestor(target, dentry)); + BUG_ON(dentry == ancestor); + BUG_ON(target == ancestor); + + /* If there is a common ancestor, mark mounts which may have + * paths that are no longer able to follow d_parent up to + * mnt_root after this move. + */ + if (ancestor) { + mark_violated_mounts(dentry, ancestor); + mark_violated_mounts(target, ancestor); + } dentry_lock_for_move(dentry, target); diff --git a/fs/internal.h b/fs/internal.h index 046767f0042e..d6a6cbd1e7a1 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -71,6 +71,7 @@ extern int __mnt_want_write_file(struct file *); extern void __mnt_drop_write(struct vfsmount *); extern void __mnt_drop_write_file(struct file *); +extern void mnt_set_violated(struct dentry *root); /* * fs_struct.c */ diff --git a/fs/namespace.c b/fs/namespace.c index 75abc9fcaafa..083d96bdbd60 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1644,6 +1644,23 @@ out_unlock: namespace_unlock(); } +void mnt_set_violated(struct dentry *root) +{ + struct mountroot *mr; + + /* Locking the mount guarantees that root is a mountpoint and + * pushes rcu path walkers onto the slow path. + */ + lock_mount_hash(); + mr = lookup_mountroot(root); + if (mr) { + spin_lock(&root->d_lock); + root->d_flags |= DCACHE_MOUNT_VIOLATED; + spin_unlock(&root->d_lock); + } + unlock_mount_hash(); +} + /* * Is the caller allowed to modify his namespace? */