From 9b37b73b7edc1562017d1fc8f163ffaaba59d5a3 Mon Sep 17 00:00:00 2001 From: Sepherosa Ziehau Date: Wed, 26 Mar 2008 14:44:59 +0000 Subject: [PATCH] Fix bugs concerning cached route entry in UDP inpcb. For an unconnected and unbound UDP socket, first sending calls in_pcbladdr() to fix the local port, which may change the target CPU of the next sending. in_pcbladdr() has a side effect to allocate the route entry cached in inpcb. If the target CPU after in_pcbladdr() is no longer the current CPU, then the route entry will be accessed/freed on non-owner CPU during later sending. Similarly, connect/disconnect a UDP socket may change the target CPU too; the target CPU may no longer the owner of the cached route entry. So, for the first sending happens on an unconnected and unbound UDP socket, the target CPU of next sending is compared with the current CPU. If they are different, then cached route entry will be freed, so next time a packet sent on this socket, a new route entry owned by the correct CPU will be cached. Same target CPU check is applied to UDP socket connect/disconnect. Originally UDP PRU_CONNECT always happens on CPU0, which will cause problem if following conditions are met: - Dst of the cached route entry is different from the dst to be connected - Cached route entry is not allocated on CPU0 This could happen if two datagram are sent on an unbounded and unconnected UDP socket, then later connectting this UDP socket will cause cached route entry being freed on different CPU. To solve this problem, PRU_CONNECT is dispatched according to existing [lf]{addr,port} pairs. If in_pcbladdr() fails after altering the cached route entry, the cached route entry is freed to make sure that freeing this cached route entry happens on its owner CPU. Reported-by: y0netan1@ Tested-by: y0netan1@ --- sys/netinet/in_pcb.c | 25 ++++++++++++++------ sys/netinet/ip_demux.c | 6 ++--- sys/netinet/udp_usrreq.c | 60 ++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index 23fa1b5bb5..afaa40f5c4 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -65,7 +65,7 @@ * * @(#)in_pcb.c 8.4 (Berkeley) 5/24/95 * $FreeBSD: src/sys/netinet/in_pcb.c,v 1.59.2.27 2004/01/02 04:06:42 ambrisko Exp $ - * $DragonFly: src/sys/netinet/in_pcb.c,v 1.44 2007/12/19 11:10:42 sephe Exp $ + * $DragonFly: src/sys/netinet/in_pcb.c,v 1.45 2008/03/26 14:44:59 sephe Exp $ */ #include "opt_ipsec.h" @@ -441,7 +441,7 @@ in_pcbladdr(struct inpcb *inp, struct sockaddr *nam, struct ucred *cred = NULL; struct sockaddr_in *sin = (struct sockaddr_in *)nam; struct sockaddr *jsin; - int jailed = 0; + int jailed = 0, alloc_route = 0; if (nam->sa_len != sizeof *sin) return (EINVAL); @@ -498,6 +498,7 @@ in_pcbladdr(struct inpcb *inp, struct sockaddr *nam, ((struct sockaddr_in *) &ro->ro_dst)->sin_addr = sin->sin_addr; rtalloc(ro); + alloc_route = 1; } /* * If we found a route, use the address @@ -536,7 +537,7 @@ in_pcbladdr(struct inpcb *inp, struct sockaddr *nam, ia = NULL; if (!jailed && ia == NULL) - return (EADDRNOTAVAIL); + goto fail; } /* * If the destination address is multicast and an outgoing @@ -555,7 +556,7 @@ in_pcbladdr(struct inpcb *inp, struct sockaddr *nam, if (ia->ia_ifp == ifp) break; if (ia == NULL) - return (EADDRNOTAVAIL); + goto fail; } } /* @@ -564,16 +565,26 @@ in_pcbladdr(struct inpcb *inp, struct sockaddr *nam, */ if (ia == NULL && jailed) { if ((jsin = prison_get_nonlocal(cred->cr_prison, AF_INET, NULL)) != NULL || - (jsin = prison_get_local(cred->cr_prison, AF_INET, NULL)) != NULL) + (jsin = prison_get_local(cred->cr_prison, AF_INET, NULL)) != NULL) { *plocal_sin = satosin(jsin); - else + } else { /* IPv6 only Jail */ - return (EADDRNOTAVAIL); + goto fail; + } } else { *plocal_sin = &ia->ia_addr; } } return (0); +fail: + if (alloc_route) { + struct route *ro = &inp->inp_route; + + if (ro->ro_rt != NULL) + RTFREE(ro->ro_rt); + bzero(ro, sizeof(*ro)); + } + return (EADDRNOTAVAIL); } /* diff --git a/sys/netinet/ip_demux.c b/sys/netinet/ip_demux.c index d808e8b7fb..6912894f70 100644 --- a/sys/netinet/ip_demux.c +++ b/sys/netinet/ip_demux.c @@ -30,7 +30,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/netinet/ip_demux.c,v 1.37 2007/12/19 11:00:22 sephe Exp $ + * $DragonFly: src/sys/netinet/ip_demux.c,v 1.38 2008/03/26 14:44:59 sephe Exp $ */ #include "opt_inet.h" @@ -319,11 +319,11 @@ udp_soport(struct socket *so, struct sockaddr *nam __unused, /* * The following processing all take place on Protocol Thread 0: - * bind() and connect() + * bind() * attach() has a null socket parameter * Fast and slow timeouts pass in null socket parameter */ - if (req == PRU_CONNECT || req == PRU_BIND || so == NULL) + if (req == PRU_BIND || so == NULL) return (&udp_thread[0].td_msgport); inp = so->so_pcb; diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index c62adfa1b2..1496bfffd9 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -65,7 +65,7 @@ * * @(#)udp_usrreq.c 8.6 (Berkeley) 5/23/95 * $FreeBSD: src/sys/netinet/udp_usrreq.c,v 1.64.2.18 2003/01/24 05:11:34 sam Exp $ - * $DragonFly: src/sys/netinet/udp_usrreq.c,v 1.42 2007/04/22 01:13:14 dillon Exp $ + * $DragonFly: src/sys/netinet/udp_usrreq.c,v 1.43 2008/03/26 14:44:59 sephe Exp $ */ #include "opt_ipsec.h" @@ -671,7 +671,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct sockaddr *dstaddr, struct udpiphdr *ui; int len = m->m_pkthdr.len; struct sockaddr_in *sin; /* really is initialized before use */ - int error = 0; + int error = 0, lport_any = 0; if (len + sizeof(struct udpiphdr) > IP_MAXPACKET) { error = EMSGSIZE; @@ -683,6 +683,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct sockaddr *dstaddr, if (error) goto release; in_pcbinswildcardhash(inp); + lport_any = 1; } if (dstaddr != NULL) { /* destination address specified */ @@ -782,6 +783,22 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct sockaddr *dstaddr, (inp->inp_socket->so_options & (SO_DONTROUTE | SO_BROADCAST)), inp->inp_moptions, inp); + /* + * If this is the first data gram sent on an unbound and unconnected + * UDP socket, lport will be changed in this function. If target + * CPU after this lport changing is no longer the current CPU, then + * free the route entry allocated on the current CPU. + */ + if (lport_any) { + if (udp_addrcpu(inp->inp_faddr.s_addr, inp->inp_laddr.s_addr, + inp->inp_fport, inp->inp_lport) != mycpuid) { + struct route *ro = &inp->inp_route; + + if (ro->ro_rt != NULL) + RTFREE(ro->ro_rt); + bzero(ro, sizeof(*ro)); + } + } return (error); release: @@ -890,9 +907,24 @@ udp_connect(struct socket *so, struct sockaddr *nam, struct thread *td) error = in_pcbconnect(inp, nam, td); } crit_exit(); - if (error == 0) + if (error == 0) { soisconnected(so); - else if (error == EAFNOSUPPORT) { /* connection dissolved */ + + /* + * This function is always called on CPU0, so we need to make + * sure that target CPU is same as CPU0, if it is not, then + * we will have to free the route entry allocated on the + * current CPU (i.e. CPU0). + */ + if (udp_addrcpu(inp->inp_faddr.s_addr, inp->inp_laddr.s_addr, + inp->inp_fport, inp->inp_lport) != mycpuid) { + struct route *ro = &inp->inp_route; + + if (ro->ro_rt != NULL) + RTFREE(ro->ro_rt); + bzero(ro, sizeof(*ro)); + } + } else if (error == EAFNOSUPPORT) { /* connection dissolved */ /* * Follow traditional BSD behavior and retain * the local port binding. But, fix the old misbehavior @@ -923,6 +955,7 @@ static int udp_disconnect(struct socket *so) { struct inpcb *inp; + in_addr_t laddr; inp = so->so_pcb; if (inp == NULL) @@ -934,6 +967,25 @@ udp_disconnect(struct socket *so) in_pcbdisconnect(inp); crit_exit(); so->so_state &= ~SS_ISCONNECTED; /* XXX */ + + /* See the comment in udp_connect() */ + if (!(inp->inp_flags & INP_WASBOUND_NOTANY)) + laddr = INADDR_ANY; + else + laddr = inp->inp_laddr.s_addr; + + /* + * If target CPU is to be changed, free the route entry + * which was allocated on the current CPU. + */ + if (udp_addrcpu(inp->inp_faddr.s_addr, laddr, + inp->inp_fport, inp->inp_lport) != mycpuid) { + struct route *ro = &inp->inp_route; + + if (ro->ro_rt != NULL) + RTFREE(ro->ro_rt); + bzero(ro, sizeof(*ro)); + } return 0; } -- 2.11.4.GIT