Source-Changes-HG archive

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

[src/trunk]: src/sys/netinet6 Fix race condition of in6_selectsrc



details:   https://anonhg.NetBSD.org/src/rev/ca4ac1ce2304
branches:  trunk
changeset: 348626:ca4ac1ce2304
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Mon Oct 31 04:16:25 2016 +0000

description:
Fix race condition of in6_selectsrc

in6_selectsrc returned a pointer to in6_addr that wan't guaranteed to be
safe by pserialize (or psref), which was racy. Let callers pass a pointer
to in6_addr and in6_selectsrc copy a result to it inside pserialize
critical sections.

diffstat:

 sys/netinet6/icmp6.c       |  26 +++++++++++++++-----------
 sys/netinet6/in6_pcb.c     |  15 ++++++++-------
 sys/netinet6/in6_src.c     |  38 +++++++++++++++++---------------------
 sys/netinet6/ip6_var.h     |   8 ++++----
 sys/netinet6/nd6_nbr.c     |  19 ++++++++++---------
 sys/netinet6/raw_ip6.c     |  27 ++++++++++-----------------
 sys/netinet6/udp6_output.c |  12 +++++++-----
 7 files changed, 71 insertions(+), 74 deletions(-)

diffs (truncated from 484 to 300 lines):

diff -r 34c32bd2486c -r ca4ac1ce2304 sys/netinet6/icmp6.c
--- a/sys/netinet6/icmp6.c      Mon Oct 31 04:15:22 2016 +0000
+++ b/sys/netinet6/icmp6.c      Mon Oct 31 04:16:25 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: icmp6.c,v 1.199 2016/10/25 02:45:10 ozaki-r Exp $      */
+/*     $NetBSD: icmp6.c,v 1.200 2016/10/31 04:16:25 ozaki-r Exp $      */
 /*     $KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $ */
 
 /*
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: icmp6.c,v 1.199 2016/10/25 02:45:10 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: icmp6.c,v 1.200 2016/10/31 04:16:25 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1999,9 +1999,9 @@
        int type, code;
        struct ifnet *outif = NULL;
        struct in6_addr origdst;
-       const struct in6_addr *src = NULL;
        struct ifnet *rcvif;
        int s;
+       bool ip6_src_filled = false;
 
        /* too short to reflect */
        if (off < sizeof(struct ip6_hdr)) {
@@ -2069,8 +2069,10 @@
                ;
        else if ((ip6a = ip6_getdstifaddr(m)) != NULL) {
                if ((ip6a->ip6a_flags &
-                    (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY)) == 0)
-                       src = &ip6a->ip6a_src;
+                    (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY)) == 0) {
+                       ip6->ip6_src = ip6a->ip6a_src;
+                       ip6_src_filled = true;
+               }
        } else {
                union {
                        struct sockaddr_in6 sin6;
@@ -2087,13 +2089,15 @@
                if (ifa != NULL) {
                        ia = ifatoia6(ifa);
                        if ((ia->ia6_flags &
-                                (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY)) == 0)
-                               src = &ia->ia_addr.sin6_addr;
+                                (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY)) == 0) {
+                               ip6->ip6_src = ia->ia_addr.sin6_addr;
+                               ip6_src_filled = true;
+                       }
                }
                pserialize_read_exit(_s);
        }
 
