Skip to content

Commit

Permalink
SCTP: Clean up OOTB handling and fix infinite loop processing
Browse files Browse the repository at this point in the history 
While processing OOTB chunks as well as chunks with an invalid
length of 0, it was possible to SCTP to get wedged inside an
infinite loop because we didn't catch the condition correctly,
or didn't mark the packet for discard correctly.
This work is based on original findings and work by
Wei Yongjun <yjwei@cn.fujitsu.com>

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
  • Loading branch information
Vlad Yasevich authored and David S. Miller committed Sep 26, 2007
1 parent d3f2596 commit ece25df
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 42 deletions.
1 change: 0 additions & 1 deletion include/net/sctp/sm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ sctp_state_fn_t sctp_sf_do_4_C;
sctp_state_fn_t sctp_sf_eat_data_6_2;
sctp_state_fn_t sctp_sf_eat_data_fast_4_4;
sctp_state_fn_t sctp_sf_eat_sack_6_2;
sctp_state_fn_t sctp_sf_tabort_8_4_8;
sctp_state_fn_t sctp_sf_operr_notify;
sctp_state_fn_t sctp_sf_t1_init_timer_expire;
sctp_state_fn_t sctp_sf_t1_cookie_timer_expire;
Expand Down
163 changes: 130 additions & 33 deletions net/sctp/sm_statefuns.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ static sctp_disposition_t sctp_sf_shut_8_4_5(const struct sctp_endpoint *ep,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands);
static sctp_disposition_t sctp_sf_tabort_8_4_8(const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands);
static struct sctp_sackhdr *sctp_sm_pull_sack(struct sctp_chunk *chunk);

static sctp_disposition_t sctp_stop_t1_and_abort(sctp_cmd_seq_t *commands,
Expand All @@ -98,6 +103,7 @@ static sctp_disposition_t sctp_stop_t1_and_abort(sctp_cmd_seq_t *commands,
struct sctp_transport *transport);

static sctp_disposition_t sctp_sf_abort_violation(
const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
void *arg,
sctp_cmd_seq_t *commands,
Expand All @@ -118,6 +124,13 @@ static sctp_disposition_t sctp_sf_violation_ctsn(
void *arg,
sctp_cmd_seq_t *commands);

static sctp_disposition_t sctp_sf_violation_chunk(
const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands);

/* Small helper function that checks if the chunk length
* is of the appropriate length. The 'required_length' argument
* is set to be the size of a specific chunk we are testing.
Expand Down Expand Up @@ -181,16 +194,21 @@ sctp_disposition_t sctp_sf_do_4_C(const struct sctp_endpoint *ep,
struct sctp_chunk *chunk = arg;
struct sctp_ulpevent *ev;

if (!sctp_vtag_verify_either(chunk, asoc))
return sctp_sf_pdiscard(ep, asoc, type, arg, commands);

/* RFC 2960 6.10 Bundling
*
* An endpoint MUST NOT bundle INIT, INIT ACK or
* SHUTDOWN COMPLETE with any other chunks.
*/
if (!chunk->singleton)
return SCTP_DISPOSITION_VIOLATION;
return sctp_sf_violation_chunk(ep, asoc, type, arg, commands);

if (!sctp_vtag_verify_either(chunk, asoc))
return sctp_sf_pdiscard(ep, asoc, type, arg, commands);
/* Make sure that the SHUTDOWN_COMPLETE chunk has a valid length. */
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

/* RFC 2960 10.2 SCTP-to-ULP
*
Expand Down Expand Up @@ -450,17 +468,17 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep,
if (!sctp_vtag_verify(chunk, asoc))
return sctp_sf_pdiscard(ep, asoc, type, arg, commands);

/* Make sure that the INIT-ACK chunk has a valid length */
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_initack_chunk_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);
/* 6.10 Bundling
* An endpoint MUST NOT bundle INIT, INIT ACK or
* SHUTDOWN COMPLETE with any other chunks.
*/
if (!chunk->singleton)
return SCTP_DISPOSITION_VIOLATION;
return sctp_sf_violation_chunk(ep, asoc, type, arg, commands);

/* Make sure that the INIT-ACK chunk has a valid length */
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_initack_chunk_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);
/* Grab the INIT header. */
chunk->subh.init_hdr = (sctp_inithdr_t *) chunk->skb->data;

