From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751339AbbCTBRz (ORCPT ); Thu, 19 Mar 2015 21:17:55 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:41903 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750731AbbCTBRx (ORCPT ); Thu, 19 Mar 2015 21:17:53 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Ian Kent Cc: Al Viro , Kernel Mailing List , David Howells , Oleg Nesterov , Trond Myklebust , "J. Bruce Fields" , Benjamin Coddington , Jeff Layton References: <20150317022308.24592.35785.stgit@pluto.fritz.box> <20150317024509.24592.88118.stgit@pluto.fritz.box> <20150319194711.GR29656@ZenIV.linux.org.uk> <1426813025.2724.2.camel@pluto.fritz.box> Date: Thu, 19 Mar 2015 20:14:05 -0500 In-Reply-To: <1426813025.2724.2.camel@pluto.fritz.box> (Ian Kent's message of "Fri, 20 Mar 2015 08:57:05 +0800") Message-ID: <87d244fp2a.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1/ViTH3q4iNAPcF6JT5hPMlJqrCl7Hii4w= X-SA-Exim-Connect-IP: 70.59.163.10 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.7 XMSubLong Long Subject * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa05 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_03 6+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject X-Spam-DCC: XMission; sa05 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Ian Kent X-Spam-Relay-Country: X-Spam-Timing: total 425 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 5 (1.2%), b_tie_ro: 4.3 (1.0%), parse: 1.81 (0.4%), extract_message_metadata: 13 (3.1%), get_uri_detail_list: 2.5 (0.6%), tests_pri_-1000: 6 (1.5%), tests_pri_-950: 1.19 (0.3%), tests_pri_-900: 0.93 (0.2%), tests_pri_-400: 26 (6.0%), check_bayes: 25 (5.8%), b_tokenize: 9 (2.1%), b_tok_get_all: 8 (1.9%), b_comp_prob: 2.0 (0.5%), b_tok_touch_all: 3.1 (0.7%), b_finish: 0.72 (0.2%), tests_pri_0: 361 (85.0%), tests_pri_500: 5 (1.2%), rewrite_mail: 0.00 (0.0%) Subject: Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Ian Kent writes: 2> On Thu, 2015-03-19 at 19:47 +0000, Al Viro wrote: >> On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote: >> > From: Ian Kent >> > >> > The mnt_namespace definition will be needed by the usermode helper >> > contained execution implementation, move it to include/linux/mount.h. >> >> I really don't like that. AFAICS, the root of the evil is that fscking >> nsproxy keeps a pointer to mnt_namespace while all it really needs to >> know about is the address of ns_common field embedded into it. Let's see... > > Thought that might be the case. > >> >> fs/namespace.c:697: struct mnt_namespace *ns = current->nsproxy->mnt_ns; >> fs/namespace.c:770: return mnt->mnt_ns == current->nsproxy->mnt_ns; >> fs/namespace.c:1502: return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN); >> fs/namespace.c:1587: return current->nsproxy->mnt_ns->seq >= mnt_ns->seq; >> fs/namespace.c:2293: struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns; >> fs/namespace.c:2961: touch_mnt_namespace(current->nsproxy->mnt_ns); >> fs/namespace.c:3003: init_task.nsproxy->mnt_ns = ns; >> fs/namespace.c:3101: ns_root.mnt = ¤t->nsproxy->mnt_ns->root->mnt; >> fs/namespace.c:3119: struct mnt_namespace *ns = current->nsproxy->mnt_ns; >> fs/namespace.c:3159: ns = &nsproxy->mnt_ns->ns; >> fs/namespace.c:3187: put_mnt_ns(nsproxy->mnt_ns); >> fs/namespace.c:3188: nsproxy->mnt_ns = mnt_ns; >> fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns; >> fs/proc_namespace.c:246: if (!nsp || !nsp->mnt_ns) { >> fs/proc_namespace.c:251: ns = nsp->mnt_ns; >> include/linux/nsproxy.h:33: struct mnt_namespace *mnt_ns; >> kernel/nsproxy.c:37: .mnt_ns = NULL, >> kernel/nsproxy.c:70: new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs); >> kernel/nsproxy.c:71: if (IS_ERR(new_nsp->mnt_ns)) { >> kernel/nsproxy.c:72: err = PTR_ERR(new_nsp->mnt_ns); >> kernel/nsproxy.c:113: if (new_nsp->mnt_ns) >> kernel/nsproxy.c:114: put_mnt_ns(new_nsp->mnt_ns); >> kernel/nsproxy.c:160: if (ns->mnt_ns) >> kernel/nsproxy.c:161: put_mnt_ns(ns->mnt_ns); >> >> OK, so we need >> >> a) add in fs/mount.h >> static inline struct mnt_namespace *current_mnt_ns(void) >> { >> return current->nsproxy->mnt_ns; >> } >> >> and replace a lot of open-coded instances. >> >> b) lift to_mnt_ns() into fs/mount.h (as static inline) >> >> c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns) >> with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover >> the original. >> >> d) make copy_mnt_ns() take and return struct ns_common * (same treatment as for >> put_mnt_ns() in (c); replace >> new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs); >> if (IS_ERR(new_nsp->mnt_ns)) { >> err = PTR_ERR(new_nsp->mnt_ns); >> goto out_ns; >> } >> with >> ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs); >> if (IS_ERR(ns)) { >> err = PTR_ERR(ns); >> goto out_ns; >> } >> new_nsp->mnt_ns = to_mnt_ns(ns); >> >> e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in >> nsproxy.h, deal with users of mnt_ns by >> * everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with >> put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again). >> * in fs/namespace.c replace >> init_task.nsproxy->mnt_ns = ns; >> with >> init_task.nsproxy->__mnt_ns = &ns->ns; >> replace >> ns = &nsproxy->mnt_ns->ns; >> with >> ns = nsproxy->__mnt_ns; >> replace >> nsproxy->mnt_ns = mnt_ns; >> with >> nsproxy>__mnt_ns = &mnt_ns->ns; >> * in fs/proc_namespace.c replace >> ns = nsp->mnt_ns; >> with >> ns = to_mnt_ns(nsp->__mnt_ns); >> and >> if (!nsp || !nsp->mnt_ns) { >> with >> if (!nsp || !nsp->__mnt_ns) { >> * in kernel/nsproxy.c: replace >> ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs); >> added in (d) with >> ns = copy_mnt_ns(flags, tsk->nsproxy->__mnt_ns, user_ns, new_fs); >> and >> new_nsp->mnt_ns = to_mnt_ns(ns); >> with >> new_nsp->__mnt_ns = ns; >> The rest of mnt_ns in that file get replaced by __mnt_ns. >> * in current_mnt_ns() replace ...->mnt_ns with to_mnt_ns(...->__mnt_ns) >> >> Do you want me to push such a series in vfs.git? After such 5 commits we have >> no pointers to struct mnt_namespace in nsproxy.h *and* no need for your patches >> to dig into ->mnt_ns->ns - we already have that as ->__mnt_ns. > > Yes please, I'd be more confident if you did this than me, there's > already enough to worry about with the series. Given that this patchset is a security hole waiting to happen I don't see why Al should bother unless there are good reasons to do this otherwise. Eric