tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Question about in_pcbbind() and in6_pcbbind()



Elad Efrat wrote:
Hi,

There are three issues that I'm wondering about regarding the two
functions mentioned in the subject:

1. When called from netinet/tcp_usrreq.c:tcp_usrrreq(), in_pcbbind()
   takes "l", but in6_pcbbind() takes NULL:
[...]

We now pass always pass the lwp.

2. When called from the same locations, you'll also notice the second
   argument (mbuf *nam) is NULL. Passing NULL is functionally equivalent
   to passing a zeroed struct sockaddr_in6 wrapped in an mbuf, with one
   difference: there's no sockaddr struct to pass to kauth(9). (We could
   just pass the address and the port, but I think we'll be wasting
   parameter space that way.)
[...]

I decided to slice in6_pcbbind(), and attached is the result (for
netinet6 only). It:

  - Keeps in6_pcbbind()'s semantics,
  - Introduces in6_pcbbind_{addr,port}()
  - Uses "dom_sa_any" when the passed argument is NULL

(we could probably use some KASSERT()s in the two new functions to make
sure we don't end up *there* with sin6 == NULL, etc.)

3. It seems that in_pcbbind(), when exhausting all free ports when
   auto-assigning a port, does the following:
[...]

This is fixed in the attached patch as well.

I've looked at both the FreeBSD and OpenBSD equivalents and it seems
that FreeBSD does address this issue by resetting the address "any",
but it does it in in6_pcbsetport(), which is also called from udp6_output(), and I'm not sure resetting the laddr on the in6pcb is
intended (or desired?) in such a context -- so I reset the address if
in6_pcbsetport() fails in in6_pcbbind() itself.

Please review. :)

Thanks,

-e.
Index: in6_pcb.c
===================================================================
RCS file: /usr/cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.102
diff -u -p -r1.102 in6_pcb.c
--- in6_pcb.c   14 Apr 2009 21:25:20 -0000      1.102
+++ in6_pcb.c   16 Apr 2009 21:06:02 -0000
@@ -79,6 +79,7 @@ __KERNEL_RCSID(0, "$NetBSD: in6_pcb.c,v 
 #include <sys/time.h>
 #include <sys/proc.h>
 #include <sys/kauth.h>
+#include <sys/domain.h>
 
 #include <net/if.h>
 #include <net/route.h>
@@ -183,144 +184,147 @@ in6_pcballoc(struct socket *so, void *v)
        return (0);
 }
 
+/*
+ * Bind address from sin6 to in6p.
+ */
 int
-in6_pcbbind(void *v, struct mbuf *nam, struct lwp *l)
+in6_pcbbind_addr(struct in6pcb *in6p, struct sockaddr_in6 *sin6, struct lwp *l)
 {
-       struct in6pcb *in6p = v;
-       struct socket *so = in6p->in6p_socket;
-       struct inpcbtable *table = in6p->in6p_table;
-       struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)NULL;
-       u_int16_t lport = 0;
-       int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
+       int error;
 
-       if (in6p->in6p_af != AF_INET6)
-               return (EINVAL);
+       /*
+        * We should check the family, but old programs
+        * incorrectly fail to intialize it.
+        */
+       if (sin6->sin6_family != AF_INET6)
+               return (EAFNOSUPPORT);
 
-       if (in6p->in6p_lport || !IN6_IS_ADDR_UNSPECIFIED(&in6p->in6p_laddr))
-               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 = 1;
-       if (nam) {
-               int error;
+#ifndef INET
+       if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr))
+               return (EADDRNOTAVAIL);
+#endif
 
