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
This commit is contained in:
Mark Johnston 2021-09-07 09:44:57 -04:00
parent e8e23ec127
commit 173a7a4ee4

View file

@ -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);