Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp2686036yba; Mon, 6 May 2019 09:58:55 -0700 (PDT) X-Google-Smtp-Source: APXvYqz+aoyw8og4WyU9gLm+0p3RJ9sGuWtLkQ4B2/Z0z7XgjTRP/4dcojkp9nH0XJi2JOPRaWWR X-Received: by 2002:a17:902:b098:: with SMTP id p24mr10464601plr.38.1557161935376; Mon, 06 May 2019 09:58:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1557161935; cv=none; d=google.com; s=arc-20160816; b=DQ6qllUez8+GTZ0tvU40fGo8Lngf0saiEPOjVdLAwkIo553s2hp6+Iktx3u7wwRo3F 9qMRwLLISfrOfDdUKyZM2svnvX1EME4bfJ2Sdj3zzn/88hmPdVezJiaQ/C2U7ljrgny/ fklBVCrfJh9nz/tDYYRMO51pYKhM8RFg8e7kfhk5Mfw7vnEuVaz7grZNHDUSetCjd5xM mLnrqEvACCDXpvOkQjd33tPqnGczMwvaB/fK+GUkQ7mxkDkscMyLBuqZBKk/9db732M0 96w+U3B5Ux2jI50fmbUe2G2d9pfBOz5i87RkI8NofU55i4Ldsh9KS/VDo8ZCEIWssCcH /6Eg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=2K6FlpmsdMO2QY5hkhFh73Fgs+vCOcPtphWoc5INJKo=; b=OT62038Z02ZzYu/IdYKxAIl0EOZtSjQJeuBA3ef+jVs6hJ9qp2XMAXaTuXc8mTw7QC o1Oa5fqTviZViVs2FxQsb6uRK2phkMMdSlMtKRlf424veMz9D+dZNI3Pvz7aFRtF86r7 SBNto2K08ANwg7FYhBqZE0wy0uhIY8nlyvlwsI5oaej8Mr2a57NNtMqOfTjqIdbRgh5o tVwFMYfkLA+RQqmr8y+hDw7j7Y480/F+9oAsma4qYaw79t+2wJu/UF4+HbYUw5A5s2EV VBPLyIeFdFO+Ehbt9qNUQIiG493Ah9VtevBHgh60rY0MPX3y9lXsugLgyY8xz0SOR/36 jMdw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 91si16340895ply.104.2019.05.06.09.58.38; Mon, 06 May 2019 09:58:55 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727075AbfEFQ4E (ORCPT + 99 others); Mon, 6 May 2019 12:56:04 -0400 Received: from mx2.mailbox.org ([80.241.60.215]:59248 "EHLO mx2.mailbox.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726473AbfEFQ4E (ORCPT ); Mon, 6 May 2019 12:56:04 -0400 Received: from smtp1.mailbox.org (smtp1.mailbox.org [80.241.60.240]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mx2.mailbox.org (Postfix) with ESMTPS id 7B83DA0AED; Mon, 6 May 2019 18:56:00 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by spamfilter05.heinlein-hosting.de (spamfilter05.heinlein-hosting.de [80.241.56.123]) (amavisd-new, port 10030) with ESMTP id cCMuw5rMncnT; Mon, 6 May 2019 18:55:48 +0200 (CEST) From: Aleksa Sarai To: Al Viro , Jeff Layton , "J. Bruce Fields" , Arnd Bergmann , David Howells Cc: Aleksa Sarai , Jann Horn , Kees Cook , Eric Biederman , Andy Lutomirski , Andrew Morton , Alexei Starovoitov , Christian Brauner , Tycho Andersen , David Drysdale , Chanho Min , Oleg Nesterov , Aleksa Sarai , Linus Torvalds , containers@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org Subject: [PATCH v6 4/6] namei: aggressively check for nd->root escape on ".." resolution Date: Tue, 7 May 2019 02:54:37 +1000 Message-Id: <20190506165439.9155-5-cyphar@cyphar.com> In-Reply-To: <20190506165439.9155-1-cyphar@cyphar.com> References: <20190506165439.9155-1-cyphar@cyphar.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This patch allows for O_BENEATH and O_THISROOT to safely permit ".." resolution (in the case of O_BENEATH the resolution will still fail if ".." resolution would resolve a path outside of the root -- while O_THISROOT will chroot(2)-style scope it). "magic link" jumps are still disallowed entirely because now they could result in inconsistent behaviour if resolution encounters a subsequent "..". The need for this patch is explained by observing there is a fairly easy-to-exploit race condition with chroot(2) (and thus by extension O_THISROOT and O_BENEATH) where a rename(2) of a path can be used to "skip over" nd->root and thus escape to the filesystem above nd->root. thread1 [attacker]: for (;;) renameat2(AT_FDCWD, "/a/b/c", AT_FDCWD, "/a/d", RENAME_EXCHANGE); thread2 [victim]: for (;;) openat(dirb, "b/c/../../etc/shadow", O_THISROOT); With fairly significant regularity, thread2 will resolve to "/etc/shadow" rather than "/a/b/etc/shadow". There is also a similar (though somewhat more privileged) attack using MS_MOVE. With this patch, such cases will be detected *during* ".." resolution (which is the weak point of chroot(2) -- since walking *into* a subdirectory tautologically cannot result in you walking *outside* nd->root -- except through a bind-mount or "magic link"). By detecting this at ".." resolution (rather than checking only at the end of the entire resolution) we can both correct escapes by jumping back to the root (in the case of O_THISROOT), as well as avoid revealing to attackers the structure of the filesystem outside of the root (through timing attacks for instance). In order to avoid a quadratic lookup with each ".." entry, we only activate the slow path if a write through &rename_lock or &mount_lock have occurred during path resolution (&rename_lock and &mount_lock are re-taken to further optimise the lookup). Since the primary attack being protected against is MS_MOVE or rename(2), not doing additional checks unless a mount or rename have occurred avoids making the common case slow. The use of path_is_under() here might seem suspect, but on further inspection of the most important race (a path was *inside* the root but is now *outside*), there appears to be no attack potential. If path_is_under() occurs before the rename, then the path will be resolved but since the path was originally inside the root there is no escape. Subsequent ".." jumps are guaranteed to check path_is_under() (by construction, &rename_lock or &mount_lock must have been taken by the attacker after path_is_under() returned in the victim), and thus will not be able to escape from the previously-inside-root path. Walking down is still safe since the entire subtree was moved (either by rename(2) or MS_MOVE) and because (as discussed above) walking down is safe. Cc: Al Viro Cc: Jann Horn Cc: Kees Cook Signed-off-by: Aleksa Sarai --- fs/namei.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 3a3cba593b85..2b6a1bf4e745 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -491,7 +491,7 @@ struct nameidata { struct path root; struct inode *inode; /* path.dentry.d_inode */ unsigned int flags; - unsigned seq, m_seq; + unsigned seq, m_seq, r_seq; int last_type; unsigned depth; int total_link_count; @@ -1739,19 +1739,35 @@ static inline int may_lookup(struct nameidata *nd) static inline int handle_dots(struct nameidata *nd, int type) { if (type == LAST_DOTDOT) { - /* - * LOOKUP_BENEATH resolving ".." is not currently safe -- races can - * cause our parent to have moved outside of the root and us to skip - * over it. - */ - if (unlikely(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT))) - return -EXDEV; + int error = 0; + if (!nd->root.mnt) set_root(nd); - if (nd->flags & LOOKUP_RCU) { - return follow_dotdot_rcu(nd); - } else - return follow_dotdot(nd); + if (nd->flags & LOOKUP_RCU) + error = follow_dotdot_rcu(nd); + else + error = follow_dotdot(nd); + if (error) + return error; + + if (unlikely(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT))) { + bool m_retry = read_seqretry(&mount_lock, nd->m_seq); + bool r_retry = read_seqretry(&rename_lock, nd->r_seq); + + /* + * Don't bother checking unless there's a racing + * rename(2) or MS_MOVE. + */ + if (likely(!m_retry && !r_retry)) + return 0; + + if (m_retry && !(nd->flags & LOOKUP_RCU)) + nd->m_seq = read_seqbegin(&mount_lock); + if (r_retry) + nd->r_seq = read_seqbegin(&rename_lock); + if (!path_is_under(&nd->path, &nd->root)) + return -EXDEV; + } } return 0; } @@ -2272,6 +2288,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags) nd->last_type = LAST_ROOT; /* if there are only slashes... */ nd->flags = flags | LOOKUP_JUMPED | LOOKUP_PARENT; nd->depth = 0; + + nd->m_seq = read_seqbegin(&mount_lock); + if (unlikely(flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT))) + nd->r_seq = read_seqbegin(&rename_lock); + if (flags & LOOKUP_ROOT) { struct dentry *root = nd->root.dentry; struct inode *inode = root->d_inode; @@ -2282,7 +2303,6 @@ static const char *path_init(struct nameidata *nd, unsigned flags) if (flags & LOOKUP_RCU) { nd->seq = __read_seqcount_begin(&nd->path.dentry->d_seq); nd->root_seq = nd->seq; - nd->m_seq = read_seqbegin(&mount_lock); } else { path_get(&nd->path); } @@ -2293,8 +2313,6 @@ static const char *path_init(struct nameidata *nd, unsigned flags) nd->path.mnt = NULL; nd->path.dentry = NULL; - nd->m_seq = read_seqbegin(&mount_lock); - if (unlikely(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT))) { error = dirfd_path_init(nd); if (unlikely(error)) -- 2.21.0