in_pcbbind and in6_pcbbind have a lot in common, the only meaningful
differences are in the checks done to ensure a sockaddr is available.

This diff splits theses checks in their own functions, and merge the
remaining code in one single function. Aside from being easier to read,
it also makes it very easy to check sockaddr availability without
actually binding.

Tested on my own laptop for the last ten days ; no regression observed
with regress/sys/netinet/in_pcbbind.

Ok ?

Index: netinet/in_pcb.c
===================================================================
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.195
diff -u -p -r1.195 in_pcb.c
--- netinet/in_pcb.c    18 Dec 2015 22:25:16 -0000      1.195
+++ netinet/in_pcb.c    23 Dec 2015 08:07:14 -0000
@@ -284,91 +284,129 @@ int
 in_pcbbind(struct inpcb *inp, struct mbuf *nam, struct proc *p)
 {
        struct socket *so = inp->inp_socket;
-       struct inpcbtable *table = inp->inp_table;
-       struct sockaddr_in *sin;
        u_int16_t lport = 0;
-       int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
+       int wild = 0;
        int error;
 
-#ifdef INET6
-       if (sotopf(so) == PF_INET6)
-               return in6_pcbbind(inp, nam, p);
-#endif /* INET6 */
-
-       if (inp->inp_lport || inp->inp_laddr.s_addr != INADDR_ANY)
+       if (inp->inp_lport != 0)
                return (EINVAL);
+
        if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) == 0 &&
            ((so->so_proto->pr_flags & PR_CONNREQUIRED) == 0 ||
             (so->so_options & SO_ACCEPTCONN) == 0))
                wild = INPLOOKUP_WILDCARD;
