From owner-freebsd-hackers@FreeBSD.ORG Sun Jan 9 22:58:11 2011
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
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 <freebsd-hackers@freebsd.org>; 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 <freebsd-hackers@freebsd.org>; Sun, 9 Jan 2011 22:58:11 +0000 (UTC)
Received: by fxm16 with SMTP id 16so18376886fxm.13
for <freebsd-hackers@freebsd.org>; 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: <whUqbEc0GlwM0C01ICP8qimcjRQ@hXkfnLhK1nGJouHuoYJSDAyzreM>
References: <AANLkTimJBkTdgs4P=XjHyTCinfCOn0Ku8bEVcR-q=Dzc@mail.gmail.com>
<I0E3jOweM2JFYO1RRyeZ3hfTJPY@5UW8B3LwPEGInFShts5A7R/S2Yo>
<AANLkTinHZNWDwS+2fiLfuDUAjOHCHrO4acR8RxZVpudm@mail.gmail.com>
<whUqbEc0GlwM0C01ICP8qimcjRQ@hXkfnLhK1nGJouHuoYJSDAyzreM>
Date: Sun, 9 Jan 2011 23:58:08 +0100
Message-ID: <AANLkTikBkEw5XU75hX-2gvqiA-JWhqh6pxiMM_UEJdMM@mail.gmail.com>
From: joris dedieu <joris.dedieu@gmail.com>
To: freebsd-hackers <freebsd-hackers@freebsd.org>
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
<freebsd-hackers.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
<mailto:freebsd-hackers-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-hackers>
List-Post: <mailto:freebsd-hackers@freebsd.org>
List-Help: <mailto:freebsd-hackers-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
<mailto:freebsd-hackers-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sun, 09 Jan 2011 22:58:12 -0000
2011/1/9 Eygene Ryabinkin <rea@freebsd.org>:
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 <rea@freebsd.org>:
>> > 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 ]
>