Author: tuexen
Date: Sat Apr  7 19:56:34 2018
New Revision: 332213
URL: https://svnweb.freebsd.org/changeset/base/332213

Log:
  MFC r324638:
  
  Fix the handling of parital and too short chunks.
  
  Ensure that the current behaviour is consistent: stop processing
  of the chunk, but finish the processing of the previous chunks.
  
  This behaviour might be changed in a later commit to ABORT the
  assoication due to a protocol violation, but changing this
  is a separate issue.
  
  MFC r324725
  
  Fix a bug introduced in r324638.
  Thanks to Felix Weinrank for making me aware of this.
  
  MFC r324726:
  
  Revert change which got in accidently.

Modified:
  stable/11/sys/netinet/sctp_input.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netinet/sctp_input.c
==============================================================================
--- stable/11/sys/netinet/sctp_input.c  Sat Apr  7 19:44:41 2018        
(r332212)
+++ stable/11/sys/netinet/sctp_input.c  Sat Apr  7 19:56:34 2018        
(r332213)
@@ -4521,7 +4521,6 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
         * until we get into jumbo grams and such..
         */
        uint8_t chunk_buf[SCTP_CHUNK_BUFFER_SIZE];
-       struct sctp_tcb *locked_tcb = stcb;
        int got_auth = 0;
        uint32_t auth_offset = 0, auth_len = 0;
        int auth_skipped = 0;
@@ -4533,31 +4532,29 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
        SCTPDBG(SCTP_DEBUG_INPUT1, "sctp_process_control: iphlen=%u, offset=%u, 
length=%u stcb:%p\n",
            iphlen, *offset, length, (void *)stcb);
 
+       if (stcb) {
+               SCTP_TCB_LOCK_ASSERT(stcb);
+       }
        /* validate chunk header length... */
        if (ntohs(ch->chunk_length) < sizeof(*ch)) {
                SCTPDBG(SCTP_DEBUG_INPUT1, "Invalid header length %d\n",
                    ntohs(ch->chunk_length));
-               if (locked_tcb) {
-                       SCTP_TCB_UNLOCK(locked_tcb);
-               }
-               return (NULL);
+               *offset = length;
+               return (stcb);
        }
        /*
         * validate the verification tag
         */
        vtag_in = ntohl(sh->v_tag);
 
-       if (locked_tcb) {
-               SCTP_TCB_LOCK_ASSERT(locked_tcb);
-       }
        if (ch->chunk_type == SCTP_INITIATION) {
                SCTPDBG(SCTP_DEBUG_INPUT1, "Its an INIT of len:%d vtag:%x\n",
                    ntohs(ch->chunk_length), vtag_in);
                if (vtag_in != 0) {
                        /* protocol error- silently discard... */
                        SCTP_STAT_INCR(sctps_badvtag);
-                       if (locked_tcb) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
+                       if (stcb != NULL) {
+                               SCTP_TCB_UNLOCK(stcb);
                        }
                        return (NULL);
                }
@@ -4580,9 +4577,6 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                        if (*offset >= length) {
                                /* no more data left in the mbuf chain */
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                return (NULL);
                        }
                        ch = (struct sctp_chunkhdr *)sctp_m_getptr(m, *offset,
@@ -4591,10 +4585,7 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                if (ch == NULL) {
                        /* Help */
                        *offset = length;
-                       if (locked_tcb) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
-                       }
-                       return (NULL);
+                       return (stcb);
                }
                if (ch->chunk_type == SCTP_COOKIE_ECHO) {
                        goto process_control_chunks;
@@ -4631,10 +4622,7 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                                 * sctp_findassociation_ep_asconf().
                                 */
                                SCTP_INP_DECR_REF(inp);
-                       } else {
-                               locked_tcb = stcb;
                        }
-
                        /* now go back and verify any auth chunk to be sure */
                        if (auth_skipped && (stcb != NULL)) {
                                struct sctp_auth_chunk *auth;
@@ -4648,10 +4636,7 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                                    auth_offset)) {
                                        /* auth HMAC failed so dump it */
                                        *offset = length;
-                                       if (locked_tcb) {
-                                               SCTP_TCB_UNLOCK(locked_tcb);
-                                       }
-                                       return (NULL);
+                                       return (stcb);
                                } else {
                                        /* remaining chunks are HMAC checked */
                                        stcb->asoc.authenticated = 1;
@@ -4667,9 +4652,6 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                            mflowtype, mflowid, inp->fibnum,
                            vrf_id, port);
                        *offset = length;
