From patchwork Thu May 28 15:03:28 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: 6499351 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 0F47D9F1C1 for ; Thu, 28 May 2015 15:08:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 19B1A20625 for ; Thu, 28 May 2015 15:08:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0067320614 for ; Thu, 28 May 2015 15:08:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753684AbbE1PIc (ORCPT ); Thu, 28 May 2015 11:08:32 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:49211 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751529AbbE1PIa (ORCPT ); Thu, 28 May 2015 11:08:30 -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 1YxzQ4-0004dz-QV; Thu, 28 May 2015 09:08:28 -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 1YxzQ2-0004bc-Rk; Thu, 28 May 2015 09:08:28 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Serge Hallyn Cc: Andy Lutomirski , Seth Forshee , Linux API , Linux Containers , Greg Kroah-Hartman , Kenton Varda , Michael Kerrisk-manpages , Richard Weinberger , 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> Date: Thu, 28 May 2015 10:03:28 -0500 In-Reply-To: <20150528140839.GD28842@ubuntumail> (Serge Hallyn's message of "Thu, 28 May 2015 14:08:39 +0000") Message-ID: <87lhg8pwvz.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: U2FsdGVkX19LkH98apUTdevvdTSmMFzVtnDGGvDOZa0= 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; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ***;Serge Hallyn X-Spam-Relay-Country: X-Spam-Timing: total 1429 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 3.2 (0.2%), b_tie_ro: 2.2 (0.2%), parse: 1.09 (0.1%), extract_message_metadata: 55 (3.9%), get_uri_detail_list: 3.7 (0.3%), tests_pri_-1000: 19 (1.3%), tests_pri_-950: 1.77 (0.1%), tests_pri_-900: 1.51 (0.1%), tests_pri_-400: 67 (4.7%), check_bayes: 54 (3.8%), b_tokenize: 14 (1.0%), b_tok_get_all: 15 (1.1%), b_comp_prob: 3.5 (0.2%), b_tok_touch_all: 5 (0.4%), b_finish: 0.72 (0.1%), tests_pri_0: 1258 (88.0%), tests_pri_500: 6 (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 Serge Hallyn writes: > Quoting Andy Lutomirski (luto@amacapital.net): >> On Fri, May 22, 2015 at 10:39 AM, Eric W. Biederman >> wrote: >> > I had hoped to get some Tested-By's on that patch series. >> >> Sorry, I've been totally swamped. >> >> I suspect that Sandstorm is okay, but I haven't had a chance to test >> it for real. Sandstorm makes only limited use of proc and sysfs in >> containers, but I'll see if I can test it for real this weekend. > > Testing this with unprivileged containers, I get > > lxc-start: conf.c: lxc_mount_auto_mounts: 808 Operation not permitted > - error mounting sysfs on > /usr/lib/x86_64-linux-gnu/lxc/sys/devices/virtual/net flags 0 Grr.. I was afraid this would break something. :( Looking at my system I see that sysfs is currently mounted "nosuid,nodev,noexec" Looking at the lxc-start code I don't see it as including any of those mount options. In practice for sysfs I think those options are meaningless (as there should be no devices and nothing executable in sysfs) but I can understand the past concerns with chmod on virtual filesystems that would incline people to use them, so I think the failure is reporting a legitimate security issue in the lxc userspace code where the the unprivileged code is currently attempting to give greater access to sysfs than was given by the original mount of sysfs. As nosuid,nodev,noexec should not impair the operation of sysfs operation it looks like you can always specify those options and just make this concern go away. Something like the untested patch below I expect. 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/conf.c b/src/lxc/conf.c index 9870455b3cae..d9ccd03afe68 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -770,8 +770,8 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sysrq-trigger", "%r/proc/sysrq-trigger", NULL, MS_BIND, NULL }, { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, NULL, "%r/proc/sysrq-trigger", NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL }, { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_RW, "proc", "%r/proc", "proc", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL }, - { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_RW, "sysfs", "%r/sys", "sysfs", 0, NULL }, - { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_RO, "sysfs", "%r/sys", "sysfs", MS_RDONLY, NULL }, + { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_RW, "sysfs", "%r/sys", "sysfs", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL }, + { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_RO, "sysfs", "%r/sys", "sysfs", MS_NODEV|MS_NOEXEC|MS_NOSUID|MS_RDONLY, NULL }, { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_MIXED, "sysfs", "%r/sys", "sysfs", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL }, { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_MIXED, "%r/sys", "%r/sys", NULL, MS_BIND, NULL }, { LXC_AUTO_SYS_MASK, LXC_AUTO_SYS_MIXED, NULL, "%r/sys", NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL }, Alternately you can read the flags off of the original mount of proc or sysfs. diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 9870455b3cae..50ea49973e80 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -712,7 +712,9 @@ static unsigned long add_required_remount_flags(const char *s, const char *d, struct statvfs sb; unsigned long required_flags = 0; - if (!(flags & MS_REMOUNT)) + if (!(flags & MS_REMOUNT) && + (strcmp(s, "proc") != 0) && + (strcmp(s, "sysfs") != 0)) return flags; if (!s)