2011/1/9 Eygene Ryabinkin <r...@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 <r...@freebsd.org>:
>> > Fri, Jan 07, 2011 at 01:57:21PM +0100, joris dedieu wrote:
>> >> What do you think about it ?
>> > [...]
>> >> +static int bindany = 0; /* 1 allows to bind a non local ip */
>> >> +SYSCTL_INT(_net_inet_ip, OID_AUTO, bindany, CTLFLAG_RW, &bindany, 0,
>> >> + "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. Something like this:
>> > {{{
>> > VNET_DEFINE(int, inp_bindany) = 0;
>> > SYSCTL_VNET_INT(_net_inet_ip, OID_AUTO, bindany, CTLFLAG_RW,
>> > &VNET_NAME(inp_bindany), 0, "Force INP_BINDANY on all sockets");
>> > }}}
>> > 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. It is better to ask at freebsd-...@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. I 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) = 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 *laddrp,
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 bound
* to any endpoint address, local or not.
*/
- if ((inp->inp_flags & INP_BINDANY) == 0 &&
+ if (VNET(inp_bindany) == 0 &&
+ (inp->inp_flags & INP_BINDANY) == 0 &&
ifa_ifwithaddr_check((struct sockaddr *)sin) == 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 != AF_INET && addr->sin_family != AF_IMPLINK) ||
(addr->sin_addr.s_addr &&
+ VNET(inp_bindany) == 0 &&
(inp->inp_flags & INP_BINDANY) == 0 &&
ifa_ifwithaddr_check((struct sockaddr *)addr) == 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. But 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 ,,,^..^,,,
> [ Life's unfair - but root password helps! | codelabs.ru ]
> [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ]
>
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
