From owner-dev-commits-src-main@freebsd.org Tue Sep 7 15:25:54 2021 Return-Path: Delivered-To: dev-commits-src-main@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 D2B72666DE1; Tue, 7 Sep 2021 15:25:54 +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 4H3pxt4Y92z4xJx; Tue, 7 Sep 2021 15:25:54 +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 6B08914D4E; Tue, 7 Sep 2021 15:25:54 +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 187FPs84055517; Tue, 7 Sep 2021 15:25:54 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 187FPsIs055516; Tue, 7 Sep 2021 15:25:54 GMT (envelope-from git) Date: Tue, 7 Sep 2021 15:25:54 GMT Message-Id: <202109071525.187FPsIs055516@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 173a7a4ee4fa - main - sctp: Fix iterator synchronization in sctp_sendall() MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 173a7a4ee4fa8fbce6b4532d4d324bc72ee25fe0 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Sep 2021 15:25:54 -0000 The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=173a7a4ee4fa8fbce6b4532d4d324bc72ee25fe0 commit 173a7a4ee4fa8fbce6b4532d4d324bc72ee25fe0 Author: Mark Johnston AuthorDate: 2021-09-07 13:44:57 +0000 Commit: Mark Johnston CommitDate: 2021-09-07 15:19:29 +0000 sctp: Fix iterator synchronization in sctp_sendall() - The SCTP_PCB_FLAGS_SND_ITERATOR_UP check was racy, since two threads could observe that the flag is not set and then both set it. I'm not sure if this is actually a problem in practice, i.e., maybe there's no problem having multiple sends for a single PCB in the iterator list? - sctp_sendall() was modifying sctp_flags without the inp lock held. The change simply acquires the PCB write lock before toggling the flag, fixing both problems. Reviewed by: tuexen MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D31813 --- sys/netinet/sctp_output.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/sys/netinet/sctp_output.c b/sys/netinet/sctp_output.c index be91084f1287..142c5dbf33b6 100644 --- a/sys/netinet/sctp_output.c +++ b/sys/netinet/sctp_output.c @@ -6770,7 +6770,9 @@ sctp_sendall_completes(void *ptr, uint32_t val SCTP_UNUSED) /* now free everything */ if (ca->inp) { /* Lets clear the flag to allow others to run. */ + SCTP_INP_WLOCK(ca->inp); ca->inp->sctp_flags &= ~SCTP_PCB_FLAGS_SND_ITERATOR_UP; + SCTP_INP_WUNLOCK(ca->inp); } sctp_m_freem(ca->m); SCTP_FREE(ca, SCTP_M_COPYAL); @@ -6825,10 +6827,6 @@ sctp_sendall(struct sctp_inpcb *inp, struct uio *uio, struct mbuf *m, int ret; struct sctp_copy_all *ca; - if (inp->sctp_flags & SCTP_PCB_FLAGS_SND_ITERATOR_UP) { - /* There is another. */ - return (EBUSY); - } if (uio->uio_resid > (ssize_t)SCTP_BASE_SYSCTL(sctp_sendall_limit)) { /* You must not be larger than the limit! */ return (EMSGSIZE); @@ -6846,6 +6844,18 @@ sctp_sendall(struct sctp_inpcb *inp, struct uio *uio, struct mbuf *m, if (srcv) { memcpy(&ca->sndrcv, srcv, sizeof(struct sctp_nonpad_sndrcvinfo)); } + + /* Serialize. */ + SCTP_INP_WLOCK(inp); + if ((inp->sctp_flags & SCTP_PCB_FLAGS_SND_ITERATOR_UP) != 0) { + SCTP_INP_WUNLOCK(inp); + sctp_m_freem(m); + SCTP_FREE(ca, SCTP_M_COPYAL); + return (EBUSY); + } + inp->sctp_flags |= SCTP_PCB_FLAGS_SND_ITERATOR_UP; + SCTP_INP_WUNLOCK(inp); + /* * take off the sendall flag, it would be bad if we failed to do * this :-0 @@ -6857,6 +6867,10 @@ sctp_sendall(struct sctp_inpcb *inp, struct uio *uio, struct mbuf *m, ca->m = sctp_copy_out_all(uio, ca->sndlen); if (ca->m == NULL) { SCTP_FREE(ca, SCTP_M_COPYAL); + sctp_m_freem(m); + SCTP_INP_WLOCK(inp); + inp->sctp_flags &= ~SCTP_PCB_FLAGS_SND_ITERATOR_UP; + SCTP_INP_WUNLOCK(inp); SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP_OUTPUT, ENOMEM); return (ENOMEM); } @@ -6869,14 +6883,15 @@ sctp_sendall(struct sctp_inpcb *inp, struct uio *uio, struct mbuf *m, ca->sndlen += SCTP_BUF_LEN(mat); } } - inp->sctp_flags |= SCTP_PCB_FLAGS_SND_ITERATOR_UP; ret = sctp_initiate_iterator(NULL, sctp_sendall_iterator, NULL, SCTP_PCB_ANY_FLAGS, SCTP_PCB_ANY_FEATURES, SCTP_ASOC_ANY_STATE, (void *)ca, 0, sctp_sendall_completes, inp, 1); if (ret) { + SCTP_INP_WLOCK(inp); inp->sctp_flags &= ~SCTP_PCB_FLAGS_SND_ITERATOR_UP; + SCTP_INP_WUNLOCK(inp); SCTP_FREE(ca, SCTP_M_COPYAL); SCTP_LTRACE_ERR_RET_PKT(m, inp, NULL, NULL, SCTP_FROM_SCTP_OUTPUT, EFAULT); return (EFAULT);