From bec9640bb0d451813b1bb1f2cc13a5bfb17c3e48 Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Thu, 30 Jul 2009 18:08:28 -0400 Subject: sctp: Disallow new connection on a closing socket If a socket has a lot of association that are in the process of of being closed/aborted, it is possible for a remote to establish new associations during the time period that the old ones are shutting down. If this was a result of a close() call, there will be no socket and will cause a memory leak. We'll prevent this by setting the socket state to CLOSING and disallow new associations when in this state. Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 7288192f7df5..50225dd2e6dc 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -334,6 +334,15 @@ sctp_disposition_t sctp_sf_do_5_1B_init(const struct sctp_endpoint *ep, if (!sctp_chunk_length_valid(chunk, sizeof(sctp_init_chunk_t))) return sctp_sf_pdiscard(ep, asoc, type, arg, commands); + /* If the INIT is coming toward a closing socket, we'll send back + * and ABORT. Essentially, this catches the race of INIT being + * backloged to the socket at the same time as the user isses close(). + * Since the socket and all its associations are going away, we + * can treat this OOTB + */ + if (sctp_sstate(ep->base.sk, CLOSING)) + return sctp_sf_tabort_8_4_8(ep, asoc, type, arg, commands); + /* Verify the INIT chunk before processing it. */ err_chunk = NULL; if (!sctp_verify_init(asoc, chunk->chunk_hdr->type, -- cgit v1.2.2 From 9c5c62be2f794c7cee533d856f9f34c3cf21ff1b Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Mon, 10 Aug 2009 13:51:03 -0400 Subject: sctp: Send user messages to the lower layer as one Currenlty, sctp breaks up user messages into fragments and sends each fragment to the lower layer by itself. This means that for each fragment we go all the way down the stack and back up. This also discourages bundling of multiple fragments when they can fit into a sigle packet (ex: due to user setting a low fragmentation threashold). We introduce a new command SCTP_CMD_SND_MSG and hand the whole message down state machine. The state machine and the side-effect parser will cork the queue, add all chunks from the message to the queue, and then un-cork the queue thus causing the chunks to get transmitted. Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 50225dd2e6dc..910926906a3a 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -4555,9 +4555,9 @@ sctp_disposition_t sctp_sf_do_prm_send(const struct sctp_endpoint *ep, void *arg, sctp_cmd_seq_t *commands) { - struct sctp_chunk *chunk = arg; + struct sctp_datamsg *msg = arg; - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(chunk)); + sctp_add_cmd_sf(commands, SCTP_CMD_SEND_MSG, SCTP_DATAMSG(msg)); return SCTP_DISPOSITION_CONSUME; } -- cgit v1.2.2 From a2f36eec5647548fa94fb68e2843b00fb9c0d46b Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Sat, 22 Aug 2009 11:24:00 +0800 Subject: sctp: drop SHUTDOWN chunk if the TSN is less than the CTSN If Cumulative TSN Ack field of SHUTDOWN chunk is less than the Cumulative TSN Ack Point then drop the SHUTDOWN chunk. Signed-off-by: Wei Yongjun Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 910926906a3a..73bdeb2b6c62 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2570,6 +2570,12 @@ sctp_disposition_t sctp_sf_do_9_2_shutdown(const struct sctp_endpoint *ep, chunk->subh.shutdown_hdr = sdh; ctsn = ntohl(sdh->cum_tsn_ack); + if (TSN_lt(ctsn, asoc->ctsn_ack_point)) { + SCTP_DEBUG_PRINTK("ctsn %x\n", ctsn); + SCTP_DEBUG_PRINTK("ctsn_ack_point %x\n", asoc->ctsn_ack_point); + return SCTP_DISPOSITION_DISCARD; + } + /* If Cumulative TSN Ack beyond the max tsn currently * send, terminating the association and respond to the * sender with an ABORT. @@ -2633,6 +2639,7 @@ sctp_disposition_t sctp_sf_do_9_2_shut_ctsn(const struct sctp_endpoint *ep, { struct sctp_chunk *chunk = arg; sctp_shutdownhdr_t *sdh; + __u32 ctsn; if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(ep, asoc, type, arg, commands); @@ -2644,12 +2651,19 @@ sctp_disposition_t sctp_sf_do_9_2_shut_ctsn(const struct sctp_endpoint *ep, commands); sdh = (sctp_shutdownhdr_t *)chunk->skb->data; + ctsn = ntohl(sdh->cum_tsn_ack); + + if (TSN_lt(ctsn, asoc->ctsn_ack_point)) { + SCTP_DEBUG_PRINTK("ctsn %x\n", ctsn); + SCTP_DEBUG_PRINTK("ctsn_ack_point %x\n", asoc->ctsn_ack_point); + return SCTP_DISPOSITION_DISCARD; + } /* If Cumulative TSN Ack beyond the max tsn currently * send, terminating the association and respond to the * sender with an ABORT. */ - if (!TSN_lt(ntohl(sdh->cum_tsn_ack), asoc->next_tsn)) + if (!TSN_lt(ctsn, asoc->next_tsn)) return sctp_sf_violation_ctsn(ep, asoc, type, arg, commands); /* verify, by checking the Cumulative TSN Ack field of the -- cgit v1.2.2 From dadb50cc1ada2906594df83d991f0bc388039bb6 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Sat, 22 Aug 2009 11:27:37 +0800 Subject: sctp: fix check the chunk length of received HEARTBEAT-ACK chunk The receiver of the HEARTBEAT should respond with a HEARTBEAT ACK that contains the Heartbeat Information field copied from the received HEARTBEAT chunk. So the received HEARTBEAT-ACK chunk must have a length of: sizeof(sctp_chunkhdr_t) + sizeof(sctp_sender_hb_info_t) A badly formatted HB-ACK chunk, it is possible that we may access invalid memory. We should really make sure that the chunk format is what we expect, before attempting to touch the data. Signed-off-by: Wei Yongjun Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 73bdeb2b6c62..7fb08a694917 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -1115,7 +1115,8 @@ sctp_disposition_t sctp_sf_backbeat_8_3(const struct sctp_endpoint *ep, return sctp_sf_pdiscard(ep, asoc, type, arg, commands); /* Make sure that the HEARTBEAT-ACK chunk has a valid length. */ - if (!sctp_chunk_length_valid(chunk, sizeof(sctp_heartbeat_chunk_t))) + if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t) + + sizeof(sctp_sender_hb_info_t))) return sctp_sf_violation_chunklen(ep, asoc, type, arg, commands); -- cgit v1.2.2 From b9f8478682445c2a3e0b87718a0563ef543ad94e Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Wed, 26 Aug 2009 09:36:25 -0400 Subject: sctp: Fix error count increments that were results of HEARTBEATS SCTP RFC 4960 states that unacknowledged HEARTBEATS count as errors agains a given transport or endpoint. As such, we should increment the error counts for only for unacknowledged HB, otherwise we detect failure too soon. This goes for both the overall error count and the path error count. Now, there is a difference in how the detection is done between the two. The path error detection is done after the increment, so to detect it properly, we actually need to exceed the path threshold. The overall error detection is done _BEFORE_ the increment. Thus to detect the failure, it's enough for the error count to match the threshold. This is why all the state functions use '>=' to detect failure, while path detection uses '>'. Thanks goes to Chunbo Luo who first proposed patches to fix this issue and made me re-read the spec and the code to figure out how this cruft really works. Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 7fb08a694917..45b8bcafd827 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -971,7 +971,7 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const struct sctp_endpoint *ep, { struct sctp_transport *transport = (struct sctp_transport *) arg; - if (asoc->overall_error_count > asoc->max_retrans) { + if (asoc->overall_error_count >= asoc->max_retrans) { sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ETIMEDOUT)); /* CMD_ASSOC_FAILED calls CMD_DELETE_TCB. */ -- cgit v1.2.2 From 31b02e1549406efa346534acad956a42bc3f28c4 Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Fri, 4 Sep 2009 18:21:00 -0400 Subject: sctp: Failover transmitted list on transport delete Add-IP feature allows users to delete an active transport. If that transport has chunks in flight, those chunks need to be moved to another transport or association may get into unrecoverable state. Reported-by: Rafael Laufer Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 45b8bcafd827..a7f18a352364 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -3543,6 +3543,12 @@ sctp_disposition_t sctp_sf_do_asconf(const struct sctp_endpoint *ep, asconf_ack = sctp_assoc_lookup_asconf_ack(asoc, hdr->serial); if (!asconf_ack) return SCTP_DISPOSITION_DISCARD; + + /* Reset the transport so that we select the correct one + * this time around. This is to make sure that we don't + * accidentally use a stale transport that's been removed. + */ + asconf_ack->transport = NULL; } else { /* ADDIP 5.2 E5) Otherwise, the ASCONF Chunk is discarded since * it must be either a stale packet or from an attacker. -- cgit v1.2.2 From f1751c57f7bb816c9b6b4cb5d79c703aaa7199da Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Fri, 4 Sep 2009 18:21:03 -0400 Subject: sctp: Catch bogus stream sequence numbers Since our TSN map is capable of holding at most a 4K chunk gap, there is no way that during this gap, a stream sequence number (unsigned short) can wrap such that the new number is smaller then the next expected one. If such a case is encountered, this is a protocol violation. Signed-off-by: Vlad Yasevich --- net/sctp/sm_statefuns.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) (limited to 'net/sctp/sm_statefuns.c') diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index a7f18a352364..c8fae1983dd1 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2891,6 +2891,9 @@ sctp_disposition_t sctp_sf_eat_data_6_2(const struct sctp_endpoint *ep, goto discard_force; case SCTP_IERROR_NO_DATA: goto consume; + case SCTP_IERROR_PROTO_VIOLATION: + return sctp_sf_abort_violation(ep, asoc, chunk, commands, + (u8 *)chunk->subh.data_hdr, sizeof(sctp_datahdr_t)); default: BUG(); } @@ -3001,6 +3004,9 @@ sctp_disposition_t sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep, break; case SCTP_IERROR_NO_DATA: goto consume; + case SCTP_IERROR_PROTO_VIOLATION: + return sctp_sf_abort_violation(ep, asoc, chunk, commands, + (u8 *)chunk->subh.data_hdr, sizeof(sctp_datahdr_t)); default: BUG(); } @@ -5877,6 +5883,9 @@ static int sctp_eat_data(const struct sctp_association *asoc, __u32 tsn; struct sctp_tsnmap *map = (struct sctp_tsnmap *)&asoc->peer.tsn_map; struct sock *sk = asoc->base.sk; + u16 ssn; + u16 sid; + u8 ordered = 0; data_hdr = chunk->subh.data_hdr = (sctp_datahdr_t *)chunk->skb->data; skb_pull(chunk->skb, sizeof(sctp_datahdr_t)); @@ -6016,8 +6025,10 @@ static int sctp_eat_data(const struct sctp_association *asoc, */ if (chunk->chunk_hdr->flags & SCTP_DATA_UNORDERED) SCTP_INC_STATS(SCTP_MIB_INUNORDERCHUNKS); - else + else { SCTP_INC_STATS(SCTP_MIB_INORDERCHUNKS); + ordered = 1; + } /* RFC 2960 6.5 Stream Identifier and Stream Sequence Number * @@ -6027,7 +6038,8 @@ static int sctp_eat_data(const struct sctp_association *asoc, * with cause set to "Invalid Stream Identifier" (See Section 3.3.10) * and discard the DATA chunk. */ - if (ntohs(data_hdr->stream) >= asoc->c.sinit_max_instreams) { + sid = ntohs(data_hdr->stream); + if (sid >= asoc->c.sinit_max_instreams) { /* Mark tsn as received even though we drop it */ sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn)); @@ -6040,6 +6052,18 @@ static int sctp_eat_data(const struct sctp_association *asoc, return SCTP_IERROR_BAD_STREAM; } + /* Check to see if the SSN is possible for this TSN. + * The biggest gap we can record is 4K wide. Since SSNs wrap + * at an unsigned short, there is no way that an SSN can + * wrap and for a valid TSN. We can simply check if the current + * SSN is smaller then the next expected one. If it is, it wrapped + * and is invalid. + */ + ssn = ntohs(data_hdr->ssn); + if (ordered && SSN_lt(ssn, sctp_ssn_peek(&asoc->ssnmap->in, sid))) { + return SCTP_IERROR_PROTO_VIOLATION; + } + /* Send the data up to the user. Note: Schedule the * SCTP_CMD_CHUNK_ULP cmd before the SCTP_CMD_GEN_SACK, as the SACK * chunk needs the updated rwnd. -- cgit v1.2.2