From owner-dev-commits-src-branches@freebsd.org Fri Jun 4 01:09:32 2021 Return-Path: Delivered-To: dev-commits-src-branches@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 558B46446B3; Fri, 4 Jun 2021 01:09:32 +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 4Fx4Rc1ZRXz3K28; Fri, 4 Jun 2021 01:09:32 +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 1E01413ECD; Fri, 4 Jun 2021 01:09:32 +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 15419WsY047769; Fri, 4 Jun 2021 01:09:32 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 15419WQZ047768; Fri, 4 Jun 2021 01:09:32 GMT (envelope-from git) Date: Fri, 4 Jun 2021 01:09:32 GMT Message-Id: <202106040109.15419WQZ047768@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 0f05c6f3a527 - stable/13 - tcp: Make error handling in tcp_usr_send() more consistent 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/stable/13 X-Git-Reftype: branch X-Git-Commit: 0f05c6f3a5274d07f17e7e2ca5a7d286ae37c09b Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Jun 2021 01:09:32 -0000 The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=0f05c6f3a5274d07f17e7e2ca5a7d286ae37c09b commit 0f05c6f3a5274d07f17e7e2ca5a7d286ae37c09b Author: Mark Johnston AuthorDate: 2021-05-21 21:44:40 +0000 Commit: Mark Johnston CommitDate: 2021-06-04 01:01:33 +0000 tcp: Make error handling in tcp_usr_send() more consistent - Free the input mbuf in a single place instead of in every error path. - Handle PRUS_NOTREADY consistently. - Flush the socket's send buffer if an implicit connect fails. At that point the mbuf has already been enqueued but we don't want to keep it in the send buffer. Reviewed by: gallatin, tuexen Discussed with: jhb Sponsored by: The FreeBSD Foundation (cherry picked from commit 7d2608a5d24ec3534dad7f24191f12a8181ea206) --- sys/netinet/tcp_usrreq.c | 67 +++++++++++++++++++----------------------------- 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 48c3be3ec42c..1e593c4b1eb2 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -1000,13 +1000,6 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, if (inp->inp_flags & (INP_TIMEWAIT | INP_DROPPED)) { if (control) m_freem(control); - - /* - * In case of PRUS_NOTREADY, tcp_usr_ready() is responsible - * for freeing memory. - */ - if ((flags & PRUS_NOTREADY) == 0) - m_freem(m); error = ECONNRESET; goto out; } @@ -1014,7 +1007,6 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, /* TCP doesn't do control messages (rights, creds, etc) */ if (control->m_len) { m_freem(control); - m_freem(m); error = EINVAL; goto out; } @@ -1022,13 +1014,10 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, control = NULL; } tp = intotcpcb(inp); - if (flags & PRUS_OOB) { - if ((error = tcp_pru_options_support(tp, PRUS_OOB)) != 0) { - if ((flags & PRUS_NOTREADY) == 0) - m_freem(m); - goto out; - } - } + if ((flags & PRUS_OOB) != 0 && + (error = tcp_pru_options_support(tp, PRUS_OOB)) != 0) + goto out; + TCPDEBUG1(); if (nam != NULL && tp->t_state < TCPS_SYN_SENT) { switch (nam->sa_family) { @@ -1036,30 +1025,24 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, case AF_INET: sinp = (struct sockaddr_in *)nam; if (sinp->sin_len != sizeof(struct sockaddr_in)) { - m_freem(m); error = EINVAL; goto out; } if ((inp->inp_vflag & INP_IPV6) != 0) { - m_freem(m); error = EAFNOSUPPORT; goto out; } if (IN_MULTICAST(ntohl(sinp->sin_addr.s_addr))) { - m_freem(m); error = EAFNOSUPPORT; goto out; } if (ntohl(sinp->sin_addr.s_addr) == INADDR_BROADCAST) { - m_freem(m); error = EACCES; goto out; } if ((error = prison_remote_ip4(td->td_ucred, - &sinp->sin_addr))) { - m_freem(m); + &sinp->sin_addr))) goto out; - } #ifdef INET6 isipv6 = 0; #endif @@ -1072,17 +1055,14 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, sin6 = (struct sockaddr_in6 *)nam; if (sin6->sin6_len != sizeof(*sin6)) { - m_freem(m); error = EINVAL; goto out; } if ((inp->inp_vflag & INP_IPV6PROTO) == 0) { - m_freem(m); error = EAFNOSUPPORT; goto out; } if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) { - m_freem(m); error = EAFNOSUPPORT; goto out; } @@ -1090,12 +1070,10 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, #ifdef INET if ((inp->inp_flags & IN6P_IPV6_V6ONLY) != 0) { error = EINVAL; - m_freem(m); goto out; } if ((inp->inp_vflag & INP_IPV4) == 0) { error = EAFNOSUPPORT; - m_freem(m); goto out; } restoreflags = true; @@ -1105,23 +1083,18 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, if (IN_MULTICAST( ntohl(sinp->sin_addr.s_addr))) { error = EAFNOSUPPORT; - m_freem(m); goto out; } if ((error = prison_remote_ip4(td->td_ucred, - &sinp->sin_addr))) { - m_freem(m); + &sinp->sin_addr))) goto out; - } isipv6 = 0; #else /* !INET */ error = EAFNOSUPPORT; - m_freem(m); goto out; #endif /* INET */ } else { if ((inp->inp_vflag & INP_IPV6) == 0) { - m_freem(m); error = EAFNOSUPPORT; goto out; } @@ -1129,23 +1102,21 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, inp->inp_vflag &= ~INP_IPV4; inp->inp_inc.inc_flags |= INC_ISIPV6; if ((error = prison_remote_ip6(td->td_ucred, - &sin6->sin6_addr))) { - m_freem(m); + &sin6->sin6_addr))) goto out; - } isipv6 = 1; } break; } #endif /* INET6 */ default: - m_freem(m); error = EAFNOSUPPORT; goto out; } } if (!(flags & PRUS_OOB)) { sbappendstream(&so->so_snd, m, flags); + m = NULL; if (nam && tp->t_state < TCPS_SYN_SENT) { /* * Do implied connect if not yet connected, @@ -1171,8 +1142,11 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, if (error == 0 || inp->inp_lport != 0) restoreflags = false; - if (error) + if (error) { + /* m is freed if PRUS_NOTREADY is unset. */ + sbflush(&so->so_snd); goto out; + } if (IS_FASTOPEN(tp->t_flags)) tcp_fastopen_connect(tp); else { @@ -1213,7 +1187,6 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, SOCKBUF_LOCK(&so->so_snd); if (sbspace(&so->so_snd) < -512) { SOCKBUF_UNLOCK(&so->so_snd); - m_freem(m); error = ENOBUFS; goto out; } @@ -1227,6 +1200,7 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, */ sbappendstream_locked(&so->so_snd, m, flags); SOCKBUF_UNLOCK(&so->so_snd); + m = NULL; if (nam && tp->t_state < TCPS_SYN_SENT) { /* * Do implied connect if not yet connected, @@ -1258,13 +1232,16 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, if (error == 0 || inp->inp_lport != 0) restoreflags = false; - if (error) + if (error != 0) { + /* m is freed if PRUS_NOTREADY is unset. */ + sbflush(&so->so_snd); goto out; + } tp->snd_wnd = TTCP_CLIENT_SND_WND; tcp_mss(tp, -1); } tp->snd_up = tp->snd_una + sbavail(&so->so_snd); - if (!(flags & PRUS_NOTREADY)) { + if ((flags & PRUS_NOTREADY) == 0) { tp->t_flags |= TF_FORCEDATA; error = tp->t_fb->tfb_tcp_output(tp); tp->t_flags &= ~TF_FORCEDATA; @@ -1275,7 +1252,15 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, &inp->inp_socket->so_snd, TCP_LOG_USERSEND, error, 0, NULL, false); + out: + /* + * In case of PRUS_NOTREADY, the caller or tcp_usr_ready() is + * responsible for freeing memory. + */ + if (m != NULL && (flags & PRUS_NOTREADY) == 0) + m_freem(m); + /* * If the request was unsuccessful and we changed flags, * restore the original flags.