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:          megan2013...@protonmail.com
  Reported by:          syzbot+bc02585076c3cc977...@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);
 
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to