Expand Down Expand Up @@ -585,7 +603,7 @@ sctp_disposition_t sctp_sf_do_5_1D_ce(const struct sctp_endpoint *ep,
* control endpoint, respond with an ABORT.
*/
if (ep == sctp_sk((sctp_get_ctl_sock()))->ep)
return sctp_sf_ootb(ep, asoc, type, arg, commands);
return sctp_sf_tabort_8_4_8(ep, asoc, type, arg, commands);

/* Make sure that the COOKIE_ECHO chunk has a valid length.
* In this case, we check that we have enough for at least a
Expand Down Expand Up @@ -2496,6 +2514,11 @@ sctp_disposition_t sctp_sf_do_9_2_reshutack(const struct sctp_endpoint *ep,
struct sctp_chunk *chunk = (struct sctp_chunk *) arg;
struct sctp_chunk *reply;

/* Make sure that the chunk has a valid length */
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

/* Since we are not going to really process this INIT, there
* is no point in verifying chunk boundries. Just generate
* the SHUTDOWN ACK.
Expand Down Expand Up @@ -2929,7 +2952,7 @@ sctp_disposition_t sctp_sf_eat_sack_6_2(const struct sctp_endpoint *ep,
*
* The return value is the disposition of the chunk.
*/
sctp_disposition_t sctp_sf_tabort_8_4_8(const struct sctp_endpoint *ep,
static sctp_disposition_t sctp_sf_tabort_8_4_8(const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const sctp_subtype_t type,
void *arg,
Expand Down Expand Up @@ -3126,14 +3149,14 @@ sctp_disposition_t sctp_sf_ootb(const struct sctp_endpoint *ep,

ch = (sctp_chunkhdr_t *) chunk->chunk_hdr;
do {
/* Break out if chunk length is less then minimal. */
/* Report violation if the chunk is less then minimal */
if (ntohs(ch->length) < sizeof(sctp_chunkhdr_t))
break;

ch_end = ((__u8 *)ch) + WORD_ROUND(ntohs(ch->length));
if (ch_end > skb_tail_pointer(skb))
break;
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

/* Now that we know we at least have a chunk header,
* do things that are type appropriate.
*/
if (SCTP_CID_SHUTDOWN_ACK == ch->type)
ootb_shut_ack = 1;

Expand All @@ -3145,6 +3168,12 @@ sctp_disposition_t sctp_sf_ootb(const struct sctp_endpoint *ep,
if (SCTP_CID_ABORT == ch->type)
return sctp_sf_pdiscard(ep, asoc, type, arg, commands);

/* Report violation if chunk len overflows */
ch_end = ((__u8 *)ch) + WORD_ROUND(ntohs(ch->length));
if (ch_end > skb_tail_pointer(skb))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

ch = (sctp_chunkhdr_t *) ch_end;
} while (ch_end < skb_tail_pointer(skb));

Expand Down Expand Up @@ -3244,6 +3273,13 @@ sctp_disposition_t sctp_sf_do_8_5_1_E_sa(const struct sctp_endpoint *ep,
void *arg,
sctp_cmd_seq_t *commands)
{
struct sctp_chunk *chunk = arg;

/* Make sure that the SHUTDOWN_ACK chunk has a valid length. */
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

/* Although we do have an association in this case, it corresponds
* to a restarted association. So the packet is treated as an OOTB
* packet and the state function that handles OOTB SHUTDOWN_ACK is
Expand Down Expand Up @@ -3658,6 +3694,16 @@ sctp_disposition_t sctp_sf_discard_chunk(const struct sctp_endpoint *ep,
void *arg,
sctp_cmd_seq_t *commands)
{
struct sctp_chunk *chunk = arg;

/* Make sure that the chunk has a valid length.
* Since we don't know the chunk type, we use a general
* chunkhdr structure to make a comparison.
*/
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

SCTP_DEBUG_PRINTK("Chunk %d is discarded\n", type.chunk);
return SCTP_DISPOSITION_DISCARD;
}
Expand Down Expand Up @@ -3713,19 +3759,28 @@ sctp_disposition_t sctp_sf_violation(const struct sctp_endpoint *ep,
void *arg,
sctp_cmd_seq_t *commands)
{
struct sctp_chunk *chunk = arg;

/* Make sure that the chunk has a valid length. */
if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t)))
return sctp_sf_violation_chunklen(ep, asoc, type, arg,
commands);

return SCTP_DISPOSITION_VIOLATION;
}

/*
* Common function to handle a protocol violation.
*/
static sctp_disposition_t sctp_sf_abort_violation(
const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
void *arg,
sctp_cmd_seq_t *commands,
const __u8 *payload,
const size_t paylen)
{
struct sctp_packet *packet = NULL;
struct sctp_chunk *chunk = arg;
struct sctp_chunk *abort = NULL;

Expand All @@ -3734,30 +3789,51 @@ static sctp_disposition_t sctp_sf_abort_violation(
if (!abort)
goto nomem;

sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS);
if (asoc) {
sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS);

if (asoc->state <= SCTP_STATE_COOKIE_ECHOED) {
sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_STOP,
SCTP_TO(SCTP_EVENT_TIMEOUT_T1_INIT));
sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR,
SCTP_ERROR(ECONNREFUSED));
sctp_add_cmd_sf(commands, SCTP_CMD_INIT_FAILED,
SCTP_PERR(SCTP_ERROR_PROTO_VIOLATION));
if (asoc->state <= SCTP_STATE_COOKIE_ECHOED) {
sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_STOP,
SCTP_TO(SCTP_EVENT_TIMEOUT_T1_INIT));
sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR,
SCTP_ERROR(ECONNREFUSED));
sctp_add_cmd_sf(commands, SCTP_CMD_INIT_FAILED,
SCTP_PERR(SCTP_ERROR_PROTO_VIOLATION));
} else {
sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR,
SCTP_ERROR(ECONNABORTED));
sctp_add_cmd_sf(commands, SCTP_CMD_ASSOC_FAILED,
SCTP_PERR(SCTP_ERROR_PROTO_VIOLATION));
SCTP_DEC_STATS(SCTP_MIB_CURRESTAB);
}
} else {
sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR,
SCTP_ERROR(ECONNABORTED));
sctp_add_cmd_sf(commands, SCTP_CMD_ASSOC_FAILED,
SCTP_PERR(SCTP_ERROR_PROTO_VIOLATION));
SCTP_DEC_STATS(SCTP_MIB_CURRESTAB);
packet = sctp_ootb_pkt_new(asoc, chunk);

