Date: Thu, 12 Mar 2015 09:16:51 +0000 (UTC) From: "Andrey V. Elsukov" <ae@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r279912 - stable/9/sys/netinet6 Message-ID: <201503120916.t2C9Gp1R077515@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ae Date: Thu Mar 12 09:16:50 2015 New Revision: 279912 URL: https://svnweb.freebsd.org/changeset/base/279912 Log: MFC r279588: Fix deadlock in IPv6 PCB code. When several threads are trying to send datagram to the same destination, but fragmentation is disabled and datagram size exceeds link MTU, ip6_output() calls pfctlinput2(PRC_MSGSIZE). It does notify all sockets wanted to know MTU to this destination. And since all threads hold PCB lock while sending, taking the lock for each PCB in the in6_pcbnotify() leads to deadlock. RFC 3542 p.11.3 suggests notify all application wanted to receive IPV6_PATHMTU ancillary data for each ICMPv6 packet too big message. But it doesn't require this, when we don't receive ICMPv6 message. Change ip6_notify_pmtu() function to be able use it directly from ip6_output() to notify only one socket, and to notify all sockets when ICMPv6 packet too big message received. MFC r279684: tcp6_ctlinput() doesn't pass MTU value to in6_pcbnotify(). Check cmdarg isn't NULL before dereference, this check was in the ip6_notify_pmtu() before r279588. PR: 197059 Sponsored by: Yandex LLC Modified: stable/9/sys/netinet6/in6_pcb.c stable/9/sys/netinet6/ip6_input.c stable/9/sys/netinet6/ip6_output.c stable/9/sys/netinet6/ip6_var.h Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/netinet6/in6_pcb.c ============================================================================== --- stable/9/sys/netinet6/in6_pcb.c Thu Mar 12 09:04:19 2015 (r279911) +++ stable/9/sys/netinet6/in6_pcb.c Thu Mar 12 09:16:50 2015 (r279912) @@ -624,18 +624,12 @@ in6_pcbnotify(struct inpcbinfo *pcbinfo, /* * If the error designates a new path MTU for a destination * and the application (associated with this socket) wanted to - * know the value, notify. Note that we notify for all - * disconnected sockets if the corresponding application - * wanted. This is because some UDP applications keep sending - * sockets disconnected. + * know the value, notify. * XXX: should we avoid to notify the value to TCP sockets? */ - if (cmd == PRC_MSGSIZE && (inp->inp_flags & IN6P_MTU) != 0 && - (IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr) || - IN6_ARE_ADDR_EQUAL(&inp->in6p_faddr, &sa6_dst->sin6_addr))) { + if (cmd == PRC_MSGSIZE && cmdarg != NULL) ip6_notify_pmtu(inp, (struct sockaddr_in6 *)dst, - (u_int32_t *)cmdarg); - } + *(u_int32_t *)cmdarg); /* * Detect if we should notify the error. If no source and Modified: stable/9/sys/netinet6/ip6_input.c ============================================================================== --- stable/9/sys/netinet6/ip6_input.c Thu Mar 12 09:04:19 2015 (r279911) +++ stable/9/sys/netinet6/ip6_input.c Thu Mar 12 09:16:50 2015 (r279912) @@ -1603,24 +1603,28 @@ ip6_savecontrol(struct inpcb *in6p, stru #undef IS2292 void -ip6_notify_pmtu(struct inpcb *in6p, struct sockaddr_in6 *dst, u_int32_t *mtu) +ip6_notify_pmtu(struct inpcb *inp, struct sockaddr_in6 *dst, u_int32_t mtu) { struct socket *so; struct mbuf *m_mtu; struct ip6_mtuinfo mtuctl; - so = in6p->inp_socket; - - if (mtu == NULL) + KASSERT(inp != NULL, ("%s: inp == NULL", __func__)); + /* + * Notify the error by sending IPV6_PATHMTU ancillary data if + * application wanted to know the MTU value. + * NOTE: we notify disconnected sockets, because some udp + * applications keep sending sockets disconnected. + * NOTE: our implementation doesn't notify connected sockets that has + * foreign address that is different than given destination addresses + * (this is permitted by RFC 3542). + */ + if ((inp->inp_flags & IN6P_MTU) == 0 || ( + !IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr) && + !IN6_ARE_ADDR_EQUAL(&inp->in6p_faddr, &dst->sin6_addr))) return; -#ifdef DIAGNOSTIC - if (so == NULL) /* I believe this is impossible */ - panic("ip6_notify_pmtu: socket is NULL"); -#endif - - bzero(&mtuctl, sizeof(mtuctl)); /* zero-clear for safety */ - mtuctl.ip6m_mtu = *mtu; + mtuctl.ip6m_mtu = mtu; mtuctl.ip6m_addr = *dst; if (sa6_recoverscope(&mtuctl.ip6m_addr)) return; @@ -1629,14 +1633,13 @@ ip6_notify_pmtu(struct inpcb *in6p, stru IPV6_PATHMTU, IPPROTO_IPV6)) == NULL) return; + so = inp->inp_socket; if (sbappendaddr(&so->so_rcv, (struct sockaddr *)dst, NULL, m_mtu) == 0) { m_freem(m_mtu); /* XXX: should count statistics */ } else sorwakeup(so); - - return; } #ifdef PULLDOWN_TEST Modified: stable/9/sys/netinet6/ip6_output.c ============================================================================== --- stable/9/sys/netinet6/ip6_output.c Thu Mar 12 09:04:19 2015 (r279911) +++ stable/9/sys/netinet6/ip6_output.c Thu Mar 12 09:16:50 2015 (r279912) @@ -996,19 +996,12 @@ passout: * Even if the DONTFRAG option is specified, we cannot send the * packet when the data length is larger than the MTU of the * outgoing interface. - * Notify the error by sending IPV6_PATHMTU ancillary data as - * well as returning an error code (the latter is not described - * in the API spec.) + * Notify the error by sending IPV6_PATHMTU ancillary data if + * application wanted to know the MTU value. Also return an + * error code (this is not described in the API spec). */ - u_int32_t mtu32; - struct ip6ctlparam ip6cp; - - mtu32 = (u_int32_t)mtu; - bzero(&ip6cp, sizeof(ip6cp)); - ip6cp.ip6c_cmdarg = (void *)&mtu32; - pfctlinput2(PRC_MSGSIZE, (struct sockaddr *)&ro_pmtu->ro_dst, - (void *)&ip6cp); - + if (inp != NULL) + ip6_notify_pmtu(inp, &dst_sa, (u_int32_t)mtu); error = EMSGSIZE; goto bad; } Modified: stable/9/sys/netinet6/ip6_var.h ============================================================================== --- stable/9/sys/netinet6/ip6_var.h Thu Mar 12 09:04:19 2015 (r279911) +++ stable/9/sys/netinet6/ip6_var.h Thu Mar 12 09:16:50 2015 (r279912) @@ -406,8 +406,7 @@ int ip6_process_hopopts(struct mbuf *, u struct mbuf **ip6_savecontrol_v4(struct inpcb *, struct mbuf *, struct mbuf **, int *); void ip6_savecontrol(struct inpcb *, struct mbuf *, struct mbuf **); -void ip6_notify_pmtu(struct inpcb *, struct sockaddr_in6 *, - u_int32_t *); +void ip6_notify_pmtu(struct inpcb *, struct sockaddr_in6 *, u_int32_t); int ip6_sysctl(int *, u_int, void *, size_t *, void *, size_t); void ip6_forward(struct mbuf *, int);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201503120916.t2C9Gp1R077515>