-                       if (locked_tcb) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
-                       }
                        return (NULL);
                }
                asoc = &stcb->asoc;
@@ -4687,8 +4669,8 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                        } else {
                                /* drop this packet... */
                                SCTP_STAT_INCR(sctps_badvtag);
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
+                               if (stcb != NULL) {
+                                       SCTP_TCB_UNLOCK(stcb);
                                }
                                return (NULL);
                        }
@@ -4701,8 +4683,8 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                                 * but it won't complete until the shutdown
                                 * is completed
                                 */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
+                               if (stcb != NULL) {
+                                       SCTP_TCB_UNLOCK(stcb);
                                }
                                snprintf(msg, sizeof(msg), "OOTB, %s:%d at %s", 
__FILE__, __LINE__, __func__);
                                op_err = 
sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code),
@@ -4721,8 +4703,8 @@ sctp_process_control(struct mbuf *m, int iphlen, int *
                                    "invalid vtag: %xh, expect %xh\n",
                                    vtag_in, asoc->my_vtag);
                                SCTP_STAT_INCR(sctps_badvtag);
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
+                               if (stcb != NULL) {
+                                       SCTP_TCB_UNLOCK(stcb);
                                }
                                *offset = length;
                                return (NULL);
@@ -4758,10 +4740,7 @@ process_control_chunks:
                if (chk_length < sizeof(*ch) ||
                    (*offset + (int)chk_length) > length) {
                        *offset = length;
-                       if (locked_tcb) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
-                       }
-                       return (NULL);
+                       return (stcb);
                }
                SCTP_STAT_INCR_COUNTER64(sctps_incontrolchunks);
                /*
@@ -4776,8 +4755,8 @@ process_control_chunks:
                            sizeof(struct sctp_init_ack_chunk), chunk_buf);
                        if (ch == NULL) {
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
+                               if (stcb != NULL) {
+                                       SCTP_TCB_UNLOCK(stcb);
                                }
                                return (NULL);
                        }
@@ -4799,8 +4778,8 @@ process_control_chunks:
                                    chunk_buf);
                                if (ch == NULL) {
                                        *offset = length;
-                                       if (locked_tcb) {
-                                               SCTP_TCB_UNLOCK(locked_tcb);
+                                       if (stcb != NULL) {
+                                               SCTP_TCB_UNLOCK(stcb);
                                        }
                                        return (NULL);
                                }
@@ -4811,8 +4790,8 @@ process_control_chunks:
                                if (ch == NULL) {
                                        SCTP_PRINTF("sctp_process_control: 
Can't get the all data....\n");
                                        *offset = length;
-                                       if (locked_tcb) {
-                                               SCTP_TCB_UNLOCK(locked_tcb);
+                                       if (stcb != NULL) {
+                                               SCTP_TCB_UNLOCK(stcb);
                                        }
                                        return (NULL);
                                }
@@ -4852,8 +4831,8 @@ process_control_chunks:
                            (length - *offset > (int)SCTP_SIZE32(chk_length))) {
                                /* RFC 4960 requires that no ABORT is sent */
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
+                               if (stcb != NULL) {
+                                       SCTP_TCB_UNLOCK(stcb);
                                }
                                return (NULL);
                        }
@@ -4873,8 +4852,8 @@ process_control_chunks:
                            mflowtype, mflowid,
                            vrf_id, port);
                        *offset = length;
-                       if ((!abort_no_unlock) && (locked_tcb)) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
+                       if ((!abort_no_unlock) && (stcb != NULL)) {
+                               SCTP_TCB_UNLOCK(stcb);
                        }
                        return (NULL);
                        break;
