Attachment 197208 Details for Bug 230950 – inp wlocking for route cache in udp6_output

[patch] inp wlocking for route cache in udp6_output

20180918-01-pr230950.diff (text/plain), 3.80 KB, created by Bjoern A. Zeeb on 2018-09-18 16:36:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bjoern A. Zeeb

Created: 2018-09-18 16:36:14 UTC

Size: 3.80 KB

patch

obsolete

>Index: sys/netinet6/udp6_usrreq.c
>===================================================================
>--- sys/netinet6/udp6_usrreq.c	(revision 338753)
>+++ sys/netinet6/udp6_usrreq.c	(working copy)
>@@ -698,7 +698,7 @@ udp6_output(struct socket *so, int flags_arg, stru
> 	u_int32_t ulen, plen;
> 	uint16_t cscov;
> 	u_short fport;
>-	uint8_t nxt, unlock_udbinfo;
>+	uint8_t nxt, unlock_udbinfo, unlock_inp;
> 
> 	/* addr6 has been validated in udp6_send(). */
> 	sin6 = (struct sockaddr_in6 *)addr6;
>@@ -734,7 +734,22 @@ udp6_output(struct socket *so, int flags_arg, stru
> 
> 	inp = sotoinpcb(so);
> 	KASSERT(inp != NULL, ("%s: inp == NULL", __func__));
>-	INP_RLOCK(inp);
>+	/*
>+	 * In the following cases we want a write lock on the inp for either
>+	 * local operations or for possible route cache updates in the IPv6
>+	 * output path:
>+	 * - on connected sockets (sin6 is NULL) for route cache updates,
>+	 * - on we are not bound to an address and source port (it is
>+	 *   in6_pcbsetport() which will require the write lock).
>+	 */
>+	if (sin6 == NULL || (IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr) &&
>+	    inp->inp_lport == 0)) {
>+		INP_WLOCK(inp);
>+		unlock_inp = UH_WLOCKED;
>+	} else {
>+		INP_RLOCK(inp);
>+		unlock_inp = UH_RLOCKED;
>+	}
> 	nxt = (inp->inp_socket->so_proto->pr_protocol == IPPROTO_UDP) ?
> 	    IPPROTO_UDP : IPPROTO_UDPLITE;
> 
>@@ -758,7 +773,10 @@ udp6_output(struct socket *so, int flags_arg, stru
> 			 * potential race in which the factors causing us to
> 			 * select the UDPv4 output routine are invalidated?
> 			 */
>-			INP_RUNLOCK(inp);
>+			if (unlock_inp == UH_WLOCKED)
>+				INP_WUNLOCK(inp);
>+			else
>+				INP_RUNLOCK(inp);
> 			if (sin6)
> 				in6_sin6_2_sin_in_sock((struct sockaddr *)sin6);
> 			pru = inetsw[ip_protox[nxt]].pr_usrreqs;
>@@ -772,7 +790,10 @@ udp6_output(struct socket *so, int flags_arg, stru
> 	if (control) {
> 		if ((error = ip6_setpktopts(control, &opt,
> 		    inp->in6p_outputopts, td->td_ucred, nxt)) != 0) {
>-			INP_RUNLOCK(inp);
>+			if (unlock_inp == UH_WLOCKED)
>+				INP_WUNLOCK(inp);
>+			else
>+				INP_RUNLOCK(inp);
> 			ip6_clearpktopts(&opt, -1);
> 			if (control)
> 				m_freem(control);
>@@ -786,12 +807,6 @@ udp6_output(struct socket *so, int flags_arg, stru
> 	pcbinfo = udp_get_inpcbinfo(so->so_proto->pr_protocol);
> 	if (sin6 != NULL &&
> 	    IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr) && inp->inp_lport == 0) {
>-		INP_RUNLOCK(inp);
>-		/*
>-		 * XXX there is a short window here which could lead to a race;
>-		 * should we re-check that what got us here is still valid?
>-		 */
>-		INP_WLOCK(inp);
> 		INP_HASH_WLOCK(pcbinfo);
> 		unlock_udbinfo = UH_WLOCKED;
> 	} else if (sin6 != NULL &&
>@@ -972,9 +987,10 @@ udp6_output(struct socket *so, int flags_arg, stru
> 		UDPLITE_PROBE(send, NULL, inp, ip6, inp, udp6);
> 	else
> 		UDP_PROBE(send, NULL, inp, ip6, inp, udp6);
>-	error = ip6_output(m, optp, &inp->inp_route6, flags,
>+	error = ip6_output(m, optp,
>+	    (unlock_inp == UH_WLOCKED) ? &inp->inp_route6 : NULL, flags,
> 	    inp->in6p_moptions, NULL, inp);
>-	if (unlock_udbinfo == UH_WLOCKED)
>+	if (unlock_inp == UH_WLOCKED)
> 		INP_WUNLOCK(inp);
> 	else
> 		INP_RUNLOCK(inp);
>@@ -987,12 +1003,20 @@ udp6_output(struct socket *so, int flags_arg, stru
> 
> release:
> 	if (unlock_udbinfo == UH_WLOCKED) {
>+		KASSERT(unlock_inp == UH_WLOCKED, ("%s: excl udbinfo lock, "
>+		    "non-excl inp lock: pcbinfo %p %#x inp %p %#x",
>+		    __func__, pcbinfo, unlock_udbinfo, inp, unlock_inp));
> 		INP_HASH_WUNLOCK(pcbinfo);
> 		INP_WUNLOCK(inp);
> 	} else if (unlock_udbinfo == UH_RLOCKED) {
>+		KASSERT(unlock_inp == UH_RLOCKED, ("%s: non-excl udbinfo lock, "
>+		    "excl inp lock: pcbinfo %p %#x inp %p %#x",
>+		    __func__, pcbinfo, unlock_udbinfo, inp, unlock_inp));
> 		INP_HASH_RUNLOCK_ET(pcbinfo, et);
> 		INP_RUNLOCK(inp);
>-	} else
>+	} else if (unlock_inp == UH_WLOCKED)
>+		INP_WUNLOCK(inp);
>+	else
> 		INP_RUNLOCK(inp);
> 	if (control) {
> 		ip6_clearpktopts(&opt, -1);

Actions: View | Diff

Attachments on bug 230950: 197157 | 197208