-               sin6 = mtod(nam, struct sockaddr_in6 *);
-               if (nam->m_len != sizeof(*sin6))
+       if ((error = sa6_embedscope(sin6, ip6_use_defzone)) != 0)
+               return (error);
+
+       if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) {
+               if ((in6p->in6p_flags & IN6P_IPV6_V6ONLY) != 0)
                        return (EINVAL);
+               if (sin6->sin6_addr.s6_addr32[3]) {
+                       struct sockaddr_in sin;
+
+                       memset(&sin, 0, sizeof(sin));
+                       sin.sin_len = sizeof(sin);
+                       sin.sin_family = AF_INET;
+                       bcopy(&sin6->sin6_addr.s6_addr32[3],
+                           &sin.sin_addr, sizeof(sin.sin_addr));
+                       if (ifa_ifwithaddr((struct sockaddr *)&sin) == 0)
+                               return EADDRNOTAVAIL;
+               }
+       } else if (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) {
+               struct ifaddr *ia = NULL;
+
+               if ((in6p->in6p_flags & IN6P_FAITH) == 0 &&
+                   (ia = ifa_ifwithaddr((struct sockaddr *)sin6)) == 0)
+                       return (EADDRNOTAVAIL);
+
                /*
-                * We should check the family, but old programs
-                * incorrectly fail to intialize it.
+                * 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 (sin6->sin6_family != AF_INET6)
-                       return (EAFNOSUPPORT);
-
-#ifndef INET
-               if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr))
+               if (ia &&
+                   ((struct in6_ifaddr *)ia)->ia6_flags &
+                   (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED))
                        return (EADDRNOTAVAIL);
-#endif
+       }
 
-               if ((error = sa6_embedscope(sin6, ip6_use_defzone)) != 0)
-                       return (error);
 
-               lport = sin6->sin6_port;
-               if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) {
-                       /*
-                        * Treat SO_REUSEADDR as SO_REUSEPORT for multicast;
-                        * allow compepte 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)
-                               reuseport = SO_REUSEADDR|SO_REUSEPORT;
-               } else if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) {
-                       if ((in6p->in6p_flags & IN6P_IPV6_V6ONLY) != 0)
-                               return (EINVAL);
-                       if (sin6->sin6_addr.s6_addr32[3]) {
-                               struct sockaddr_in sin;
-
-                               memset(&sin, 0, sizeof(sin));
-                               sin.sin_len = sizeof(sin);
-                               sin.sin_family = AF_INET;
-                               bcopy(&sin6->sin6_addr.s6_addr32[3],
-                                   &sin.sin_addr, sizeof(sin.sin_addr));
-                               if (ifa_ifwithaddr((struct sockaddr *)&sin) == 
0)
-                                       return EADDRNOTAVAIL;
-                       }
-               } else if (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) {
-                       struct ifaddr *ia = NULL;
+       in6p->in6p_laddr = sin6->sin6_addr;
 
-                       if ((in6p->in6p_flags & IN6P_FAITH) == 0 &&
-                           (ia = ifa_ifwithaddr((struct sockaddr *)sin6)) == 0)
-                               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 (ia &&
-                           ((struct in6_ifaddr *)ia)->ia6_flags &
-                           (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED))
-                               return (EADDRNOTAVAIL);
-               }
-               if (lport) {
+       return (0);
+}
+
+/*
+ * Bind port from sin6 to in6p.
+ */
+int
+in6_pcbbind_port(struct in6pcb *in6p, struct sockaddr_in6 *sin6, struct lwp *l)
+{
+       struct inpcbtable *table = in6p->in6p_table;
+       struct socket *so = in6p->in6p_socket;
+       int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
+
+       if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) == 0 &&
+          ((so->so_proto->pr_flags & PR_CONNREQUIRED) == 0 ||
+           (so->so_options & SO_ACCEPTCONN) == 0))
+               wild = 1;
+
 #ifndef IPNOPRIVPORTS
-                       int priv;
+       int priv;
 
-                       /*
-                        * NOTE: all operating systems use suser() for
-                        * privilege check!  do not rewrite it into SS_PRIV.
-                        */
-                       priv = (l && !kauth_authorize_generic(l->l_cred,
-                           KAUTH_GENERIC_ISSUSER, NULL)) ? 1 : 0;
-                       /* GROSS */
-                       if (ntohs(lport) < IPV6PORT_RESERVED && !priv)
-                               return (EACCES);
+       /*
+        * NOTE: all operating systems use suser() for
+        * privilege check!  do not rewrite it into SS_PRIV.
+        */
+       priv = (l && !kauth_authorize_generic(l->l_cred,
+           KAUTH_GENERIC_ISSUSER, NULL)) ? 1 : 0;
+       /* GROSS */
+       if (ntohs(sin6->sin6_port) < IPV6PORT_RESERVED && !priv)
+               return (EACCES);
 #endif
 
