From owner-svn-src-all@freebsd.org Sun Aug 16 11:50:39 2020 Return-Path: Delivered-To: svn-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 153403B9056; Sun, 16 Aug 2020 11:50:39 +0000 (UTC) (envelope-from tuexen@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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BTwV66sz9z4Fwx; Sun, 16 Aug 2020 11:50:38 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D0ACDF47F; Sun, 16 Aug 2020 11:50:38 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 07GBocFo025895; Sun, 16 Aug 2020 11:50:38 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 07GBob8r025885; Sun, 16 Aug 2020 11:50:37 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <202008161150.07GBob8r025885@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Sun, 16 Aug 2020 11:50:37 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r364268 - head/sys/netinet X-SVN-Group: head X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: head/sys/netinet X-SVN-Commit-Revision: 364268 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Aug 2020 11:50:39 -0000 Author: tuexen Date: Sun Aug 16 11:50:37 2020 New Revision: 364268 URL: https://svnweb.freebsd.org/changeset/base/364268 Log: Improve the handling of concurrent send() calls for SCTP sockets, especially when having the explicit EOR mode enabled. Reported by: Megan2013678@protonmail.com Reported by: syzbot+bc02585076c3cc977f9b@syzkaller.appspotmail.com MFC after: 3 days Modified: head/sys/netinet/sctp_input.c head/sys/netinet/sctp_output.c head/sys/netinet/sctp_pcb.c head/sys/netinet/sctp_structs.h head/sys/netinet/sctp_usrreq.c head/sys/netinet/sctputil.c head/sys/netinet/sctputil.h Modified: head/sys/netinet/sctp_input.c ============================================================================== --- head/sys/netinet/sctp_input.c Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctp_input.c Sun Aug 16 11:50:37 2020 (r364268) @@ -829,7 +829,6 @@ sctp_handle_abort(struct sctp_abort_chunk *abort, #ifdef SCTP_ASOCLOG_OF_TSNS sctp_print_out_track_log(stcb); #endif - SCTP_ADD_SUBSTATE(stcb, SCTP_STATE_WAS_ABORTED); (void)sctp_free_assoc(stcb->sctp_ep, stcb, SCTP_NORMAL_PROC, SCTP_FROM_SCTP_INPUT + SCTP_LOC_8); SCTPDBG(SCTP_DEBUG_INPUT2, "sctp_handle_abort: finished\n"); @@ -1866,7 +1865,7 @@ sctp_process_cookie_existing(struct mbuf *m, int iphle /* send up all the data */ SCTP_TCB_SEND_LOCK(stcb); - sctp_report_all_outbound(stcb, 0, 1, SCTP_SO_LOCKED); + sctp_report_all_outbound(stcb, 0, SCTP_SO_LOCKED); for (i = 0; i < stcb->asoc.streamoutcnt; i++) { stcb->asoc.strmout[i].chunks_on_queues = 0; #if defined(SCTP_DETAILED_STR_STATS) Modified: head/sys/netinet/sctp_output.c ============================================================================== --- head/sys/netinet/sctp_output.c Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctp_output.c Sun Aug 16 11:50:37 2020 (r364268) @@ -13148,12 +13148,21 @@ skip_preblock: if (sinfo_flags & SCTP_UNORDERED) { SCTP_STAT_INCR(sctps_sends_with_unord); } + sp->processing = 1; TAILQ_INSERT_TAIL(&strm->outqueue, sp, next); stcb->asoc.ss_functions.sctp_ss_add_to_stream(stcb, asoc, strm, sp, 1); SCTP_TCB_SEND_UNLOCK(stcb); } else { SCTP_TCB_SEND_LOCK(stcb); sp = TAILQ_LAST(&strm->outqueue, sctp_streamhead); + if (sp->processing) { + SCTP_TCB_SEND_UNLOCK(stcb); + SCTP_LTRACE_ERR_RET(inp, stcb, net, SCTP_FROM_SCTP_OUTPUT, EINVAL); + error = EINVAL; + goto out; + } else { + sp->processing = 1; + } SCTP_TCB_SEND_UNLOCK(stcb); if (sp == NULL) { /* ???? Huh ??? last msg is gone */ @@ -13195,13 +13204,14 @@ skip_preblock: } /* Update the mbuf and count */ SCTP_TCB_SEND_LOCK(stcb); - if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + if ((stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) || + (stcb->asoc.state & SCTP_STATE_WAS_ABORTED)) { /* * we need to get out. Peer probably * aborted. */ sctp_m_freem(mm); - if (stcb->asoc.state & SCTP_PCB_FLAGS_WAS_ABORTED) { + if (stcb->asoc.state & SCTP_STATE_WAS_ABORTED) { SCTP_LTRACE_ERR_RET(NULL, stcb, NULL, SCTP_FROM_SCTP_OUTPUT, ECONNRESET); error = ECONNRESET; } @@ -13405,7 +13415,8 @@ skip_preblock: } } SCTP_TCB_SEND_LOCK(stcb); - if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + if ((stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) || + (stcb->asoc.state & SCTP_STATE_WAS_ABORTED)) { SCTP_TCB_SEND_UNLOCK(stcb); goto out_unlocked; } @@ -13421,6 +13432,7 @@ skip_preblock: strm->last_msg_incomplete = 0; asoc->stream_locked = 0; } + sp->processing = 0; } else { SCTP_PRINTF("Huh no sp TSNH?\n"); strm->last_msg_incomplete = 0; Modified: head/sys/netinet/sctp_pcb.c ============================================================================== --- head/sys/netinet/sctp_pcb.c Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctp_pcb.c Sun Aug 16 11:50:37 2020 (r364268) @@ -4733,6 +4733,7 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc /* there is no asoc, really TSNH :-0 */ return (1); } + SCTP_TCB_SEND_LOCK(stcb); if (stcb->asoc.alternate) { sctp_free_remote_addr(stcb->asoc.alternate); stcb->asoc.alternate = NULL; @@ -4767,6 +4768,7 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc /* nope, reader or writer in the way */ sctp_timer_start(SCTP_TIMER_TYPE_ASOCKILL, inp, stcb, NULL); /* no asoc destroyed */ + SCTP_TCB_SEND_UNLOCK(stcb); SCTP_TCB_UNLOCK(stcb); #ifdef SCTP_LOG_CLOSING sctp_log_closing(inp, stcb, 8); @@ -4835,6 +4837,7 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc SCTP_CLEAR_SUBSTATE(stcb, SCTP_STATE_IN_ACCEPT_QUEUE); sctp_timer_start(SCTP_TIMER_TYPE_ASOCKILL, inp, stcb, NULL); } + SCTP_TCB_SEND_UNLOCK(stcb); SCTP_TCB_UNLOCK(stcb); if ((inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_ALLGONE) || (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE)) @@ -4868,10 +4871,12 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc if (from_inpcbfree == SCTP_NORMAL_PROC) { atomic_add_int(&stcb->asoc.refcnt, 1); + SCTP_TCB_SEND_UNLOCK(stcb); SCTP_TCB_UNLOCK(stcb); SCTP_INP_INFO_WLOCK(); SCTP_INP_WLOCK(inp); SCTP_TCB_LOCK(stcb); + SCTP_TCB_SEND_LOCK(stcb); } /* Double check the GONE flag */ if ((inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_ALLGONE) || @@ -4919,6 +4924,7 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc SCTP_INP_INFO_WUNLOCK(); SCTP_INP_WUNLOCK(inp); } + SCTP_TCB_SEND_UNLOCK(stcb); SCTP_TCB_UNLOCK(stcb); return (0); } @@ -4950,7 +4956,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc * in case. */ /* anything on the wheel needs to be removed */ - SCTP_TCB_SEND_LOCK(stcb); for (i = 0; i < asoc->streamoutcnt; i++) { struct sctp_stream_out *outs; @@ -4981,7 +4986,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc sctp_free_a_strmoq(stcb, sp, SCTP_SO_LOCKED); } } - SCTP_TCB_SEND_UNLOCK(stcb); /* sa_ignore FREED_MEMORY */ TAILQ_FOREACH_SAFE(strrst, &asoc->resetHead, next_resp, nstrrst) { TAILQ_REMOVE(&asoc->resetHead, strrst, next_resp); @@ -5183,6 +5187,7 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tc /* Insert new items here :> */ /* Get rid of LOCK */ + SCTP_TCB_SEND_UNLOCK(stcb); SCTP_TCB_UNLOCK(stcb); SCTP_TCB_LOCK_DESTROY(stcb); SCTP_TCB_SEND_LOCK_DESTROY(stcb); Modified: head/sys/netinet/sctp_structs.h ============================================================================== --- head/sys/netinet/sctp_structs.h Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctp_structs.h Sun Aug 16 11:50:37 2020 (r364268) @@ -534,6 +534,7 @@ struct sctp_stream_queue_pending { uint8_t sender_all_done; uint8_t put_last_out; uint8_t discard_rest; + uint8_t processing; }; /* Modified: head/sys/netinet/sctp_usrreq.c ============================================================================== --- head/sys/netinet/sctp_usrreq.c Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctp_usrreq.c Sun Aug 16 11:50:37 2020 (r364268) @@ -190,6 +190,7 @@ sctp_notify(struct sctp_inpcb *inp, } else if ((icmp_code == ICMP_UNREACH_PROTOCOL) || (icmp_code == ICMP_UNREACH_PORT)) { /* Treat it like an ABORT. */ + SCTP_ADD_SUBSTATE(stcb, SCTP_STATE_WAS_ABORTED); sctp_abort_notification(stcb, 1, 0, NULL, SCTP_SO_NOT_LOCKED); (void)sctp_free_assoc(inp, stcb, SCTP_NORMAL_PROC, SCTP_FROM_SCTP_USRREQ + SCTP_LOC_2); Modified: head/sys/netinet/sctputil.c ============================================================================== --- head/sys/netinet/sctputil.c Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctputil.c Sun Aug 16 11:50:37 2020 (r364268) @@ -4239,7 +4239,7 @@ sctp_ulp_notify(uint32_t notification, struct sctp_tcb } void -sctp_report_all_outbound(struct sctp_tcb *stcb, uint16_t error, int holds_lock, int so_locked) +sctp_report_all_outbound(struct sctp_tcb *stcb, uint16_t error, int so_locked) { struct sctp_association *asoc; struct sctp_stream_out *outs; @@ -4261,9 +4261,6 @@ sctp_report_all_outbound(struct sctp_tcb *stcb, uint16 return; } /* now through all the gunk freeing chunks */ - if (holds_lock == 0) { - SCTP_TCB_SEND_LOCK(stcb); - } /* sent queue SHOULD be empty */ TAILQ_FOREACH_SAFE(chk, &asoc->sent_queue, sctp_next, nchk) { TAILQ_REMOVE(&asoc->sent_queue, chk, sctp_next); @@ -4340,10 +4337,6 @@ sctp_report_all_outbound(struct sctp_tcb *stcb, uint16 /* sa_ignore FREED_MEMORY */ } } - - if (holds_lock == 0) { - SCTP_TCB_SEND_UNLOCK(stcb); - } } void @@ -4363,8 +4356,11 @@ sctp_abort_notification(struct sctp_tcb *stcb, uint8_t (stcb->asoc.state & SCTP_STATE_CLOSED_SOCKET)) { return; } + SCTP_TCB_SEND_LOCK(stcb); + SCTP_ADD_SUBSTATE(stcb, SCTP_STATE_WAS_ABORTED); /* Tell them we lost the asoc */ - sctp_report_all_outbound(stcb, error, 0, so_locked); + sctp_report_all_outbound(stcb, error, so_locked); + SCTP_TCB_SEND_UNLOCK(stcb); if (from_peer) { sctp_ulp_notify(SCTP_NOTIFY_ASSOC_REM_ABORTED, stcb, error, abort, so_locked); } else { @@ -4393,7 +4389,6 @@ sctp_abort_association(struct sctp_inpcb *inp, struct if (stcb != NULL) { /* We have a TCB to abort, send notification too */ sctp_abort_notification(stcb, 0, 0, NULL, SCTP_SO_NOT_LOCKED); - SCTP_ADD_SUBSTATE(stcb, SCTP_STATE_WAS_ABORTED); /* Ok, now lets free it */ SCTP_STAT_INCR_COUNTER32(sctps_aborted); if ((SCTP_GET_STATE(stcb) == SCTP_STATE_OPEN) || @@ -4482,8 +4477,6 @@ sctp_abort_an_association(struct sctp_inpcb *inp, stru } } return; - } else { - SCTP_ADD_SUBSTATE(stcb, SCTP_STATE_WAS_ABORTED); } /* notify the peer */ sctp_send_abort_tcb(stcb, op_err, so_locked); Modified: head/sys/netinet/sctputil.h ============================================================================== --- head/sys/netinet/sctputil.h Sun Aug 16 11:37:28 2020 (r364267) +++ head/sys/netinet/sctputil.h Sun Aug 16 11:50:37 2020 (r364268) @@ -164,7 +164,7 @@ void sctp_stop_timers_for_shutdown(struct sctp_tcb *); /* Stop all timers for association and remote addresses. */ void sctp_stop_association_timers(struct sctp_tcb *, bool); -void sctp_report_all_outbound(struct sctp_tcb *, uint16_t, int, int); +void sctp_report_all_outbound(struct sctp_tcb *, uint16_t, int); int sctp_expand_mapping_array(struct sctp_association *, uint32_t);