+
        if (nam) {
-               sin = mtod(nam, struct sockaddr_in *);
-               if (nam->m_len != sizeof(*sin))
+               switch (sotopf(so)) {
+#ifdef INET6
+               case PF_INET6: {
+                       struct sockaddr_in6 *sin6;
+
+                       if (TAILQ_EMPTY(&in6_ifaddr))
+                               return (EADDRNOTAVAIL);
+                       if (!IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6))
+                               return (EINVAL);
+
+                       sin6 = mtod(nam, struct sockaddr_in6 *);
+                       if (nam->m_len != sizeof(*sin6))
+                               return (EINVAL);
+                       if (sin6->sin6_family != AF_INET6)
+                               return (EAFNOSUPPORT);
+                       if ((error = in6_pcbaddrisavail(inp, sin6, wild, p)))
+                               return (error);
+                       inp->inp_laddr6 = sin6->sin6_addr;
+                       lport = sin6->sin6_port;
+                       break;
+               }
+#endif
+               case PF_INET: {
+                       struct sockaddr_in *sin;
+
+                       if (inp->inp_laddr.s_addr != INADDR_ANY)
+                               return (EINVAL);
+
+                       sin = mtod(nam, struct sockaddr_in *);
+                       if (nam->m_len != sizeof(*sin))
+                               return (EINVAL);
+                       if (sin->sin_family != AF_INET)
+                               return (EAFNOSUPPORT);
+                       if ((error = in_pcbaddrisavail(inp, sin, wild, p)))
+                               return (error);
+                       inp->inp_laddr = sin->sin_addr;
+                       lport = sin->sin_port;
+                       break;
+               }
+               default:
                        return (EINVAL);
+               }
+       }
+
+       if (lport == 0)
+               if ((error = in_pcbpickport(&lport, wild, inp, p)))
+                       return (error);
+       inp->inp_lport = lport;
+       in_pcbrehash(inp);
+       return (0);
+}
+
+int
+in_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in *sin, int wild,
+    struct proc *p)
+{
+       struct socket *so = inp->inp_socket;
+       struct inpcbtable *table = inp->inp_table;
+       u_int16_t lport = sin->sin_port;
+       int reuseport = (so->so_options & SO_REUSEPORT);
+       int error;
 
-               if (sin->sin_family != AF_INET)
-                       return (EAFNOSUPPORT);
+       if (IN_MULTICAST(sin->sin_addr.s_addr)) {
+               /*
+                * Treat SO_REUSEADDR as SO_REUSEPORT for multicast;
+                * allow complete duplication of binding if
+                * SO_REUSEPORT is set, or if SO_REUSEADDR is set
+                * and a multicast address is bound on both
+                * new and duplicated sockets.
+                */
+               if (so->so_options & (SO_REUSEADDR|SO_REUSEPORT))
+                       reuseport = SO_REUSEADDR|SO_REUSEPORT;
+       } else if (sin->sin_addr.s_addr != INADDR_ANY) {
+
+               if ((so->so_options & SO_BINDANY) == 0 ||
+                   (so->so_type != SOCK_DGRAM) ||
+                   (sin->sin_addr.s_addr != INADDR_BROADCAST &&
+                    !in_broadcast(sin->sin_addr, inp->inp_rtableid))) {
+                       struct ifaddr *ifa;
 
-               lport = sin->sin_port;
-               if (IN_MULTICAST(sin->sin_addr.s_addr)) {
-                       /*
-                        * Treat SO_REUSEADDR as SO_REUSEPORT for multicast;
-                        * allow complete duplication of binding if
-                        * SO_REUSEPORT is set, or if SO_REUSEADDR is set
-                        * and a multicast address is bound on both
-                        * new and duplicated sockets.
-                        */
-                       if (so->so_options & (SO_REUSEADDR|SO_REUSEPORT))
-                               reuseport = SO_REUSEADDR|SO_REUSEPORT;
-               } else if (sin->sin_addr.s_addr != INADDR_ANY) {
-                       sin->sin_port = 0;              /* yech... */
-                       /* ... must also clear the zeropad in the sockaddr */
+                       /* ifa_ifwithaddr bcmp the sockaddrs ... yech */
+                       sin->sin_port = 0;
                        memset(sin->sin_zero, 0, sizeof(sin->sin_zero));
+                       ifa = ifa_ifwithaddr(sintosa(sin), inp->inp_rtableid);
+                       sin->sin_port = lport;
 
-                       if (!((so->so_options & SO_BINDANY) ||
-                           (sin->sin_addr.s_addr == INADDR_BROADCAST &&
-                            so->so_type == SOCK_DGRAM))) {
-                               struct in_ifaddr *ia;
-
-                               ia = ifatoia(ifa_ifwithaddr(sintosa(sin),
-                                   inp->inp_rtableid));
-
-                               /* SOCK_RAW does not use in_pcbbind() */
-                               if (ia == NULL &&
-                                   (so->so_type != SOCK_DGRAM ||
-                                   !in_broadcast(sin->sin_addr,
-                                       inp->inp_rtableid)))
-                                               return (EADDRNOTAVAIL);
-                       }
+                       if (ifa == NULL)
+                               return (EADDRNOTAVAIL);
                }
-               if (lport) {
-                       struct inpcb *t;
+       }
+       if (lport) {
+               struct inpcb *t;
 
-                       /* GROSS */
-                       if (ntohs(lport) < IPPORT_RESERVED &&
-                           (error = suser(p, 0)))
-                               return (EACCES);
-                       if (so->so_euid) {
-                               t = in_pcblookup(table, &zeroin_addr, 0,
-                                   &sin->sin_addr, lport, INPLOOKUP_WILDCARD,
-                                   inp->inp_rtableid);
-                               if (t &&
-                                   (so->so_euid != t->inp_socket->so_euid))
-                                       return (EADDRINUSE);
-                       }
+               /* GROSS */
+               if (ntohs(lport) < IPPORT_RESERVED &&
+                   (error = suser(p, 0)))
+                       return (EACCES);
+               if (so->so_euid) {
                        t = in_pcblookup(table, &zeroin_addr, 0,
-                           &sin->sin_addr, lport, wild, inp->inp_rtableid);
-                       if (t && (reuseport & t->inp_socket->so_options) == 0)
+                           &sin->sin_addr, lport, INPLOOKUP_WILDCARD,
+                           inp->inp_rtableid);
+                       if (t && (so->so_euid != t->inp_socket->so_euid))
                                return (EADDRINUSE);
                }
-               inp->inp_laddr = sin->sin_addr;
+               t = in_pcblookup(table, &zeroin_addr, 0,
+                   &sin->sin_addr, lport, wild, inp->inp_rtableid);
+               if (t && (reuseport & t->inp_socket->so_options) == 0)
+                       return (EADDRINUSE);
        }
 
-       if (lport == 0)
-               if ((error = in_pcbpickport(&lport, wild, inp, p)))
-                       return (error);
-       inp->inp_lport = lport;
-       in_pcbrehash(inp);
        return (0);
 }
 
@@ -408,9 +446,11 @@ in_pcbpickport(u_int16_t *lport, int wil
 
        count = higher - lower;
        candidate = lower + arc4random_uniform(count);
+#ifdef INET6
        if (sotopf(so) == PF_INET6)
                laddr = &inp->inp_laddr6;
        else
+#endif
                laddr = &inp->inp_laddr;
 
        do {
Index: netinet/in_pcb.h
===================================================================
RCS file: /cvs/src/sys/netinet/in_pcb.h,v
retrieving revision 1.93
diff -u -p -r1.93 in_pcb.h
--- netinet/in_pcb.h    3 Dec 2015 10:34:24 -0000       1.93
+++ netinet/in_pcb.h    23 Dec 2015 08:07:14 -0000
@@ -244,6 +244,8 @@ extern struct baddynamicports baddynamic
 void    in_losing(struct inpcb *);
 int     in_pcballoc(struct socket *, struct inpcbtable *);
 int     in_pcbbind(struct inpcb *, struct mbuf *, struct proc *);
+int     in_pcbaddrisavail(struct inpcb *, struct sockaddr_in *, int,
+           struct proc *);
 int     in_pcbconnect(struct inpcb *, struct mbuf *);
 void    in_pcbdetach(struct inpcb *);
 void    in_pcbdisconnect(struct inpcb *);
@@ -261,7 +263,8 @@ struct inpcb *
         in6_pcblookup_listen(struct inpcbtable *,
                               struct in6_addr *, u_int, int, struct mbuf *,
                               u_int);
-int     in6_pcbbind(struct inpcb *, struct mbuf *, struct proc *);
+int     in6_pcbaddrisavail(struct inpcb *, struct sockaddr_in6 *, int,
+           struct proc *);
 int     in6_pcbconnect(struct inpcb *, struct mbuf *);
 int     in6_setsockaddr(struct inpcb *, struct mbuf *);
 int     in6_setpeeraddr(struct inpcb *, struct mbuf *);
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.128
diff -u -p -r1.128 tcp_usrreq.c
--- netinet/tcp_usrreq.c        11 Sep 2015 07:42:35 -0000      1.128
+++ netinet/tcp_usrreq.c        23 Dec 2015 08:07:14 -0000
@@ -220,14 +220,7 @@ tcp_usrreq(so, req, m, nam, control, p)
         * Give the socket an address.
         */
        case PRU_BIND:
-#ifdef INET6
-               if (inp->inp_flags & INP_IPV6)
-                       error = in6_pcbbind(inp, nam, p);
-               else
-#endif
-                       error = in_pcbbind(inp, nam, p);
-               if (error)
-                       break;
+               error = in_pcbbind(inp, nam, p);
                break;
 
        /*
@@ -235,12 +228,7 @@ tcp_usrreq(so, req, m, nam, control, p)
         */
        case PRU_LISTEN:
                if (inp->inp_lport == 0) {
-#ifdef INET6
-                       if (inp->inp_flags & INP_IPV6)
-                               error = in6_pcbbind(inp, NULL, p);
-                       else
-#endif
-                               error = in_pcbbind(inp, NULL, p);
+                       error = in_pcbbind(inp, NULL, p);
                }
                /* If the in_pcbbind() above is called, the tp->pf
                   should still be whatever it was before. */
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.208
diff -u -p -r1.208 udp_usrreq.c
--- netinet/udp_usrreq.c        3 Dec 2015 14:05:28 -0000       1.208
+++ netinet/udp_usrreq.c        23 Dec 2015 08:07:14 -0000
@@ -1094,12 +1094,7 @@ udp_usrreq(struct socket *so, int req, s
                break;
 
        case PRU_BIND:
-#ifdef INET6
-               if (inp->inp_flags & INP_IPV6)
-                       error = in6_pcbbind(inp, addr, p);
-               else
-#endif
-                       error = in_pcbbind(inp, addr, p);
+               error = in_pcbbind(inp, addr, p);
                break;
 
        case PRU_LISTEN:
Index: netinet6/in6_pcb.c
===================================================================
RCS file: /cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.84
diff -u -p -r1.84 in6_pcb.c
--- netinet6/in6_pcb.c  18 Dec 2015 22:25:16 -0000      1.84
+++ netinet6/in6_pcb.c  23 Dec 2015 08:07:14 -0000
@@ -150,138 +150,95 @@ u_char inet6ctlerrmap[PRC_NCMDS] = {
 };
 #endif
 
-/*
- * Bind an address (or at least a port) to an PF_INET6 socket.
- */
 int
-in6_pcbbind(struct inpcb *inp, struct mbuf *nam, struct proc *p)
+in6_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in6 *sin6, int wild,
+    struct proc *p)
 {
        struct socket *so = inp->inp_socket;
-
-       struct inpcbtable *head = inp->inp_table;
-       struct sockaddr_in6 *sin6;
-       u_short lport = 0;
-       int wild = INPLOOKUP_IPV6, reuseport = (so->so_options & SO_REUSEPORT);
+       struct inpcbtable *table = inp->inp_table;
+       u_short lport = sin6->sin6_port;
+       int reuseport = (so->so_options & SO_REUSEPORT);
        int error;
 
-       /*
-        * REMINDER:  Once up to speed, flow label processing should go here,
-        * too.  (Same with in6_pcbconnect.)
-        */
-       if (TAILQ_EMPTY(&in6_ifaddr))
+       wild |= INPLOOKUP_IPV6;
+       /* KAME hack: embed scopeid */
+       if (in6_embedscope(&sin6->sin6_addr, sin6, inp) != 0)
+               return EINVAL;
+       /* this must be cleared for ifa_ifwithaddr() */
+       sin6->sin6_scope_id = 0;
+       /* reject IPv4 mapped address, we have no support for it */
+       if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr))
                return EADDRNOTAVAIL;
 
-       if (inp->inp_lport != 0 || !IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6))
-               return EINVAL;  /* If already bound, EINVAL! */
-
-       if ((so->so_options & (SO_REUSEADDR | SO_REUSEPORT)) == 0 &&
-           ((so->so_proto->pr_flags & PR_CONNREQUIRED) == 0 ||
-            (so->so_options & SO_ACCEPTCONN) == 0))
-               wild |= INPLOOKUP_WILDCARD;
-
-       /*
-        * If I did get a sockaddr passed in...
-        */
-       if (nam) {
-               sin6 = mtod(nam, struct sockaddr_in6 *);
-               if (nam->m_len != sizeof (*sin6))
-                       return EINVAL;
-
+       if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) {
                /*
-                * Unlike v4, I have no qualms about EAFNOSUPPORT if the
-                * wretched family is not filled in!
+                * Treat SO_REUSEADDR as SO_REUSEPORT for multicast;
+                * allow complete duplication of binding if
+                * SO_REUSEPORT is set, or if SO_REUSEADDR is set
+                * and a multicast address is bound on both
+                * new and duplicated sockets.
                 */
-               if (sin6->sin6_family != AF_INET6)
-                       return EAFNOSUPPORT;
-
-               /* KAME hack: embed scopeid */
-               if (in6_embedscope(&sin6->sin6_addr, sin6, inp) != 0)
-                       return EINVAL;
-               /* this must be cleared for ifa_ifwithaddr() */
-               sin6->sin6_scope_id = 0;
-
-               lport = sin6->sin6_port;
-
-               /* reject IPv4 mapped address, we have no support for it */
-               if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr))
+               if (so->so_options & (SO_REUSEADDR|SO_REUSEPORT))
+                       reuseport = SO_REUSEADDR | SO_REUSEPORT;
+       } else if (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) {
+               struct ifaddr *ifa = NULL;
+
+               sin6->sin6_port = 0;  /*
+                                      * Yechhhh, because of upcoming
+                                      * call to ifa_ifwithaddr(), which
+                                      * does bcmp's over the PORTS as
+                                      * well.  (What about flow?)
+                                      */
+               sin6->sin6_flowinfo = 0;
+               if (!(so->so_options & SO_BINDANY) &&
+                   (ifa = ifa_ifwithaddr(sin6tosa(sin6),
+                   inp->inp_rtableid)) == NULL)
                        return EADDRNOTAVAIL;
+               sin6->sin6_port = lport;
 
-               if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) {
-                       /*
-                        * Treat SO_REUSEADDR as SO_REUSEPORT for multicast;
-                        * allow complete duplication of binding if
-                        * SO_REUSEPORT is set, or if SO_REUSEADDR is set
-                        * and a multicast address is bound on both
-                        * new and duplicated sockets.
-                        */
-                       if (so->so_options & (SO_REUSEADDR|SO_REUSEPORT))
-                               reuseport = SO_REUSEADDR | SO_REUSEPORT;
-               } else if (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) {
-                       struct ifaddr *ifa = NULL;
-
-                       sin6->sin6_port = 0;  /*
-                                              * Yechhhh, because of upcoming
-                                              * call to ifa_ifwithaddr(), which
-                                              * does bcmp's over the PORTS as
-                                              * well.  (What about flow?)
-                                              */
-                       sin6->sin6_flowinfo = 0;
-                       if (!(so->so_options & SO_BINDANY) &&
-                           (ifa = ifa_ifwithaddr(sin6tosa(sin6),
-                           inp->inp_rtableid)) == NULL)
-                               return EADDRNOTAVAIL;
-
-                       /*
-                        * bind to an anycast address might accidentally
-                        * cause sending a packet with an anycast source
-                        * address, so we forbid it.
-                        *
-                        * We should allow to bind to a deprecated address,
-                        * since the application dare to use it.
-                        * But, can we assume that they are careful enough
-                        * to check if the address is deprecated or not?
-                        * Maybe, as a safeguard, we should have a setsockopt
-                        * flag to control the bind(2) behavior against
-                        * deprecated addresses (default: forbid bind(2)).
-                        */
-                       if (ifa &&
-                           ifatoia6(ifa)->ia6_flags &
-                           (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED))
-                               return (EADDRNOTAVAIL);
-               }
-               if (lport) {
-                       struct inpcb *t;
-
-                       /*
-                        * Question:  Do we wish to continue the Berkeley
-                        * tradition of ports < IPPORT_RESERVED be only for
-                        * root?
-                        * Answer: For now yes, but IMHO, it should be REMOVED!
-                        * OUCH: One other thing, is there no better way of
-                        * finding a process for a socket instead of using
-                        * curproc?  (Marked with BSD's {in,}famous XXX ?
-                        */
-                       if (ntohs(lport) < IPPORT_RESERVED &&
-                           (error = suser(p, 0)))
-                               return error;
-
-                       t = in_pcblookup(head,
-                           (struct in_addr *)&zeroin6_addr, 0,
-                           (struct in_addr *)&sin6->sin6_addr, lport,
-                           wild, inp->inp_rtableid);
-
-                       if (t && (reuseport & t->inp_socket->so_options) == 0)
-                               return EADDRINUSE;
-               }
-               inp->inp_laddr6 = sin6->sin6_addr;
+               /*
+                * bind to an anycast address might accidentally
+                * cause sending a packet with an anycast source
+                * address, so we forbid it.
+                *
+                * We should allow to bind to a deprecated address,
+                * since the application dare to use it.
+                * But, can we assume that they are careful enough
+                * to check if the address is deprecated or not?
+                * Maybe, as a safeguard, we should have a setsockopt
+                * flag to control the bind(2) behavior against
+                * deprecated addresses (default: forbid bind(2)).
+                */
+               if (ifa &&
+                   ifatoia6(ifa)->ia6_flags &
+                   (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED))
+                       return (EADDRNOTAVAIL);
+       }
+       if (lport) {
+               struct inpcb *t;
+
+               /*
+                * Question:  Do we wish to continue the Berkeley
+                * tradition of ports < IPPORT_RESERVED be only for
+                * root?
+                * Answer: For now yes, but IMHO, it should be REMOVED!
+                * OUCH: One other thing, is there no better way of
+                * finding a process for a socket instead of using
+                * curproc?  (Marked with BSD's {in,}famous XXX ?
+                */
+               if (ntohs(lport) < IPPORT_RESERVED &&
+                   (error = suser(p, 0)))
+                       return error;
+
+               t = in_pcblookup(table,
+                   (struct in_addr *)&zeroin6_addr, 0,
+                   (struct in_addr *)&sin6->sin6_addr, lport,
+                   wild, inp->inp_rtableid);
+               if (t && (reuseport & t->inp_socket->so_options) == 0)
+                       return EADDRINUSE;
        }
 