-                       if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) {
+       if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) {
+               /*
+                * Treat SO_REUSEADDR as SO_REUSEPORT for multicast;
+                * allow compepte 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)
+                       reuseport = SO_REUSEADDR|SO_REUSEPORT;
+       }
+
+       if (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) {
 #ifdef INET
-                               struct inpcb *t;
+               struct inpcb *t;
 
-                               t = in_pcblookup_port(table,
-                                   *(struct in_addr 
*)&sin6->sin6_addr.s6_addr32[3],
-                                   lport, wild);
-                               if (t && (reuseport & 
t->inp_socket->so_options) == 0)
-                                       return (EADDRINUSE);
+               t = in_pcblookup_port(table,
+                   *(struct in_addr *)&sin6->sin6_addr.s6_addr32[3],
+                   sin6->sin6_port, wild);
+               if (t && (reuseport & t->inp_socket->so_options) == 0)
+                       return (EADDRINUSE);
 #else
-                               return (EADDRNOTAVAIL);
+               return (EADDRNOTAVAIL);
 #endif
-                       }
+       }
 
-                       {
-                               struct in6pcb *t;
+       {
+               struct in6pcb *t;
 
-                               t = in6_pcblookup_port(table, &sin6->sin6_addr,
-                                   lport, wild);
-                               if (t && (reuseport & 
t->in6p_socket->so_options) == 0)
-                                       return (EADDRINUSE);
-                       }
-               }
-               in6p->in6p_laddr = sin6->sin6_addr;
+               t = in6_pcblookup_port(table, &sin6->sin6_addr,
+                   sin6->sin6_port, wild);
+               if (t && (reuseport & t->in6p_socket->so_options) == 0)
+                       return (EADDRINUSE);
        }
 
-       if (lport == 0) {
+       if (sin6->sin6_port == 0) {
                int e;
                e = in6_pcbsetport(&in6p->in6p_laddr, in6p, l);
                if (e != 0)
                        return (e);
        } else {
-               in6p->in6p_lport = lport;
+               in6p->in6p_lport = sin6->sin6_port;
                in6_pcbstate(in6p, IN6P_BOUND);
        }
 
@@ -328,6 +332,55 @@ in6_pcbbind(void *v, struct mbuf *nam, s
        LIST_INSERT_HEAD(IN6PCBHASH_PORT(table, in6p->in6p_lport),
            &in6p->in6p_head, inph_lhash);
 
+       return (0);
+}
+
+int
+in6_pcbbind(void *v, struct mbuf *nam, struct lwp *l)
+{
+       struct in6pcb *in6p = v;
+       struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)NULL;
+       int error;
+
+       if (in6p->in6p_af != AF_INET6)
+               return (EINVAL);
+
+       /*
+        * If we already have a local port or a local address it means we're
+        * bounded.
+        */
+       if (in6p->in6p_lport || !IN6_IS_ADDR_UNSPECIFIED(&in6p->in6p_laddr))
+               return (EINVAL);
+
+       if (nam != NULL) {
+               /* We were provided a sockaddr_in6 to use. */
+               sin6 = mtod(nam, struct sockaddr_in6 *);
+               if (nam->m_len != sizeof(*sin6))
+                       return (EINVAL);
+       } else {
+               /* We always bind to *something*, even if it's "anything". */
+               sin6 = (struct sockaddr_in6 *)
+                   
__UNCONST(in6p->in6p_socket->so_proto->pr_domain->dom_sa_any);
+       }
+
+       /* Bind address. */
+       error = in6_pcbbind_addr(in6p, sin6, l);
+       if (error)
+               return (error);
+
+       /* Bind port. */
+       error = in6_pcbbind_port(in6p, sin6, l);
+       if (error) {
+               /*
+                * Reset the address here to "any" so we don't "leak" the
+                * in6pcb.
+                */
+               in6p->in6p_laddr = in6addr_any;
+
+               return (error);
+       }
+
+
 #if 0
        in6p->in6p_flowinfo = 0;        /* XXX */
 #endif
Index: in6_pcb.h
===================================================================
RCS file: /usr/cvs/src/sys/netinet6/in6_pcb.h,v
retrieving revision 1.32
diff -u -p -r1.32 in6_pcb.h
--- in6_pcb.h   2 May 2007 20:40:26 -0000       1.32
+++ in6_pcb.h   16 Apr 2009 20:41:05 -0000
@@ -153,6 +153,8 @@ void        in6_losing(struct in6pcb *);
 void   in6_pcbinit(struct inpcbtable *, int, int);
 int    in6_pcballoc(struct socket *, void *);
 int    in6_pcbbind(void *, struct mbuf *, struct lwp *);
+int    in6_pcbbind_addr(struct in6pcb *, struct sockaddr_in6 *, struct lwp *);
+int    in6_pcbbind_port(struct in6pcb *, struct sockaddr_in6 *, struct lwp *);
 int    in6_pcbconnect(void *, struct mbuf *, struct lwp *);
 void   in6_pcbdetach(struct in6pcb *);
 void   in6_pcbdisconnect(struct in6pcb *);


Home | Main Index | Thread Index | Old Index