git: 8cb4c3631630 - stable/13 - [udp6] fix possible panic due to lack of locking.

Andrey V. Elsukov ae at FreeBSD.org
Mon Feb 15 10:51:25 UTC 2021


The branch stable/13 has been updated by ae:

URL: https://cgit.FreeBSD.org/src/commit/?id=8cb4c363163062bceec92383eae43f5a32049c73

commit 8cb4c363163062bceec92383eae43f5a32049c73
Author:     Andrey V. Elsukov <ae at FreeBSD.org>
AuthorDate: 2021-02-11 08:38:41 +0000
Commit:     Andrey V. Elsukov <ae at FreeBSD.org>
CommitDate: 2021-02-15 10:48:56 +0000

    [udp6] fix possible panic due to lack of locking.
    
    The lookup for a IPv6 multicast addresses corresponding to
    the destination address in the datagram is protected by the
    NET_EPOCH section. Access to each PCB is protected by INP_RLOCK
    during comparing. But access to socket's so_options field is
    not protected. And in some cases it is possible, that PCB
    pointer is still valid, but inp_socket is not. The patch wides
    lock holding to protect access to inp_socket. It copies locking
    strategy from IPv4 UDP handling.
    
    PR:     232192
    Obtained from:  Yandex LLC
    Sponsored by:   Yandex LLC
    Differential Revision:  https://reviews.freebsd.org/D28232
    
    (cherry picked from commit 3c782d9c91666886d868426f0aea040d1a1a8ee4)
---
 sys/netinet6/udp6_usrreq.c | 61 +++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c
index 1535be90e1b0..3a001fea077d 100644
--- a/sys/netinet6/udp6_usrreq.c
+++ b/sys/netinet6/udp6_usrreq.c
@@ -353,6 +353,13 @@ skip_checksum:
 					continue;
 			}
 
+			INP_RLOCK(inp);
+
+			if (__predict_false(inp->inp_flags2 & INP_FREED)) {
+				INP_RUNLOCK(inp);
+				continue;
+			}
+
 			/*
 			 * XXXRW: Because we weren't holding either the inpcb
 			 * or the hash lock when we checked for a match 
@@ -365,16 +372,10 @@ skip_checksum:
 			 * and source-specific multicast. [RFC3678]
 			 */
 			imo = inp->in6p_moptions;
-			if (imo && IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst)) {
+			if (imo != NULL) {
 				struct sockaddr_in6	 mcaddr;
 				int			 blocked;
 
-				INP_RLOCK(inp);
-				if (__predict_false(inp->inp_flags2 & INP_FREED)) {
-					INP_RUNLOCK(inp);
-					continue;
-				}
-
 				bzero(&mcaddr, sizeof(struct sockaddr_in6));
 				mcaddr.sin6_len = sizeof(struct sockaddr_in6);
 				mcaddr.sin6_family = AF_INET6;
@@ -389,33 +390,30 @@ skip_checksum:
 					if (blocked == MCAST_NOTSMEMBER ||
 					    blocked == MCAST_MUTED)
 						UDPSTAT_INC(udps_filtermcast);
-					INP_RUNLOCK(inp); /* XXX */
+					INP_RUNLOCK(inp);
 					continue;
 				}
-
-				INP_RUNLOCK(inp);
 			}
+
 			if (last != NULL) {
 				struct mbuf *n;
 
 				if ((n = m_copym(m, 0, M_COPYALL, M_NOWAIT)) !=
 				    NULL) {
-					INP_RLOCK(last);
-					if (__predict_true(last->inp_flags2 & INP_FREED) == 0) {
-						if (nxt == IPPROTO_UDPLITE)
-							UDPLITE_PROBE(receive, NULL, last,
-							    ip6, last, uh);
-						else
-							UDP_PROBE(receive, NULL, last,
-							    ip6, last, uh);
-						if (udp6_append(last, n, off, fromsa)) {
-							/* XXX-BZ do we leak m here? */
-							*mp = NULL;
-							return (IPPROTO_DONE);
-						}
+					if (nxt == IPPROTO_UDPLITE)
+						UDPLITE_PROBE(receive, NULL,
+						    last, ip6, last, uh);
+					else
+						UDP_PROBE(receive, NULL, last,
+						    ip6, last, uh);
+					if (udp6_append(last, n, off,
+					    fromsa)) {
+						INP_RUNLOCK(inp);
+						goto badunlocked;
 					}
-					INP_RUNLOCK(last);
 				}
+				/* Release PCB lock taken on previous pass. */
+				INP_RUNLOCK(last);
 			}
 			last = inp;
 			/*
@@ -441,15 +439,12 @@ skip_checksum:
 			UDPSTAT_INC(udps_noportmcast);
 			goto badunlocked;
 		}
-		INP_RLOCK(last);
-		if (__predict_true(last->inp_flags2 & INP_FREED) == 0) {
-			if (nxt == IPPROTO_UDPLITE)
-				UDPLITE_PROBE(receive, NULL, last, ip6, last, uh);
-			else
-				UDP_PROBE(receive, NULL, last, ip6, last, uh);
-			if (udp6_append(last, m, off, fromsa) == 0)
-				INP_RUNLOCK(last);
-		} else
+
+		if (nxt == IPPROTO_UDPLITE)
+			UDPLITE_PROBE(receive, NULL, last, ip6, last, uh);
+		else
+			UDP_PROBE(receive, NULL, last, ip6, last, uh);
+		if (udp6_append(last, m, off, fromsa) == 0)
 			INP_RUNLOCK(last);
 		*mp = NULL;
 		return (IPPROTO_DONE);


More information about the dev-commits-src-all mailing list