From patchwork Thu May 28 21:32:14 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: 6501901 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 971A6C0020 for ; Thu, 28 May 2015 21:37:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 94746206B5 for ; Thu, 28 May 2015 21:37:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 72C8720626 for ; Thu, 28 May 2015 21:37:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754515AbbE1VhR (ORCPT ); Thu, 28 May 2015 17:37:17 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:37150 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754676AbbE1VhP (ORCPT ); Thu, 28 May 2015 17:37:15 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1Yy5UI-0007Wf-07; Thu, 28 May 2015 15:37:14 -0600 Received: from 67-3-205-90.omah.qwest.net ([67.3.205.90] 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 1Yy5UG-0000Zl-6N; Thu, 28 May 2015 15:37:13 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Richard Weinberger Cc: Serge Hallyn , Andy Lutomirski , Seth Forshee , Linux API , Linux Containers , Greg Kroah-Hartman , Kenton Varda , Michael Kerrisk-manpages , Linux FS Devel , Tejun Heo References: <87pp63jcca.fsf@x220.int.ebiederm.org> <87siaxuvik.fsf@x220.int.ebiederm.org> <87wq004im1.fsf@x220.int.ebiederm.org> <20150528140839.GD28842@ubuntumail> <55676E32.3050006@nod.at> <87382gh3uo.fsf@x220.int.ebiederm.org> <55677AEF.1090809@nod.at> Date: Thu, 28 May 2015 16:32:14 -0500 In-Reply-To: <55677AEF.1090809@nod.at> (Richard Weinberger's message of "Thu, 28 May 2015 22:30:39 +0200") Message-ID: <87iobcfkwx.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+YUN3LnJEmDfgw9FApN1alJhvNXXh6OZA= X-SA-Exim-Connect-IP: 67.3.205.90 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=unavailable version=3.3.1 X-Spam-DCC: XMission; sa04 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ***;Richard Weinberger X-Spam-Relay-Country: X-Spam-Timing: total 1302 ms - load_scoreonly_sql: 0.03 (0.0%), signal_user_changed: 3.3 (0.3%), b_tie_ro: 2.4 (0.2%), parse: 0.62 (0.0%), extract_message_metadata: 13 (1.0%), get_uri_detail_list: 2.2 (0.2%), tests_pri_-1000: 7 (0.5%), tests_pri_-950: 0.95 (0.1%), tests_pri_-900: 0.84 (0.1%), tests_pri_-400: 23 (1.7%), check_bayes: 22 (1.7%), b_tokenize: 7 (0.6%), b_tok_get_all: 7 (0.6%), b_comp_prob: 1.81 (0.1%), b_tok_touch_all: 3.3 (0.3%), b_finish: 0.57 (0.0%), tests_pri_0: 1247 (95.7%), tests_pri_500: 5.0 (0.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: [CFT][PATCH 00/10] Making new mounts of proc and sysfs as safe as bind mounts (take 2) 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 Richard Weinberger writes: > Am 28.05.2015 um 21:57 schrieb Eric W. Biederman: >>> FWIW, it breaks also libvirt-lxc: >>> Error: internal error: guest failed to start: Failed to re-mount /proc/sys on /proc/sys flags=1021: Operation not permitted >> >> Interesting. I had not anticipated a failure there? And it is failing >> in remount? Oh that is interesting. >> >> That implies that there is some flag of the original mount of /proc that >> the remount of /proc/sys is clearing, and that previously >> >> The flags specified are current rdonly,remount,bind so I expect there >> are some other flags on proc that libvirt-lxc is clearing by accident >> and we did not fail before because the kernel was not enforcing things. > > Please see: > http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/lxc/lxc_container.c;h=9a9ae5c2aaf0f90ff472f24fda43c077b44998c7;hb=HEAD#l933 > lxcContainerMountBasicFS() > > and: > http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/lxc/lxc_container.c;h=9a9ae5c2aaf0f90ff472f24fda43c077b44998c7;hb=HEAD#l850 > lxcBasicMounts > >> What are the mount flags in a working libvirt-lxc? > > See: > test1:~ # cat /proc/self/mountinfo > 149 147 0:56 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw > 150 149 0:56 /sys /proc/sys ro,nodev,relatime - proc proc rw > If you need more info, please let me know. :-) Oh interesting I had not realized libvirt-lxc had grown an unprivileged mode using user namespaces. This does appear to be a classic remount bug, where you are not preserving the permissions. It appears the fact that the code failed to enforce locked permissions on the fresh mount of proc was hiding this bug until now. I expect what you actually want is the code below: As the there is little point in making /proc/sys read-only in a user-namespace, as the permission checks are uid based and no-one should have the global uid 0 in your container. Making mounting /proc/sys read-only rather pointless. Eric --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9a9ae5c2aaf0..f008a7484bfe 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -850,7 +850,7 @@ typedef struct { static const virLXCBasicMountInfo lxcBasicMounts[] = { { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false, false }, - { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false, false, false }, + { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false }, { "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", NULL, MS_BIND, false, false, true }, { "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", NULL, MS_BIND, false, false, true }, { "sysfs", "/sys", "sysfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false }, Or possibly just: diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9a9ae5c2aaf0..a60ccbd12bfc 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -850,7 +850,7 @@ typedef struct { static const virLXCBasicMountInfo lxcBasicMounts[] = { { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false, false }, - { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false, false, false }, + { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, true, false, false }, { "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", NULL, MS_BIND, false, false, true }, { "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", NULL, MS_BIND, false, false, true }, { "sysfs", "/sys", "sysfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false },