@@ -4884,15 +4863,14 @@ process_control_chunks:
                        SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_INIT-ACK\n");
                        if (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) {
                                /* We are not interested anymore */
-                               if ((stcb) && 
(stcb->asoc.total_output_queue_size)) {
+                               if ((stcb != NULL) && 
(stcb->asoc.total_output_queue_size)) {
                                        ;
                                } else {
-                                       if ((locked_tcb != NULL) && (locked_tcb 
!= stcb)) {
-                                               /* Very unlikely */
-                                               SCTP_TCB_UNLOCK(locked_tcb);
+                                       if (stcb != NULL) {
+                                               SCTP_TCB_UNLOCK(stcb);
                                        }
                                        *offset = length;
-                                       if (stcb) {
+                                       if (stcb != NULL) {
 #if defined(__APPLE__) || defined(SCTP_SO_LOCK_TESTING)
                                                so = SCTP_INP_SO(inp);
                                                
atomic_add_int(&stcb->asoc.refcnt, 1);
@@ -4914,10 +4892,7 @@ process_control_chunks:
                        if ((num_chunks > 1) ||
                            (length - *offset > (int)SCTP_SIZE32(chk_length))) {
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
-                               return (NULL);
+                               return (stcb);
                        }
                        if ((netp) && (*netp)) {
                                ret = sctp_handle_init_ack(m, iphlen, *offset,
@@ -4941,10 +4916,7 @@ process_control_chunks:
                        if ((stcb != NULL) && (ret == 0)) {
                                sctp_chunk_output(stcb->sctp_ep, stcb, 
SCTP_OUTPUT_FROM_CONTROL_PROC, SCTP_SO_NOT_LOCKED);
                        }
-                       if (locked_tcb) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
-                       }
-                       return (NULL);
+                       return (stcb);
                        break;
                case SCTP_SELECTIVE_ACK:
                        {
@@ -5136,10 +5108,7 @@ process_control_chunks:
                        if ((stcb == NULL) || (chk_length != sizeof(struct 
sctp_heartbeat_chunk))) {
                                /* Its not ours */
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
-                               return (NULL);
+                               return (stcb);
                        }
                        /* He's alive so give him credit */
                        if (SCTP_BASE_SYSCTL(sctp_logging_level) & 
SCTP_THRESHOLD_LOGGING) {
@@ -5169,10 +5138,7 @@ process_control_chunks:
                            (void *)stcb);
                        if ((stcb == NULL) || (chk_length != sizeof(struct 
sctp_shutdown_chunk))) {
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
-                               return (NULL);
+                               return (stcb);
                        }
                        if (netp && *netp) {
                                int abort_flag = 0;
@@ -5209,7 +5175,7 @@ process_control_chunks:
                                if (inp->sctp_flags & 
SCTP_PCB_FLAGS_SOCKET_GONE) {
                                        /* We are not interested anymore */
                        abend:
-                                       if (stcb) {
+                                       if (stcb != NULL) {
                                                SCTP_TCB_UNLOCK(stcb);
                                        }
                                        *offset = length;
@@ -5254,6 +5220,9 @@ process_control_chunks:
                                        }
                                }
                                if (netp) {
+                                       struct sctp_tcb *locked_stcb;
+
+                                       locked_stcb = stcb;
                                        ret_buf =
                                            sctp_handle_cookie_echo(m, iphlen,
                                            *offset,
@@ -5264,11 +5233,17 @@ process_control_chunks:
                                            auth_skipped,
                                            auth_offset,
                                            auth_len,
-                                           &locked_tcb,
+                                           &locked_stcb,
                                            mflowtype,
                                            mflowid,
                                            vrf_id,
                                            port);
+                                       if ((locked_stcb != NULL) && 
(locked_stcb != stcb)) {
+                                               SCTP_TCB_UNLOCK(locked_stcb);
+                                       }
+                                       if (stcb != NULL) {
+                                               SCTP_TCB_LOCK_ASSERT(stcb);
+                                       }
                                } else {
                                        ret_buf = NULL;
                                }
@@ -5276,8 +5251,8 @@ process_control_chunks:
                                        SCTP_ASOC_CREATE_UNLOCK(linp);
                                }
                                if (ret_buf == NULL) {
-                                       if (locked_tcb) {
-                                               SCTP_TCB_UNLOCK(locked_tcb);
+                                       if (stcb != NULL) {
+                                               SCTP_TCB_UNLOCK(stcb);
                                        }
                                        SCTPDBG(SCTP_DEBUG_INPUT3,
                                            "GAK, null buffer\n");
@@ -5304,10 +5279,7 @@ process_control_chunks:
                case SCTP_COOKIE_ACK:
                        SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_COOKIE-ACK, stcb 
%p\n", (void *)stcb);
                        if ((stcb == NULL) || chk_length != sizeof(struct 
sctp_cookie_ack_chunk)) {
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
-                               return (NULL);
+                               return (stcb);
                        }
                        if (inp->sctp_flags & SCTP_PCB_FLAGS_SOCKET_GONE) {
                                /* We are not interested anymore */
@@ -5349,11 +5321,8 @@ process_control_chunks:
                        /* He's alive so give him credit */
                        if ((stcb == NULL) || (chk_length != sizeof(struct 
sctp_ecne_chunk))) {
                                /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        if (stcb) {
                                if (stcb->asoc.ecn_supported == 0) {
@@ -5376,12 +5345,8 @@ process_control_chunks:
                        SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_ECN-CWR\n");
                        /* He's alive so give him credit */
                        if ((stcb == NULL) || (chk_length != sizeof(struct 
sctp_cwr_chunk))) {
-                               /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        if (stcb) {
                                if (stcb->asoc.ecn_supported == 0) {
@@ -5404,10 +5369,7 @@ process_control_chunks:
                        if ((num_chunks > 1) ||
                            (length - *offset > (int)SCTP_SIZE32(chk_length))) {
                                *offset = length;
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
-                               return (NULL);
+                               return (stcb);
                        }
                        if ((stcb) && netp && *netp) {
                                sctp_handle_shutdown_complete((struct 
sctp_shutdown_complete_chunk *)ch,
@@ -5440,11 +5402,8 @@ process_control_chunks:
                        SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_ASCONF-ACK\n");
                        if (chk_length < sizeof(struct sctp_asconf_ack_chunk)) {
                                /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        if ((stcb) && netp && *netp) {
                                if (stcb->asoc.asconf_supported == 0) {
@@ -5470,11 +5429,8 @@ process_control_chunks:
                        SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_FWD-TSN\n");
                        if (chk_length < sizeof(struct sctp_forward_tsn_chunk)) 
{
                                /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        /* He's alive so give him credit */
                        if (stcb) {
@@ -5537,11 +5493,8 @@ process_control_chunks:
                        SCTPDBG(SCTP_DEBUG_INPUT3, "SCTP_STREAM_RESET\n");
                        if (((stcb == NULL) || (ch == NULL) || (chk_length < 
sizeof(struct sctp_stream_reset_tsn_req)))) {
                                /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        if (stcb->asoc.reconfig_supported == 0) {
                                goto unknown_chunk;
@@ -5557,11 +5510,8 @@ process_control_chunks:
                        /* re-get it all please */
                        if (chk_length < sizeof(struct sctp_pktdrop_chunk)) {
                                /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        if (ch && (stcb) && netp && (*netp)) {
                                if (stcb->asoc.pktdrop_supported == 0) {
@@ -5570,7 +5520,6 @@ process_control_chunks:
                                sctp_handle_packet_dropped((struct 
sctp_pktdrop_chunk *)ch,
                                    stcb, *netp,
                                    min(chk_length, (sizeof(chunk_buf) - 4)));
-
                        }
                        break;
                case SCTP_AUTHENTICATION:
@@ -5592,11 +5541,8 @@ process_control_chunks:
                            (chk_length > (sizeof(struct sctp_auth_chunk) +
                            SCTP_AUTH_DIGEST_LEN_MAX))) {
                                /* Its not ours */
-                               if (locked_tcb) {
-                                       SCTP_TCB_UNLOCK(locked_tcb);
-                               }
                                *offset = length;
-                               return (NULL);
+                               return (stcb);
                        }
                        if (got_auth == 1) {
                                /* skip this chunk... it's already auth'd */
@@ -5661,11 +5607,8 @@ next_chunk:
                ch = (struct sctp_chunkhdr *)sctp_m_getptr(m, *offset,
                    sizeof(struct sctp_chunkhdr), chunk_buf);
                if (ch == NULL) {
-                       if (locked_tcb) {
-                               SCTP_TCB_UNLOCK(locked_tcb);
-                       }
                        *offset = length;
-                       return (NULL);
+                       return (stcb);
                }
        }                       /* while */
 
_______________________________________________
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