From patchwork Thu May 5 19:42:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Seth Forshee X-Patchwork-Id: 619040 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3r150W1LtJz9t6Y; Fri, 6 May 2016 05:42:35 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical-com.20150623.gappssmtp.com header.i=@canonical-com.20150623.gappssmtp.com header.b=BiRm9drA; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1ayPAL-0001lD-KA; Thu, 05 May 2016 19:42:29 +0000 Received: from mail-oi0-f41.google.com ([209.85.218.41]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.76) (envelope-from ) id 1ayPAB-0001k9-Mu for kernel-team@lists.ubuntu.com; Thu, 05 May 2016 19:42:19 +0000 Received: by mail-oi0-f41.google.com with SMTP id x201so115014765oif.3 for ; Thu, 05 May 2016 12:42:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=TzTlQQXtpSwmI7B583JlYF7MnZBd7qmdNs7L+gU3FJY=; b=BiRm9drAsuc2ryWfiDSNYnESni6wirnDP2yWM9DcEy/WCzaRvxFCWP0tL5gEcOuxVW kueSgmrf3pbyLxtwE5fyTdJ5Nz+8kccTizo8wOgpJD73L0PfHh2OJx41LU9BxJOFGyNN 0hRzKkFr2LJG8zj584eHGOyKw2o46NrSQ5sER20N2yLYABgHwyn3EAnIs1YU67IGl1pr 4FTpCwbRFrq+CnHpg10DsR+721n0J+LD59o3b9bSuf+F2ZdcBecH1YfJqUbw/n+999YP 3KXohQIdWOXUEgHby8qTyU8Eb5Lb2UXLRdu07mmdfCs34AUSKrK/ZEBzAH4+IwidllYj VakQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=TzTlQQXtpSwmI7B583JlYF7MnZBd7qmdNs7L+gU3FJY=; b=DnGlS/Xh0neu7t5aoTutQQ1exIEX8GyZD85CfwG7plapwE1B5UjT0p4jNIvbtt+4Vz w9dOJCFLP45EtMnjP2BvYaIKb3woa7tKQwxw8mwo/vfYlBSmS6XGscZfNbhIvbCCqQmo czqs5STQDV052qhtd/+lmOPx/24159Ym+/Kym6Ect9dN2+xecId0e6ptBRxCsvoUCmCF IGHrhCDe9O6YtfrHYpUetQDceJwkjU233ECqpxGX7KSs/eqeET+s2yADG/kAPXO3jafB G+296M5U4ERObqKyyhne8ruyA1S2zwGlNkn6PxqeB0y3emJCE/wCOAd40NR8B8E+dT59 zfiQ== X-Gm-Message-State: AOPr4FWNSX8TGN3CQj50St+v1TKgpv0U1jqUsD/sXZqbk1qp2tbvgzYLOh2xk17ausrmDTLk X-Received: by 10.157.46.5 with SMTP id q5mr8757916otb.162.1462477338357; Thu, 05 May 2016 12:42:18 -0700 (PDT) Received: from localhost ([2605:a601:aab:f920:9c94:497d:570f:4caa]) by smtp.gmail.com with ESMTPSA id k125sm3571129oia.21.2016.05.05.12.42.17 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 05 May 2016 12:42:17 -0700 (PDT) From: Seth Forshee To: kernel-team@lists.ubuntu.com Subject: [PATCH 2/2][t/u/v/w/x/y SRU] propogate_mnt: Handle the first propogated copy being a slave Date: Thu, 5 May 2016 14:42:15 -0500 Message-Id: <1462477335-79634-3-git-send-email-seth.forshee@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1462477335-79634-1-git-send-email-seth.forshee@canonical.com> References: <1462477335-79634-1-git-send-email-seth.forshee@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: "Eric W. Biederman" BugLink: http://bugs.launchpad.net/bugs/1572316 When the first propgated copy was a slave the following oops would result: > BUG: unable to handle kernel NULL pointer dereference at 0000000000000010 > IP: [] propagate_one+0xbe/0x1c0 > PGD bacd4067 PUD bac66067 PMD 0 > Oops: 0000 [#1] SMP > Modules linked in: > CPU: 1 PID: 824 Comm: mount Not tainted 4.6.0-rc5userns+ #1523 > Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 > task: ffff8800bb0a8000 ti: ffff8800bac3c000 task.ti: ffff8800bac3c000 > RIP: 0010:[] [] propagate_one+0xbe/0x1c0 > RSP: 0018:ffff8800bac3fd38 EFLAGS: 00010283 > RAX: 0000000000000000 RBX: ffff8800bb77ec00 RCX: 0000000000000010 > RDX: 0000000000000000 RSI: ffff8800bb58c000 RDI: ffff8800bb58c480 > RBP: ffff8800bac3fd48 R08: 0000000000000001 R09: 0000000000000000 > R10: 0000000000001ca1 R11: 0000000000001c9d R12: 0000000000000000 > R13: ffff8800ba713800 R14: ffff8800bac3fda0 R15: ffff8800bb77ec00 > FS: 00007f3c0cd9b7e0(0000) GS:ffff8800bfb00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000010 CR3: 00000000bb79d000 CR4: 00000000000006e0 > Stack: > ffff8800bb77ec00 0000000000000000 ffff8800bac3fd88 ffffffff811fbf85 > ffff8800bac3fd98 ffff8800bb77f080 ffff8800ba713800 ffff8800bb262b40 > 0000000000000000 0000000000000000 ffff8800bac3fdd8 ffffffff811f1da0 > Call Trace: > [] propagate_mnt+0x105/0x140 > [] attach_recursive_mnt+0x120/0x1e0 > [] graft_tree+0x63/0x70 > [] do_add_mount+0x9b/0x100 > [] do_mount+0x2aa/0xdf0 > [] ? strndup_user+0x4e/0x70 > [] SyS_mount+0x75/0xc0 > [] do_syscall_64+0x4b/0xa0 > [] entry_SYSCALL64_slow_path+0x25/0x25 > Code: 00 00 75 ec 48 89 0d 02 22 22 01 8b 89 10 01 00 00 48 89 05 fd 21 22 01 39 8e 10 01 00 00 0f 84 e0 00 00 00 48 8b 80 d8 00 00 00 <48> 8b 50 10 48 89 05 df 21 22 01 48 89 15 d0 21 22 01 8b 53 30 > RIP [] propagate_one+0xbe/0x1c0 > RSP > CR2: 0000000000000010 > ---[ end trace 2725ecd95164f217 ]--- This oops happens with the namespace_sem held and can be triggered by non-root users. An all around not pleasant experience. To avoid this scenario when finding the appropriate source mount to copy stop the walk up the mnt_master chain when the first source mount is encountered. Further rewrite the walk up the last_source mnt_master chain so that it is clear what is going on. The reason why the first source mount is special is that it it's mnt_parent is not a mount in the dest_mnt propagation tree, and as such termination conditions based up on the dest_mnt mount propgation tree do not make sense. To avoid other kinds of confusion last_dest is not changed when computing last_source. last_dest is only used once in propagate_one and that is above the point of the code being modified, so changing the global variable is meaningless and confusing. Cc: stable@vger.kernel.org fixes: f2ebb3a921c1ca1e2ddd9242e95a1989a50c4c68 ("smarter propagate_mnt()") Reported-by: Tycho Andersen Reviewed-by: Seth Forshee Tested-by: Seth Forshee Signed-off-by: "Eric W. Biederman" (cherry picked from commit 5ec0811d30378ae104f250bfc9b3640242d81e3f) Signed-off-by: Seth Forshee --- fs/pnode.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/fs/pnode.c b/fs/pnode.c index c524fdd..9989970 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -198,7 +198,7 @@ static struct mount *next_group(struct mount *m, struct mount *origin) /* all accesses are serialized by namespace_sem */ static struct user_namespace *user_ns; -static struct mount *last_dest, *last_source, *dest_master; +static struct mount *last_dest, *first_source, *last_source, *dest_master; static struct mountpoint *mp; static struct hlist_head *list; @@ -221,20 +221,22 @@ static int propagate_one(struct mount *m) type = CL_MAKE_SHARED; } else { struct mount *n, *p; + bool done; for (n = m; ; n = p) { p = n->mnt_master; - if (p == dest_master || IS_MNT_MARKED(p)) { - while (last_dest->mnt_master != p) { - last_source = last_source->mnt_master; - last_dest = last_source->mnt_parent; - } - if (!peers(n, last_dest)) { - last_source = last_source->mnt_master; - last_dest = last_source->mnt_parent; - } + if (p == dest_master || IS_MNT_MARKED(p)) break; - } } + do { + struct mount *parent = last_source->mnt_parent; + if (last_source == first_source) + break; + done = parent->mnt_master == p; + if (done && peers(n, parent)) + break; + last_source = last_source->mnt_master; + } while (!done); + type = CL_SLAVE; /* beginning of peer group among the slaves? */ if (IS_MNT_SHARED(m)) @@ -286,6 +288,7 @@ int propagate_mnt(struct mount *dest_mnt, struct mountpoint *dest_mp, */ user_ns = current->nsproxy->mnt_ns->user_ns; last_dest = dest_mnt; + first_source = source_mnt; last_source = source_mnt; mp = dest_mp; list = tree_list;