From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=XrxjGss4ifBoJYGApXxggSDFp5ymM6KBNYrGZZ6kGMQ=; b=C5KCn+TpkB2+1gs6z7KV8C9abqB/u1K4VzBoEKbjvKUKSLCs6LkqYk1GJ1r1cE1sTO anC/+/gYWCfnP+hCjRq5Pgr5sUhWAvakAExaQ08p2Wz+QQ9lxd/xzIwFWa/oOajHKpkG LTNcfLFP4akBddV02i2mHtn8c6eTlXXWm9UPYkYb3kbIGnT7GqK6MRb2UWqoRsfwNNl+ K/R+KaaV56zK//H2KXtvaypqJ08HUBvBtrT5i96c8kK4ztqghMipvcKhZWX9fsQJsNP1 94CzgNvAozxpi6owGBSp6xjetvMKCzH3wjJJ6oYa71vKneTrv8l/FJ/NSw/LiIg3o+Kp Zu4w== Date: Mon, 30 Nov 2015 14:12:08 -0800 From: Stephen Hemminger Message-ID: <20151130141208.516b1046@xeon-e3> In-Reply-To: <87egf7183c.fsf_-_@x220.int.ebiederm.org> References: <565B7F7D.80208@nod.at> <87egf7183c.fsf_-_@x220.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Bridge] [PATCH net] bridge: Only call /sbin/bridge-stp for the initial network namespace List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Eric W. Biederman" Cc: Kees Cook , "kernel-hardening@lists.openwall.com" , Richard Weinberger , bridge@lists.linux-foundation.org, "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , David Miller On Mon, 30 Nov 2015 15:38:15 -0600 ebiederm@xmission.com (Eric W. Biederman) wrote: > > There is no defined mechanism to pass network namespace information > into /sbin/bridge-stp therefore don't even try to invoke it except > for bridge devices in the initial network namespace. > > It is possible for unprivileged users to cause /sbin/bridge-stp to be > invoked for any network device name which if /sbin/bridge-stp does not > guard against unreasonable arguments or being invoked twice on the same > network device could cause problems. > > Signed-off-by: "Eric W. Biederman" > --- > net/bridge/br_stp_if.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index 5396ff08af32..742fa89528ab 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -142,7 +142,9 @@ static void br_stp_start(struct net_bridge *br) > char *envp[] = { NULL }; > struct net_bridge_port *p; > > - r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); > + r = -ENOENT; > + if (dev_net(br->dev) == &init_net) > + r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); I don't think this will cause loud screams. But it might break people that use containers to run virtual networks for testing. One coding nit: Why are you afraid of using an else? From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com Date: Mon, 30 Nov 2015 14:12:08 -0800 From: Stephen Hemminger Message-ID: <20151130141208.516b1046@xeon-e3> In-Reply-To: <87egf7183c.fsf_-_@x220.int.ebiederm.org> References: <565B7F7D.80208@nod.at> <87egf7183c.fsf_-_@x220.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [kernel-hardening] Re: [PATCH net] bridge: Only call /sbin/bridge-stp for the initial network namespace To: "Eric W. Biederman" Cc: David Miller , Richard Weinberger , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "kernel-hardening@lists.openwall.com" , bridge@lists.linux-foundation.org, Kees Cook List-ID: On Mon, 30 Nov 2015 15:38:15 -0600 ebiederm@xmission.com (Eric W. Biederman) wrote: > > There is no defined mechanism to pass network namespace information > into /sbin/bridge-stp therefore don't even try to invoke it except > for bridge devices in the initial network namespace. > > It is possible for unprivileged users to cause /sbin/bridge-stp to be > invoked for any network device name which if /sbin/bridge-stp does not > guard against unreasonable arguments or being invoked twice on the same > network device could cause problems. > > Signed-off-by: "Eric W. Biederman" > --- > net/bridge/br_stp_if.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index 5396ff08af32..742fa89528ab 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -142,7 +142,9 @@ static void br_stp_start(struct net_bridge *br) > char *envp[] = { NULL }; > struct net_bridge_port *p; > > - r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); > + r = -ENOENT; > + if (dev_net(br->dev) == &init_net) > + r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); I don't think this will cause loud screams. But it might break people that use containers to run virtual networks for testing. One coding nit: Why are you afraid of using an else? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755286AbbK3WMB (ORCPT ); Mon, 30 Nov 2015 17:12:01 -0500 Received: from mail-pa0-f48.google.com ([209.85.220.48]:35159 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754206AbbK3WL7 (ORCPT ); Mon, 30 Nov 2015 17:11:59 -0500 Date: Mon, 30 Nov 2015 14:12:08 -0800 From: Stephen Hemminger To: ebiederm@xmission.com (Eric W. Biederman) Cc: David Miller , Richard Weinberger , "netdev\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" , "kernel-hardening\@lists.openwall.com" , bridge@lists.linux-foundation.org, Kees Cook Subject: Re: [PATCH net] bridge: Only call /sbin/bridge-stp for the initial network namespace Message-ID: <20151130141208.516b1046@xeon-e3> In-Reply-To: <87egf7183c.fsf_-_@x220.int.ebiederm.org> References: <565B7F7D.80208@nod.at> <87egf7183c.fsf_-_@x220.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 30 Nov 2015 15:38:15 -0600 ebiederm@xmission.com (Eric W. Biederman) wrote: > > There is no defined mechanism to pass network namespace information > into /sbin/bridge-stp therefore don't even try to invoke it except > for bridge devices in the initial network namespace. > > It is possible for unprivileged users to cause /sbin/bridge-stp to be > invoked for any network device name which if /sbin/bridge-stp does not > guard against unreasonable arguments or being invoked twice on the same > network device could cause problems. > > Signed-off-by: "Eric W. Biederman" > --- > net/bridge/br_stp_if.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index 5396ff08af32..742fa89528ab 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -142,7 +142,9 @@ static void br_stp_start(struct net_bridge *br) > char *envp[] = { NULL }; > struct net_bridge_port *p; > > - r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); > + r = -ENOENT; > + if (dev_net(br->dev) == &init_net) > + r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); I don't think this will cause loud screams. But it might break people that use containers to run virtual networks for testing. One coding nit: Why are you afraid of using an else? From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH net] bridge: Only call /sbin/bridge-stp for the initial network namespace Date: Mon, 30 Nov 2015 14:12:08 -0800 Message-ID: <20151130141208.516b1046@xeon-e3> References: <565B7F7D.80208@nod.at> <87egf7183c.fsf_-_@x220.int.ebiederm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Kees Cook , "kernel-hardening@lists.openwall.com" , Richard Weinberger , bridge@lists.linux-foundation.org, "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , David Miller To: ebiederm@xmission.com (Eric W. Biederman) Return-path: In-Reply-To: <87egf7183c.fsf_-_@x220.int.ebiederm.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: bridge-bounces@lists.linux-foundation.org Errors-To: bridge-bounces@lists.linux-foundation.org List-Id: netdev.vger.kernel.org On Mon, 30 Nov 2015 15:38:15 -0600 ebiederm@xmission.com (Eric W. Biederman) wrote: > > There is no defined mechanism to pass network namespace information > into /sbin/bridge-stp therefore don't even try to invoke it except > for bridge devices in the initial network namespace. > > It is possible for unprivileged users to cause /sbin/bridge-stp to be > invoked for any network device name which if /sbin/bridge-stp does not > guard against unreasonable arguments or being invoked twice on the same > network device could cause problems. > > Signed-off-by: "Eric W. Biederman" > --- > net/bridge/br_stp_if.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index 5396ff08af32..742fa89528ab 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -142,7 +142,9 @@ static void br_stp_start(struct net_bridge *br) > char *envp[] = { NULL }; > struct net_bridge_port *p; > > - r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); > + r = -ENOENT; > + if (dev_net(br->dev) == &init_net) > + r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC); I don't think this will cause loud screams. But it might break people that use containers to run virtual networks for testing. One coding nit: Why are you afraid of using an else?