-       if (src == NULL) {
+       if (!ip6_src_filled) {
                int e;
                struct sockaddr_in6 sin6;
                struct route ro;
@@ -2107,9 +2111,10 @@
                sockaddr_in6_init(&sin6, &ip6->ip6_dst, 0, 0, 0);
 
                memset(&ro, 0, sizeof(ro));
-               src = in6_selectsrc(&sin6, NULL, NULL, &ro, NULL, NULL, NULL, &e);
+               e = in6_selectsrc(&sin6, NULL, NULL, &ro, NULL, NULL, NULL,
+                   &ip6->ip6_src);
                rtcache_free(&ro);
-               if (src == NULL) {
+               if (e != 0) {
                        nd6log(LOG_DEBUG,
                            "source can't be determined: "
                            "dst=%s, error=%d\n",
@@ -2118,7 +2123,6 @@
                }
        }
 
-       ip6->ip6_src = *src;
        ip6->ip6_flow = 0;
        ip6->ip6_vfc &= ~IPV6_VERSION_MASK;
        ip6->ip6_vfc |= IPV6_VERSION;
diff -r 34c32bd2486c -r ca4ac1ce2304 sys/netinet6/in6_pcb.c
--- a/sys/netinet6/in6_pcb.c    Mon Oct 31 04:15:22 2016 +0000
+++ b/sys/netinet6/in6_pcb.c    Mon Oct 31 04:16:25 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in6_pcb.c,v 1.150 2016/09/29 12:19:47 roy Exp $        */
+/*     $NetBSD: in6_pcb.c,v 1.151 2016/10/31 04:16:25 ozaki-r Exp $    */
 /*     $KAME: in6_pcb.c,v 1.84 2001/02/08 18:02:08 itojun Exp $        */
 
 /*
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in6_pcb.c,v 1.150 2016/09/29 12:19:47 roy Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6_pcb.c,v 1.151 2016/10/31 04:16:25 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -444,6 +444,7 @@
 {
        struct in6pcb *in6p = v;
        struct in6_addr *in6a = NULL;
+       struct in6_addr ia6;
        struct ifnet *ifp = NULL;       /* outgoing interface */
        int error = 0;
        int scope_ambiguous = 0;
@@ -530,10 +531,9 @@
                 * with the address specified by setsockopt(IPV6_PKTINFO).
                 * Is it the intended behavior?
                 */
-               in6a = in6_selectsrc(sin6, in6p->in6p_outputopts,
-                                    in6p->in6p_moptions,
-                                    &in6p->in6p_route,
-                                    &in6p->in6p_laddr, &ifp, &psref, &error);
+               error = in6_selectsrc(sin6, in6p->in6p_outputopts,
+                   in6p->in6p_moptions, &in6p->in6p_route, &in6p->in6p_laddr,
+                   &ifp, &psref, &ia6);
                if (ifp && scope_ambiguous &&
                    (error = in6_setscope(&sin6->sin6_addr, ifp, NULL)) != 0) {
                        if_put(ifp, &psref);
@@ -541,13 +541,14 @@
                        return(error);
                }
 
-               if (in6a == NULL) {
+               if (error != 0) {
                        if_put(ifp, &psref);
                        curlwp_bindx(bound);
                        if (error == 0)
                                error = EADDRNOTAVAIL;
                        return (error);
                }
+               in6a = &ia6;
        }
 
        if (ifp != NULL) {
diff -r 34c32bd2486c -r ca4ac1ce2304 sys/netinet6/in6_src.c
--- a/sys/netinet6/in6_src.c    Mon Oct 31 04:15:22 2016 +0000
+++ b/sys/netinet6/in6_src.c    Mon Oct 31 04:16:25 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: in6_src.c,v 1.71 2016/10/31 02:50:31 ozaki-r Exp $     */
+/*     $NetBSD: in6_src.c,v 1.72 2016/10/31 04:16:25 ozaki-r Exp $     */
 /*     $KAME: in6_src.c,v 1.159 2005/10/19 01:40:32 t-momose Exp $     */
 
 /*
@@ -66,7 +66,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in6_src.c,v 1.71 2016/10/31 02:50:31 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6_src.c,v 1.72 2016/10/31 04:16:25 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -171,10 +171,10 @@
 #define BREAK(r) goto out
 #endif
 
-struct in6_addr *
+int
 in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, 
        struct ip6_moptions *mopts, struct route *ro, struct in6_addr *laddr, 
-       struct ifnet **ifpp, struct psref *psref, int *errorp)
+       struct ifnet **ifpp, struct psref *psref, struct in6_addr *ret_ia6)
 {
        struct in6_addr dst;
        struct ifnet *ifp = NULL;
@@ -189,7 +189,6 @@
        u_int8_t ip6po_usecoa = 0;
 #endif /* MIP6 && NMIP > 0 */
        struct psref local_psref;
-       struct in6_addr *ret_ia = NULL;
        int bound = curlwp_bind();
 #define PSREF (psref == NULL) ? &local_psref : psref
        int s;
@@ -198,7 +197,6 @@
                (ifpp == NULL && psref == NULL));
 
        dst = dstsock->sin6_addr; /* make a copy for local operation */
-       *errorp = 0;
        if (ifpp)
                *ifpp = NULL;
 
@@ -238,8 +236,8 @@
                srcsock.sin6_len = sizeof(srcsock);
                srcsock.sin6_addr = pi->ipi6_addr;
                if (ifp) {
-                       *errorp = in6_setscope(&srcsock.sin6_addr, ifp, NULL);
-                       if (*errorp != 0)
+                       error = in6_setscope(&srcsock.sin6_addr, ifp, NULL);
+                       if (error != 0)
                                goto exit;
                }
 
@@ -249,15 +247,14 @@
                    ia6->ia6_flags &
                    (IN6_IFF_ANYCAST | IN6_IFF_NOTREADY)) {
                        pserialize_read_exit(_s);
-                       *errorp = EADDRNOTAVAIL;
+                       error = EADDRNOTAVAIL;
                        goto exit;
                }
                pi->ipi6_addr = srcsock.sin6_addr; /* XXX: this overrides pi */
                if (ifpp)
                        *ifpp = ifp;
-               ret_ia = &ia6->ia_addr.sin6_addr;
+               *ret_ia6 = ia6->ia_addr.sin6_addr;
                pserialize_read_exit(_s);
-               /* XXX don't return pointer */
                goto exit;
        }
 
@@ -267,7 +264,7 @@
         * though it would eventually cause an error.
         */
        if (laddr && !IN6_IS_ADDR_UNSPECIFIED(laddr)) {
-               ret_ia = laddr;
+               *ret_ia6 = *laddr;
                goto exit;
        }
 
@@ -275,10 +272,8 @@
         * The outgoing interface is crucial in the general selection procedure
         * below.  If it is not known at this point, we fail.
         */
-       if (ifp == NULL) {
-               *errorp = error;
+       if (ifp == NULL)
                goto exit;
-       }
 
        /*
         * If the address is not yet determined, choose the best one based on
@@ -297,8 +292,8 @@
        }
 #endif /* MIP6 && NMIP > 0 */
 
-       *errorp = in6_setscope(&dst, ifp, &odstzone);
-       if (*errorp != 0)
+       error = in6_setscope(&dst, ifp, &odstzone);
+       if (error != 0)
                goto exit;
 
        s = pserialize_read_enter();
@@ -560,19 +555,20 @@
          out:
                break;
        }
