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);

Reply via email to