From owner-dev-commits-src-all@freebsd.org Thu Feb 11 09:23:53 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 52741540670; Thu, 11 Feb 2021 09:23:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Dbrm91xnPz3DlS; Thu, 11 Feb 2021 09:23:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 35C9C66A9; Thu, 11 Feb 2021 09:23:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 11B9NrCK038739; Thu, 11 Feb 2021 09:23:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 11B9NrCX038738; Thu, 11 Feb 2021 09:23:53 GMT (envelope-from git) Date: Thu, 11 Feb 2021 09:23:53 GMT Message-Id: <202102110923.11B9NrCX038738@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Andrey V. Elsukov" Subject: git: 3c782d9c9166 - main - [udp6] fix possible panic due to lack of locking. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: ae X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 3c782d9c91666886d868426f0aea040d1a1a8ee4 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Feb 2021 09:23:53 -0000 The branch main has been updated by ae: URL: https://cgit.FreeBSD.org/src/commit/?id=3c782d9c91666886d868426f0aea040d1a1a8ee4 commit 3c782d9c91666886d868426f0aea040d1a1a8ee4 Author: Andrey V. Elsukov AuthorDate: 2021-02-11 08:38:41 +0000 Commit: Andrey V. Elsukov CommitDate: 2021-02-11 09:00:25 +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 MFC after: 3 days Sponsored by: Yandex LLC Differential Revision: https://reviews.freebsd.org/D28232 --- 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);