From d4b509bd7fb914aa7f0e4031ceb55a6afa5739de Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Thu, 4 Nov 2004 01:25:23 +0000 Subject: [PATCH] Until this change, the UDP input code used global variables udp_in, udp_in6, and udp_ip6 to pass socket address state between udp_input(), udp_append(), and soappendaddr_locked(). While file in the default configuration, when running with multiple netisrs or direct ithread dispatch, this can result in races wherein user processes using recvmsg() get back the wrong source IP/port. To correct this and related races: - Eliminate udp_ip6, which is believed to be generated but then never used. Eliminate ip_2_ip6_hdr() as it is now unneeded. - Eliminate setting, testing, and existence of 'init' status fields for the IPv6 structures. While with multiple UDP delivery this could lead to amortization of IPv4 -> IPv6 conversion when delivering an IPv4 UDP packet to an IPv6 socket, it added substantial complexity and side effects. - Move global structures into the stack, declaring udp_in in udp_input(), and udp_in6 in udp_append() to be used if a conversion is required. Pass &udp_in into udp_append(). - Re-annotate comments to reflect updates. With this change, UDP appears to operate correctly in the presence of substantial inbound processing parallelism. This solution avoids introducing additional synchronization, but does increase the potential stack depth. Discovered by: kris (Bug Magnet) MFC after: 3 weeks --- sys/netinet/udp_usrreq.c | 81 ++++++++++++---------------------------- 1 file changed, 24 insertions(+), 57 deletions(-) diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 26a804da42f4..d808f7bfac15 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -120,26 +120,8 @@ struct udpstat udpstat; /* from udp_var.h */ SYSCTL_STRUCT(_net_inet_udp, UDPCTL_STATS, stats, CTLFLAG_RW, &udpstat, udpstat, "UDP statistics (struct udpstat, netinet/udp_var.h)"); -static struct sockaddr_in udp_in = { sizeof(udp_in), AF_INET }; -#ifdef INET6 -struct udp_in6 { - struct sockaddr_in6 uin6_sin; - u_char uin6_init_done : 1; -} udp_in6 = { - { sizeof(udp_in6.uin6_sin), AF_INET6 }, - 0 -}; -struct udp_ip6 { - struct ip6_hdr uip6_ip6; - u_char uip6_init_done : 1; -} udp_ip6; -#endif /* INET6 */ - static void udp_append(struct inpcb *last, struct ip *ip, struct mbuf *n, - int off); -#ifdef INET6 -static void ip_2_ip6_hdr(struct ip6_hdr *ip6, struct ip *ip); -#endif + int off, struct sockaddr_in *udp_in); static int udp_detach(struct socket *so); static int udp_output(struct inpcb *, struct mbuf *, struct sockaddr *, @@ -171,6 +153,7 @@ udp_input(m, off) struct mbuf *opts = 0; int len; struct ip save_ip; + struct sockaddr_in udp_in; udpstat.udps_ipackets++; @@ -206,11 +189,11 @@ udp_input(m, off) * Construct sockaddr format source address. * Stuff source address and datagram in user buffer. */ + bzero(&udp_in, sizeof(udp_in)); + udp_in.sin_len = sizeof(udp_in); + udp_in.sin_family = AF_INET; udp_in.sin_port = uh->uh_sport; udp_in.sin_addr = ip->ip_src; -#ifdef INET6 - udp_in6.uin6_init_done = udp_ip6.uip6_init_done = 0; -#endif /* * Make mbuf data length reflect UDP length. @@ -338,7 +321,8 @@ udp_input(m, off) if (n != NULL) udp_append(last, ip, n, iphlen + - sizeof(struct udphdr)); + sizeof(struct udphdr), + &udp_in); INP_UNLOCK(last); } last = inp; @@ -363,7 +347,8 @@ udp_input(m, off) udpstat.udps_noportbcast++; goto badheadlocked; } - udp_append(last, ip, m, iphlen + sizeof(struct udphdr)); + udp_append(last, ip, m, iphlen + sizeof(struct udphdr), + &udp_in); INP_UNLOCK(last); INP_INFO_RUNLOCK(&udbinfo); return; @@ -399,7 +384,7 @@ udp_input(m, off) return; } INP_LOCK(inp); - udp_append(inp, ip, m, iphlen + sizeof(struct udphdr)); + udp_append(inp, ip, m, iphlen + sizeof(struct udphdr), &udp_in); INP_UNLOCK(inp); INP_INFO_RUNLOCK(&udbinfo); return; @@ -415,39 +400,25 @@ udp_input(m, off) return; } -#ifdef INET6 -static void -ip_2_ip6_hdr(ip6, ip) - struct ip6_hdr *ip6; - struct ip *ip; -{ - bzero(ip6, sizeof(*ip6)); - - ip6->ip6_vfc = IPV6_VERSION; - ip6->ip6_plen = ip->ip_len; - ip6->ip6_nxt = ip->ip_p; - ip6->ip6_hlim = ip->ip_ttl; - ip6->ip6_src.s6_addr32[2] = ip6->ip6_dst.s6_addr32[2] = - IPV6_ADDR_INT32_SMP; - ip6->ip6_src.s6_addr32[3] = ip->ip_src.s_addr; - ip6->ip6_dst.s6_addr32[3] = ip->ip_dst.s_addr; -} -#endif - /* - * subroutine of udp_input(), mainly for source code readability. - * caller must properly init udp_ip6 and udp_in6 beforehand. + * Subroutine of udp_input(), which appends the provided mbuf chain to the + * passed pcb/socket. The caller must provide a sockaddr_in via udp_in that + * contains the source address. If the socket ends up being an IPv6 socket, + * udp_append() will convert to a sockaddr_in6 before passing the address + * into the socket code. */ static void -udp_append(last, ip, n, off) +udp_append(last, ip, n, off, udp_in) struct inpcb *last; struct ip *ip; struct mbuf *n; int off; + struct sockaddr_in *udp_in; { struct sockaddr *append_sa; struct socket *so; struct mbuf *opts = 0; + struct sockaddr_in6 udp_in6; INP_LOCK_ASSERT(last); @@ -473,10 +444,6 @@ udp_append(last, ip, n, off) if (last->inp_vflag & INP_IPV6) { int savedflags; - if (udp_ip6.uip6_init_done == 0) { - ip_2_ip6_hdr(&udp_ip6.uip6_ip6, ip); - udp_ip6.uip6_init_done = 1; - } savedflags = last->inp_flags; last->inp_flags &= ~INP_UNMAPPABLEOPTS; ip6_savecontrol(last, n, &opts); @@ -487,14 +454,14 @@ udp_append(last, ip, n, off) } #ifdef INET6 if (last->inp_vflag & INP_IPV6) { - if (udp_in6.uin6_init_done == 0) { - in6_sin_2_v4mapsin6(&udp_in, &udp_in6.uin6_sin); - udp_in6.uin6_init_done = 1; - } - append_sa = (struct sockaddr *)&udp_in6.uin6_sin; + bzero(&udp_in6, sizeof(udp_in6)); + udp_in6.sin6_len = sizeof(udp_in6); + udp_in6.sin6_family = AF_INET6; + in6_sin_2_v4mapsin6(udp_in, &udp_in6); + append_sa = (struct sockaddr *)&udp_in6; } else #endif - append_sa = (struct sockaddr *)&udp_in; + append_sa = (struct sockaddr *)udp_in; m_adj(n, off); so = last->inp_socket;