From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) Subject: Re: [PATCH v2 09/10] netns: Add a limit on the number of net namespaces Date: Tue, 26 Jul 2016 15:00:05 -0500 Message-ID: <87d1m0p3ui.fsf@x220.int.ebiederm.org> References: <87d1m754jc.fsf@x220.int.ebiederm.org> <20160721164014.17534-1-ebiederm@xmission.com> <20160721164014.17534-9-ebiederm@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: (Andrei Vagin's message of "Mon, 25 Jul 2016 23:01:19 -0700") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Andrei Vagin Cc: Kees Cook , netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Containers , LKML , Andy Lutomirski , Seth Forshee , Nikolay Borisov , Linux API , linux-fsdevel , Jann Horn List-Id: containers.vger.kernel.org Andrei Vagin writes: > On Thu, Jul 21, 2016 at 9:40 AM, Eric W. Biederman wrote: >> index 2c2eb1b629b1..a489f192d619 100644 >> --- a/net/core/net_namespace.c >> +++ b/net/core/net_namespace.c >> @@ -266,6 +266,16 @@ struct net *get_net_ns_by_id(struct net *net, int id) >> return peer; >> } >> >> +static bool inc_net_namespaces(struct user_namespace *ns) >> +{ >> + return inc_ucount(ns, UCOUNT_NET_NAMESPACES); >> +} >> + >> +static void dec_net_namespaces(struct user_namespace *ns) >> +{ >> + dec_ucount(ns, UCOUNT_NET_NAMESPACES); >> +} >> + >> /* >> * setup_net runs the initializers for the network namespace object. >> */ >> @@ -276,6 +286,9 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns) >> int error = 0; >> LIST_HEAD(net_exit_list); >> >> + if (!inc_net_namespaces(user_ns)) >> + return -ENFILE; > > I think you need to move this check after initilizing net->passive. > When setup_net returns an error, net_drop_ns is called: > Good point. Ouch! > void net_drop_ns(void *p) > { > struct net *ns = p; > if (ns && atomic_dec_and_test(&ns->passive)) > net_free(ns); > } > > Actually, I think it would be better to make this check before > net_alloc(). You are probably right. I seem to be trying to be entirely too clever putting that in setup_net so I can cover the initial network namespace. Which really does not need to be counted. As clearly I also goofed up the decrement on error case as well. Eric From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758118AbcGZUNv (ORCPT ); Tue, 26 Jul 2016 16:13:51 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:45445 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757140AbcGZUNn (ORCPT ); Tue, 26 Jul 2016 16:13:43 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Andrei Vagin Cc: Linux Containers , Kees Cook , netdev@vger.kernel.org, LKML , Andy Lutomirski , Seth Forshee , Nikolay Borisov , Linux API , linux-fsdevel , Jann Horn References: <87d1m754jc.fsf@x220.int.ebiederm.org> <20160721164014.17534-1-ebiederm@xmission.com> <20160721164014.17534-9-ebiederm@xmission.com> Date: Tue, 26 Jul 2016 15:00:05 -0500 In-Reply-To: (Andrei Vagin's message of "Mon, 25 Jul 2016 23:01:19 -0700") Message-ID: <87d1m0p3ui.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1bS8j0-00055y-IF;;;mid=<87d1m0p3ui.fsf@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=67.3.204.119;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+Gwloaw+dFXkQi54HXC5BQ1wOYPZhV5Pg= X-SA-Exim-Connect-IP: 67.3.204.119 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 * [sa07 1397; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: XMission; sa07 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Andrei Vagin X-Spam-Relay-Country: X-Spam-Timing: total 2312 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 4.8 (0.2%), b_tie_ro: 3.6 (0.2%), parse: 1.01 (0.0%), extract_message_metadata: 18 (0.8%), get_uri_detail_list: 1.93 (0.1%), tests_pri_-1000: 4.6 (0.2%), tests_pri_-950: 1.62 (0.1%), tests_pri_-900: 1.38 (0.1%), tests_pri_-400: 26 (1.1%), check_bayes: 24 (1.1%), b_tokenize: 9 (0.4%), b_tok_get_all: 7 (0.3%), b_comp_prob: 2.7 (0.1%), b_tok_touch_all: 2.7 (0.1%), b_finish: 0.80 (0.0%), tests_pri_0: 2248 (97.2%), check_dkim_signature: 0.55 (0.0%), check_dkim_adsp: 2005 (86.7%), tests_pri_500: 3.5 (0.2%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH v2 09/10] netns: Add a limit on the number of net namespaces X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -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 Andrei Vagin writes: > On Thu, Jul 21, 2016 at 9:40 AM, Eric W. Biederman wrote: >> index 2c2eb1b629b1..a489f192d619 100644 >> --- a/net/core/net_namespace.c >> +++ b/net/core/net_namespace.c >> @@ -266,6 +266,16 @@ struct net *get_net_ns_by_id(struct net *net, int id) >> return peer; >> } >> >> +static bool inc_net_namespaces(struct user_namespace *ns) >> +{ >> + return inc_ucount(ns, UCOUNT_NET_NAMESPACES); >> +} >> + >> +static void dec_net_namespaces(struct user_namespace *ns) >> +{ >> + dec_ucount(ns, UCOUNT_NET_NAMESPACES); >> +} >> + >> /* >> * setup_net runs the initializers for the network namespace object. >> */ >> @@ -276,6 +286,9 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns) >> int error = 0; >> LIST_HEAD(net_exit_list); >> >> + if (!inc_net_namespaces(user_ns)) >> + return -ENFILE; > > I think you need to move this check after initilizing net->passive. > When setup_net returns an error, net_drop_ns is called: > Good point. Ouch! > void net_drop_ns(void *p) > { > struct net *ns = p; > if (ns && atomic_dec_and_test(&ns->passive)) > net_free(ns); > } > > Actually, I think it would be better to make this check before > net_alloc(). You are probably right. I seem to be trying to be entirely too clever putting that in setup_net so I can cover the initial network namespace. Which really does not need to be counted. As clearly I also goofed up the decrement on error case as well. Eric