From e629b9fc5654920213c73cd91cbd0c949efc2395 Mon Sep 17 00:00:00 2001 From: Michael Tuexen Date: Fri, 11 Sep 2015 13:54:33 +0000 Subject: [PATCH] Ensure that ERROR chunks are always padded by implementing this in the routine, which queues an ERROR chunk, instead on relyinh on the callers to do so. Since one caller missed this, this actially fixes a bug. MFC after: 1 week --- sys/netinet/sctp_constants.h | 2 ++ sys/netinet/sctp_indata.c | 6 +---- sys/netinet/sctp_input.c | 12 ++++------ sys/netinet/sctp_output.c | 44 +++++++++++++++++++++++++----------- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/sys/netinet/sctp_constants.h b/sys/netinet/sctp_constants.h index 760c3b472cc..a26bc5b5c33 100644 --- a/sys/netinet/sctp_constants.h +++ b/sys/netinet/sctp_constants.h @@ -66,6 +66,8 @@ __FBSDID("$FreeBSD$"); */ #define SCTP_LARGEST_INIT_ACCEPTED (65535 - 2048) +/* Largest length of a chunk */ +#define SCTP_MAX_CHUNK_LENGTH 0xffff /* Number of addresses where we just skip the counting */ #define SCTP_COUNT_LIMIT 40 diff --git a/sys/netinet/sctp_indata.c b/sys/netinet/sctp_indata.c index 7d92655cc74..d9ca6694678 100644 --- a/sys/netinet/sctp_indata.c +++ b/sys/netinet/sctp_indata.c @@ -2513,11 +2513,7 @@ sctp_process_data(struct mbuf **mm, int iphlen, int *offset, int length, SCTP_BUF_LEN(merr) = sizeof(*phd); SCTP_BUF_NEXT(merr) = SCTP_M_COPYM(m, *offset, chk_length, M_NOWAIT); if (SCTP_BUF_NEXT(merr)) { - if (sctp_pad_lastmbuf(SCTP_BUF_NEXT(merr), SCTP_SIZE32(chk_length) - chk_length, NULL) == NULL) { - sctp_m_freem(merr); - } else { - sctp_queue_op_err(stcb, merr); - } + sctp_queue_op_err(stcb, merr); } else { sctp_m_freem(merr); } diff --git a/sys/netinet/sctp_input.c b/sys/netinet/sctp_input.c index 297ddf2c788..9f8d032888b 100644 --- a/sys/netinet/sctp_input.c +++ b/sys/netinet/sctp_input.c @@ -5602,16 +5602,12 @@ process_control_chunks: SCTP_BUF_LEN(mm) = sizeof(*phd); SCTP_BUF_NEXT(mm) = SCTP_M_COPYM(m, *offset, len, M_NOWAIT); if (SCTP_BUF_NEXT(mm)) { - if (sctp_pad_lastmbuf(SCTP_BUF_NEXT(mm), SCTP_SIZE32(len) - len, NULL) == NULL) { - sctp_m_freem(mm); - } else { #ifdef SCTP_MBUF_LOGGING - if (SCTP_BASE_SYSCTL(sctp_logging_level) & SCTP_MBUF_LOGGING_ENABLE) { - sctp_log_mbc(SCTP_BUF_NEXT(mm), SCTP_MBUF_ICOPY); - } -#endif - sctp_queue_op_err(stcb, mm); + if (SCTP_BASE_SYSCTL(sctp_logging_level) & SCTP_MBUF_LOGGING_ENABLE) { + sctp_log_mbc(SCTP_BUF_NEXT(mm), SCTP_MBUF_ICOPY); } +#endif + sctp_queue_op_err(stcb, mm); } else { sctp_m_freem(mm); } diff --git a/sys/netinet/sctp_output.c b/sys/netinet/sctp_output.c index 772ec949a9a..f515d6a6cd2 100644 --- a/sys/netinet/sctp_output.c +++ b/sys/netinet/sctp_output.c @@ -8850,9 +8850,37 @@ sctp_queue_op_err(struct sctp_tcb *stcb, struct mbuf *op_err) */ struct sctp_chunkhdr *hdr; struct sctp_tmit_chunk *chk; - struct mbuf *mat; + struct mbuf *mat, *last_mbuf; + uint32_t chunk_length; + uint16_t padding_length; SCTP_TCB_LOCK_ASSERT(stcb); + SCTP_BUF_PREPEND(op_err, sizeof(struct sctp_chunkhdr), M_NOWAIT); + if (op_err == NULL) { + return; + } + last_mbuf = NULL; + chunk_length = 0; + for (mat = op_err; mat != NULL; mat = SCTP_BUF_NEXT(mat)) { + chunk_length += SCTP_BUF_LEN(mat); + if (SCTP_BUF_NEXT(mat) == NULL) { + last_mbuf = mat; + } + } + if (chunk_length > SCTP_MAX_CHUNK_LENGTH) { + sctp_m_freem(op_err); + return; + } + padding_length = chunk_length % 4; + if (padding_length != 0) { + padding_length = 4 - padding_length; + } + if (padding_length != 0) { + if (sctp_add_pad_tombuf(last_mbuf, padding_length) == NULL) { + sctp_m_freem(op_err); + return; + } + } sctp_alloc_a_chunk(stcb, chk); if (chk == NULL) { /* no memory */ @@ -8860,15 +8888,7 @@ sctp_queue_op_err(struct sctp_tcb *stcb, struct mbuf *op_err) return; } chk->copy_by_ref = 0; - SCTP_BUF_PREPEND(op_err, sizeof(struct sctp_chunkhdr), M_NOWAIT); - if (op_err == NULL) { - sctp_free_a_chunk(stcb, chk, SCTP_SO_NOT_LOCKED); - return; - } - chk->send_size = 0; - for (mat = op_err; mat != NULL; mat = SCTP_BUF_NEXT(mat)) { - chk->send_size += SCTP_BUF_LEN(mat); - } + chk->send_size = (uint16_t) chunk_length; chk->sent = SCTP_DATAGRAM_UNSENT; chk->snd_count = 0; chk->asoc = &stcb->asoc; @@ -8878,9 +8898,7 @@ sctp_queue_op_err(struct sctp_tcb *stcb, struct mbuf *op_err) hdr->chunk_type = SCTP_OPERATION_ERROR; hdr->chunk_flags = 0; hdr->chunk_length = htons(chk->send_size); - TAILQ_INSERT_TAIL(&chk->asoc->control_send_queue, - chk, - sctp_next); + TAILQ_INSERT_TAIL(&chk->asoc->control_send_queue, chk, sctp_next); chk->asoc->ctrl_queue_cnt++; }