-       pserialize_read_exit(s);
 
        if ((ia = ia_best) == NULL) {
-               *errorp = EADDRNOTAVAIL;
+               pserialize_read_exit(s);
+               error = EADDRNOTAVAIL;
                goto exit;
        }
 
-       ret_ia = &ia->ia_addr.sin6_addr;
+       *ret_ia6 = ia->ia_addr.sin6_addr;
+       pserialize_read_exit(s);
 exit:
        if (ifpp == NULL)
                if_put(ifp, PSREF);
        curlwp_bindx(bound);
-       return ret_ia;
+       return error;
 #undef PSREF
 }
 #undef REPLACE
diff -r 34c32bd2486c -r ca4ac1ce2304 sys/netinet6/ip6_var.h
--- a/sys/netinet6/ip6_var.h    Mon Oct 31 04:15:22 2016 +0000
+++ b/sys/netinet6/ip6_var.h    Mon Oct 31 04:16:25 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ip6_var.h,v 1.68 2016/08/23 09:59:20 knakahara Exp $   */
+/*     $NetBSD: ip6_var.h,v 1.69 2016/10/31 04:16:25 ozaki-r Exp $     */
 /*     $KAME: ip6_var.h,v 1.33 2000/06/11 14:59:20 jinmei Exp $        */
 
 /*
@@ -396,9 +396,9 @@
 
 struct route;
 
-struct         in6_addr *in6_selectsrc(struct sockaddr_in6 *,
-       struct ip6_pktopts *, struct ip6_moptions *, struct route *,
-       struct in6_addr *, struct ifnet **, struct psref *, int *);
+int    in6_selectsrc(struct sockaddr_in6 *, struct ip6_pktopts *,
+          struct ip6_moptions *, struct route *, struct in6_addr *,
+          struct ifnet **, struct psref *, struct in6_addr *);
 int in6_selectroute(struct sockaddr_in6 *, struct ip6_pktopts *,
        struct ip6_moptions *, struct route *, struct ifnet **,
        struct psref *, struct rtentry **, int);
diff -r 34c32bd2486c -r ca4ac1ce2304 sys/netinet6/nd6_nbr.c
--- a/sys/netinet6/nd6_nbr.c    Mon Oct 31 04:15:22 2016 +0000
+++ b/sys/netinet6/nd6_nbr.c    Mon Oct 31 04:16:25 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nd6_nbr.c,v 1.128 2016/10/18 07:30:31 ozaki-r Exp $    */
+/*     $NetBSD: nd6_nbr.c,v 1.129 2016/10/31 04:16:25 ozaki-r Exp $    */
 /*     $KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $        */



Home | Main Index | Thread Index | Old Index