sctp: move the format error check out of __sctp_sf_do_9_1_abort

When T2 timer is to be stopped, the asoc should also be deleted,
otherwise, there will be no chance to call sctp_association_free
and the asoc could last in memory forever.

However, in sctp_sf_shutdown_sent_abort(), after adding the cmd
SCTP_CMD_TIMER_STOP for T2 timer, it may return error due to the
format error from __sctp_sf_do_9_1_abort() and miss adding
SCTP_CMD_ASSOC_FAILED where the asoc will be deleted.

This patch is to fix it by moving the format error check out of
__sctp_sf_do_9_1_abort(), and do it before adding the cmd
SCTP_CMD_TIMER_STOP for T2 timer.

Thanks Hangbin for reporting this issue by the fuzz testing.

v1->v2:
  - improve the comment in the code as Marcelo's suggestion.

Fixes: 96ca468b86 ("sctp: check invalid value of length parameter in error cause")
Reported-by: Hangbin Liu <liuhangbin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Xin Long 2020-02-18 12:07:53 +08:00 committed by David S. Miller
parent 8a9093c798
commit 245709ec8b
1 changed files with 20 additions and 9 deletions

View File

@ -170,6 +170,16 @@ static inline bool sctp_chunk_length_valid(struct sctp_chunk *chunk,
return true; return true;
} }
/* Check for format error in an ABORT chunk */
static inline bool sctp_err_chunk_valid(struct sctp_chunk *chunk)
{
struct sctp_errhdr *err;
sctp_walk_errors(err, chunk->chunk_hdr);
return (void *)err == (void *)chunk->chunk_end;
}
/********************************************************** /**********************************************************
* These are the state functions for handling chunk events. * These are the state functions for handling chunk events.
**********************************************************/ **********************************************************/
@ -2255,6 +2265,9 @@ enum sctp_disposition sctp_sf_shutdown_pending_abort(
sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest)) sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest))
return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands); return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
if (!sctp_err_chunk_valid(chunk))
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
return __sctp_sf_do_9_1_abort(net, ep, asoc, type, arg, commands); return __sctp_sf_do_9_1_abort(net, ep, asoc, type, arg, commands);
} }
@ -2298,6 +2311,9 @@ enum sctp_disposition sctp_sf_shutdown_sent_abort(
sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest)) sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest))
return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands); return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
if (!sctp_err_chunk_valid(chunk))
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
/* Stop the T2-shutdown timer. */ /* Stop the T2-shutdown timer. */
sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_STOP, sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_STOP,
SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN)); SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
@ -2565,6 +2581,9 @@ enum sctp_disposition sctp_sf_do_9_1_abort(
sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest)) sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest))
return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands); return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
if (!sctp_err_chunk_valid(chunk))
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
return __sctp_sf_do_9_1_abort(net, ep, asoc, type, arg, commands); return __sctp_sf_do_9_1_abort(net, ep, asoc, type, arg, commands);
} }
@ -2582,16 +2601,8 @@ static enum sctp_disposition __sctp_sf_do_9_1_abort(
/* See if we have an error cause code in the chunk. */ /* See if we have an error cause code in the chunk. */
len = ntohs(chunk->chunk_hdr->length); len = ntohs(chunk->chunk_hdr->length);
if (len >= sizeof(struct sctp_chunkhdr) + sizeof(struct sctp_errhdr)) { if (len >= sizeof(struct sctp_chunkhdr) + sizeof(struct sctp_errhdr))
struct sctp_errhdr *err;
sctp_walk_errors(err, chunk->chunk_hdr);
if ((void *)err != (void *)chunk->chunk_end)
return sctp_sf_pdiscard(net, ep, asoc, type, arg,
commands);
error = ((struct sctp_errhdr *)chunk->skb->data)->cause; error = ((struct sctp_errhdr *)chunk->skb->data)->cause;
}
sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNRESET)); sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNRESET));
/* ASSOC_FAILED will DELETE_TCB. */ /* ASSOC_FAILED will DELETE_TCB. */