Although the sysctls controlling the port range are labelled "port(hi)?first"
and
"port(hi)?last", no ordering is enforced and you can have portfirst > portlast.
in_pcbbind() (and in6_pcbsetport()) work around this by duplicating the loop
looking
for an available port.
This diff introduce temporary bounds and compare them to guarantee that
first <= last, thus allowing deduplication of the port scan loop.
Tested on my laptop with a narrow port range and heavy cheezburger browsing, no
fault
detected. Deeper testing welcome.
Should I include in6_pcbsetport() changes right now or should ipv4 be validated
first ?
--
Vincent Gross
Index: netinet/in_pcb.c
===================================================================
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.180
diff -u -p -r1.180 in_pcb.c
--- netinet/in_pcb.c 22 Sep 2015 09:34:38 -0000 1.180
+++ netinet/in_pcb.c 1 Oct 2015 09:47:16 -0000
@@ -360,67 +360,43 @@ in_pcbbind(struct inpcb *inp, struct mbu
inp->inp_laddr = sin->sin_addr;
}
if (lport == 0) {
- u_int16_t first, last;
+ u_int16_t bound_a, bound_b, first, last;
int count;
if (inp->inp_flags & INP_HIGHPORT) {
- first = ipport_hifirstauto; /* sysctl */
- last = ipport_hilastauto;
+ bound_a = ipport_hifirstauto; /* sysctl */
+ bound_b = ipport_hilastauto;
} else if (inp->inp_flags & INP_LOWPORT) {
if ((error = suser(p, 0)))
return (EACCES);
- first = IPPORT_RESERVED-1; /* 1023 */
- last = 600; /* not IPPORT_RESERVED/2 */
+ bound_a = IPPORT_RESERVED-1; /* 1023 */
+ bound_b = 600; /* not IPPORT_RESERVED/2 */
} else {
- first = ipport_firstauto; /* sysctl */
- last = ipport_lastauto;
+ bound_a = ipport_firstauto; /* sysctl */
+ bound_b = ipport_lastauto;
}
-
- /*
- * Simple check to ensure all ports are not used up causing
- * a deadlock here.
- *
- * We split the two cases (up and down) so that the direction
- * is not being tested on each round of the loop.
- */
-
- if (first > last) {
- /*
- * counting down
- */
- count = first - last;
- if (count)
- lastport = first - arc4random_uniform(count);
-
- do {
- if (count-- < 0) /* completely used? */
- return (EADDRNOTAVAIL);
- --lastport;
- if (lastport > first || lastport < last)
- lastport = first;
- lport = htons(lastport);
- } while (in_baddynamic(lastport,
so->so_proto->pr_protocol) ||
- in_pcblookup(table, &zeroin_addr, 0,
- &inp->inp_laddr, lport, wild, inp->inp_rtableid));
+ if (bound_a < bound_b) {
+ first = bound_a;
+ last = bound_b;
} else {
- /*
- * counting up
- */
- count = last - first;
- if (count)
- lastport = first + arc4random_uniform(count);
-
- do {
- if (count-- < 0) /* completely used? */
- return (EADDRNOTAVAIL);
- ++lastport;
- if (lastport < first || lastport > last)
- lastport = first;
- lport = htons(lastport);
- } while (in_baddynamic(lastport,
so->so_proto->pr_protocol) ||
- in_pcblookup(table, &zeroin_addr, 0,
- &inp->inp_laddr, lport, wild, inp->inp_rtableid));
+ first = bound_b;
+ last = bound_a;
}
+ /* first <= last */
+
+ count = last - first;
+ lastport = first + arc4random_uniform(count);
+
+ do {
+ if (count-- < 0) /* completely used? */
+ return (EADDRNOTAVAIL);
+ ++lastport;
+ if (lastport < first || lastport > last)
+ lastport = first;
+ lport = htons(lastport);
+ } while (in_baddynamic(lastport, so->so_proto->pr_protocol) ||
+ in_pcblookup(table, &zeroin_addr, 0,
+ &inp->inp_laddr, lport, wild, inp->inp_rtableid));
}
inp->inp_lport = lport;
in_pcbrehash(inp);