From owner-freebsd-hackers@FreeBSD.ORG Sun Jan 9 22:58:11 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F22E3106566C for ; Sun, 9 Jan 2011 22:58:11 +0000 (UTC) (envelope-from joris.dedieu@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id 831568FC18 for ; Sun, 9 Jan 2011 22:58:11 +0000 (UTC) Received: by fxm16 with SMTP id 16so18376886fxm.13 for ; Sun, 09 Jan 2011 14:58:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:content-type :content-transfer-encoding; bh=LecgpzlWkwbNILo9+YzNQ6AGOt3R0fbjLLFMR3+P7fg=; b=qV1QC6bQ/YjALC8qeI71tia3hgHtrTfxCsZx8mhgQYzJ+aDVdATAN2m9W/C55XQ4hZ c2YEhwOR9goXuN7cin1kfPTEiP3+LktwKauX5qeF9tIlrZF/rza+itTeoD9lZ/XNfpfO mXx7BtVJq8zPt9wddLeS80MOLndFx3asqwnaE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:content-transfer-encoding; b=Qa/2YuNttfG0h2yi/cgW70UkPEsfobVa73hUedS1jjVsmGxHXRMl0Nd+F3aVw9jkRj xfsBkVPlshSsQ7PCkkMpCK+ht6AJGywh8+Q8S3kr5bZzrRPQ4YhbSrOCxyhm70F+n2f6 AGTQxS1vVMsskWXG2cnfu5XHEKvYdIcEfDN20= MIME-Version: 1.0 Received: by 10.223.100.5 with SMTP id w5mr11917658fan.20.1294613888430; Sun, 09 Jan 2011 14:58:08 -0800 (PST) Received: by 10.223.86.207 with HTTP; Sun, 9 Jan 2011 14:58:08 -0800 (PST) In-Reply-To: References: Date: Sun, 9 Jan 2011 23:58:08 +0100 Message-ID: From: joris dedieu To: freebsd-hackers Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: binding non local ip. X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 Jan 2011 22:58:12 -0000 2011/1/9 Eygene Ryabinkin : Sorry for my mail client broken that do not send mails to the list :) I'll take care. > Joris, good day. > > Sun, Jan 09, 2011 at 06:29:20PM +0100, joris dedieu wrote: >> Thanks Eygene for this greate review ! > > No problems ;)) > >> 2011/1/7 Eygene Ryabinkin : >> > Fri, Jan 07, 2011 at 01:57:21PM +0100, joris dedieu wrote: >> >> What do you think about it ? >> > [...] >> >> +static int =A0 =A0 bindany =3D 0; /* 1 allows to bind a non local ip= */ >> >> +SYSCTL_INT(_net_inet_ip, OID_AUTO, bindany, CTLFLAG_RW, &bindany, 0, >> >> + =A0 =A0"Allow to bind a non local ip"); >> > >> > On at least 8.x, you will likely want to use VNET_* macros to enable >> > your new sysctl to be virtualized. =A0Something like this: >> > {{{ >> > VNET_DEFINE(int, inp_bindany) =3D 0; >> > SYSCTL_VNET_INT(_net_inet_ip, OID_AUTO, bindany, CTLFLAG_RW, >> > =A0 =A0 =A0 =A0&VNET_NAME(inp_bindany), 0, "Force INP_BINDANY on all s= ockets"); >> > }}} >> > and use VNET(inp_bindany) in subsequent code. >> Ok it make sense. I will use VNET_*. There are a lot of SYSCTL_* in >> netinet and netinet6. Is changing this for VNET_* an open task? > > I think that the most of them that are applicable to VNET were > already converted. =A0It is better to ask at freebsd-net@freebsd.org. > >> Greate. It makes me understand the way a lot of things are written. >> Avoid branching if you can. >> I see that OPSET macro in netinet/ip_output.c lock the inp struct. Is >> there a need of it there ? > > Yes. =A0I had overlooked the need of locking here, sorry. I wrote a better patch that avoid locking and inp struct modification. diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index d742887..f41e4da 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -321,6 +321,9 @@ in_pcbbind(struct inpcb *inp, struct sockaddr *nam, struct ucred *cred) * * On error, the values of *laddrp and *lportp are not changed. */ +VNET_DEFINE(int, inp_bindany) =3D 0; +SYSCTL_VNET_INT(_net_inet_ip, OID_AUTO, bindany, CTLFLAG_RW, + &VNET_NAME(inp_bindany), 0, "Force INP_BINDANY on all sockets"); int in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam, in_addr_t *laddr= p, u_short *lportp, struct ucred *cred) @@ -392,7 +395,8 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam, in_addr_t *laddrp, * If INP_BINDANY is set, then the socket may be bo= und * to any endpoint address, local or not. */ - if ((inp->inp_flags & INP_BINDANY) =3D=3D 0 && + if (VNET(inp_bindany) =3D=3D 0 && + (inp->inp_flags & INP_BINDANY) =3D=3D 0 && ifa_ifwithaddr_check((struct sockaddr *)sin) = =3D=3D 0) return (EADDRNOTAVAIL); } diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h index 4ba19e6..3720121 100644 --- a/sys/netinet/in_pcb.h +++ b/sys/netinet/in_pcb.h @@ -467,6 +467,7 @@ VNET_DECLARE(int, ipport_randomcps); VNET_DECLARE(int, ipport_randomtime); VNET_DECLARE(int, ipport_stoprandom); VNET_DECLARE(int, ipport_tcpallocs); +VNET_DECLARE(int, inp_bindany); #define V_ipport_reservedhigh VNET(ipport_reservedhigh) #define V_ipport_reservedlow VNET(ipport_reservedlow) diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index c91d4a9..17a2e78 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -897,6 +897,7 @@ rip_bind(struct socket *so, struct sockaddr *nam, struct thread *td) if (TAILQ_EMPTY(&V_ifnet) || (addr->sin_family !=3D AF_INET && addr->sin_family !=3D AF_IMPL= INK) || (addr->sin_addr.s_addr && + VNET(inp_bindany) =3D=3D 0 && (inp->inp_flags & INP_BINDANY) =3D=3D 0 && ifa_ifwithaddr_check((struct sockaddr *)addr) =3D=3D 0)) return (EADDRNOTAVAIL); > >> Do you mean there is a way to control user input (ie 0 or 42, but >> nothing else)? > > No, I meant that if you'll use the custom sysctl value handler (via > SYSCTL_VNET_PROC, not vie SYSCTL_VNET_INT), then you can convert any > non-zero value to INP_BINDANY and zero to zero. =A0But given the need of > locking, I don't think that this won't be good to take this road: one > simple non-conditional logical instruction will be harmless even if it > is executed when it is not needed; but the block of > lock-logicalop-unlock will be worse. > -- > Eygene Ryabinkin =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0,,,^..^,,, > [ Life's unfair - but root password helps! =A0 =A0 =A0 =A0 =A0 | codelabs= .ru ] > [ 82FE 06BC D497 C0DE 49EC =A04FF0 16AF 9EAE 8152 ECFB | freebsd.org ] >