if (!packet)
goto nomem_pkt;

if (sctp_test_T_bit(abort))
packet->vtag = ntohl(chunk->sctp_hdr->vtag);

abort->skb->sk = ep->base.sk;

sctp_packet_append_chunk(packet, abort);

sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT,
SCTP_PACKET(packet));

SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS);
}

sctp_add_cmd_sf(commands, SCTP_CMD_DISCARD_PACKET, SCTP_NULL());
sctp_sf_pdiscard(ep, asoc, SCTP_ST_CHUNK(0), arg, commands);

SCTP_INC_STATS(SCTP_MIB_ABORTEDS);

return SCTP_DISPOSITION_ABORT;

nomem_pkt:
sctp_chunk_free(abort);
nomem:
return SCTP_DISPOSITION_NOMEM;
}
Expand Down Expand Up @@ -3790,7 +3866,7 @@ static sctp_disposition_t sctp_sf_violation_chunklen(
{
char err_str[]="The following chunk had invalid length:";

return sctp_sf_abort_violation(asoc, arg, commands, err_str,
return sctp_sf_abort_violation(ep, asoc, arg, commands, err_str,
sizeof(err_str));
}

Expand All @@ -3809,10 +3885,31 @@ static sctp_disposition_t sctp_sf_violation_ctsn(
{
char err_str[]="The cumulative tsn ack beyond the max tsn currently sent:";

return sctp_sf_abort_violation(asoc, arg, commands, err_str,
return sctp_sf_abort_violation(ep, asoc, arg, commands, err_str,
sizeof(err_str));
}

/* Handle protocol violation of an invalid chunk bundling. For example,
* when we have an association and we recieve bundled INIT-ACK, or
* SHUDOWN-COMPLETE, our peer is clearly violationg the "MUST NOT bundle"
* statement from the specs. Additinally, there might be an attacker
* on the path and we may not want to continue this communication.
*/
static sctp_disposition_t sctp_sf_violation_chunk(
const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands)
{
char err_str[]="The following chunk violates protocol:";

if (!asoc)
return sctp_sf_violation(ep, asoc, type, arg, commands);

return sctp_sf_abort_violation(ep, asoc, arg, commands, err_str,
sizeof(err_str));
}
/***************************************************************************
* These are the state functions for handling primitive (Section 10) events.
***************************************************************************/
Expand Down
16 changes: 8 additions & 8 deletions net/sctp/sm_statetable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand Down Expand Up @@ -173,7 +173,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand All @@ -194,7 +194,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand All @@ -216,7 +216,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_violation), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand Down Expand Up @@ -258,7 +258,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand Down Expand Up @@ -300,7 +300,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand Down Expand Up @@ -499,7 +499,7 @@ static const sctp_sm_table_entry_t addip_chunk_event_table[SCTP_NUM_ADDIP_CHUNK_
/* SCTP_STATE_EMPTY */ \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_CLOSED */ \
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
TYPE_SCTP_FUNC(sctp_sf_ootb), \
/* SCTP_STATE_COOKIE_WAIT */ \
TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
/* SCTP_STATE_COOKIE_ECHOED */ \
Expand Down Expand Up @@ -528,7 +528,7 @@ chunk_event_table_unknown[SCTP_STATE_NUM_STATES] = {
/* SCTP_STATE_EMPTY */
TYPE_SCTP_FUNC(sctp_sf_ootb),
/* SCTP_STATE_CLOSED */
TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8),
TYPE_SCTP_FUNC(sctp_sf_ootb),
/* SCTP_STATE_COOKIE_WAIT */
TYPE_SCTP_FUNC(sctp_sf_unk_chunk),
/* SCTP_STATE_COOKIE_ECHOED */
Expand Down

0 comments on commit ece25df

Please sign in to comment.