-       if (lport == 0)
-               if ((error = in_pcbpickport(&lport, wild, inp, p)))
-                       return (error);
-       inp->inp_lport = lport;
-       in_pcbrehash(inp);
-       return 0;
+       return (0);
 }
 
 /*
@@ -351,7 +308,7 @@ in6_pcbconnect(struct inpcb *inp, struct
 
        if (IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6)) {
                if (inp->inp_lport == 0 &&
-                   in6_pcbbind(inp, NULL, curproc) == EADDRNOTAVAIL)
+                   in_pcbbind(inp, NULL, curproc) == EADDRNOTAVAIL)
                        return (EADDRNOTAVAIL);
                inp->inp_laddr6 = *in6a;
        }
Index: netinet6/ip6_divert.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.38
diff -u -p -r1.38 ip6_divert.c
--- netinet6/ip6_divert.c       11 Sep 2015 08:17:06 -0000      1.38
+++ netinet6/ip6_divert.c       23 Dec 2015 08:07:14 -0000
@@ -292,7 +292,7 @@ divert6_usrreq(struct socket *so, int re
 
        case PRU_BIND:
                s = splsoftnet();
-               error = in6_pcbbind(inp, addr, p);
+               error = in_pcbbind(inp, addr, p);
                splx(s);
                break;
 
Index: netinet6/udp6_output.c
===================================================================
RCS file: /cvs/src/sys/netinet6/udp6_output.c,v
retrieving revision 1.41
diff -u -p -r1.41 udp6_output.c
--- netinet6/udp6_output.c      2 Dec 2015 22:13:44 -0000       1.41
+++ netinet6/udp6_output.c      23 Dec 2015 08:07:14 -0000
@@ -162,7 +162,7 @@ udp6_output(struct inpcb *in6p, struct m
 
                if (in6p->inp_lport == 0){
                        int s = splsoftnet();
-                       error = in6_pcbbind(in6p, NULL, p);
+                       error = in_pcbbind(in6p, NULL, p);
                        splx(s);
                        if (error)
                                goto release;

Reply via email to