On Tue, Jun 20, 2017 at 04:05:11PM +0800, Xin Long wrote: > It's a bad thing not to handle errors when updating asoc. The memory > allocation failure in any of the functions called in sctp_assoc_update() > would cause sctp to work unexpectedly. > > This patch is to fix it by aborting the asoc and reporting the error when > any of these functions fails. > > Signed-off-by: Xin Long <lucien.xin@xxxxxxxxx> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx> > --- > include/net/sctp/structs.h | 4 ++-- > net/sctp/associola.c | 25 ++++++++++++++----------- > net/sctp/sm_sideeffect.c | 24 +++++++++++++++++++++++- > 3 files changed, 39 insertions(+), 14 deletions(-) > > diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h > index 5051317..e26763b 100644 > --- a/include/net/sctp/structs.h > +++ b/include/net/sctp/structs.h > @@ -1953,8 +1953,8 @@ struct sctp_transport *sctp_assoc_is_match(struct sctp_association *, > const union sctp_addr *, > const union sctp_addr *); > void sctp_assoc_migrate(struct sctp_association *, struct sock *); > -void sctp_assoc_update(struct sctp_association *old, > - struct sctp_association *new); > +int sctp_assoc_update(struct sctp_association *old, > + struct sctp_association *new); > > __u32 sctp_association_get_next_tsn(struct sctp_association *); > > diff --git a/net/sctp/associola.c b/net/sctp/associola.c > index 72b07dd..757be41 100644 > --- a/net/sctp/associola.c > +++ b/net/sctp/associola.c > @@ -1112,8 +1112,8 @@ void sctp_assoc_migrate(struct sctp_association *assoc, struct sock *newsk) > } > > /* Update an association (possibly from unexpected COOKIE-ECHO processing). */ > -void sctp_assoc_update(struct sctp_association *asoc, > - struct sctp_association *new) > +int sctp_assoc_update(struct sctp_association *asoc, > + struct sctp_association *new) > { > struct sctp_transport *trans; > struct list_head *pos, *temp; > @@ -1124,8 +1124,10 @@ void sctp_assoc_update(struct sctp_association *asoc, > asoc->peer.sack_needed = new->peer.sack_needed; > asoc->peer.auth_capable = new->peer.auth_capable; > asoc->peer.i = new->peer.i; > - sctp_tsnmap_init(&asoc->peer.tsn_map, SCTP_TSN_MAP_INITIAL, > - asoc->peer.i.initial_tsn, GFP_ATOMIC); > + > + if (!sctp_tsnmap_init(&asoc->peer.tsn_map, SCTP_TSN_MAP_INITIAL, > + asoc->peer.i.initial_tsn, GFP_ATOMIC)) > + return -ENOMEM; > > /* Remove any peer addresses not present in the new association. */ > list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) { > @@ -1169,11 +1171,11 @@ void sctp_assoc_update(struct sctp_association *asoc, > } else { > /* Add any peer addresses from the new association. */ > list_for_each_entry(trans, &new->peer.transport_addr_list, > - transports) { > - if (!sctp_assoc_lookup_paddr(asoc, &trans->ipaddr)) > - sctp_assoc_add_peer(asoc, &trans->ipaddr, > - GFP_ATOMIC, trans->state); > - } > + transports) > + if (!sctp_assoc_lookup_paddr(asoc, &trans->ipaddr) && > + !sctp_assoc_add_peer(asoc, &trans->ipaddr, > + GFP_ATOMIC, trans->state)) > + return -ENOMEM; > > asoc->ctsn_ack_point = asoc->next_tsn - 1; > asoc->adv_peer_ack_point = asoc->ctsn_ack_point; > @@ -1182,7 +1184,8 @@ void sctp_assoc_update(struct sctp_association *asoc, > sctp_stream_update(&asoc->stream, &new->stream); > > /* get a new assoc id if we don't have one yet. */ > - sctp_assoc_set_id(asoc, GFP_ATOMIC); > + if (sctp_assoc_set_id(asoc, GFP_ATOMIC)) > + return -ENOMEM; > } > > /* SCTP-AUTH: Save the peer parameters from the new associations > @@ -1200,7 +1203,7 @@ void sctp_assoc_update(struct sctp_association *asoc, > asoc->peer.peer_hmacs = new->peer.peer_hmacs; > new->peer.peer_hmacs = NULL; > > - sctp_auth_asoc_init_active_key(asoc, GFP_ATOMIC); > + return sctp_auth_asoc_init_active_key(asoc, GFP_ATOMIC); > } > > /* Update the retran path for sending a retransmitted packet. > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c > index 7623566..dfe1fcb 100644 > --- a/net/sctp/sm_sideeffect.c > +++ b/net/sctp/sm_sideeffect.c > @@ -818,6 +818,28 @@ static void sctp_cmd_setup_t2(sctp_cmd_seq_t *cmds, > asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = t->rto; > } > > +static void sctp_cmd_assoc_update(sctp_cmd_seq_t *cmds, > + struct sctp_association *asoc, > + struct sctp_association *new) > +{ > + struct net *net = sock_net(asoc->base.sk); > + struct sctp_chunk *abort; > + > + if (!sctp_assoc_update(asoc, new)) > + return; > + > + abort = sctp_make_abort(asoc, NULL, sizeof(sctp_errhdr_t)); > + if (abort) { > + sctp_init_cause(abort, SCTP_ERROR_RSRC_LOW, 0); > + sctp_add_cmd_sf(cmds, SCTP_CMD_REPLY, SCTP_CHUNK(abort)); > + } > + sctp_add_cmd_sf(cmds, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNABORTED)); > + sctp_add_cmd_sf(cmds, SCTP_CMD_ASSOC_FAILED, > + SCTP_PERR(SCTP_ERROR_RSRC_LOW)); > + SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS); > + SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB); > +} > + > /* Helper function to change the state of an association. */ > static void sctp_cmd_new_state(sctp_cmd_seq_t *cmds, > struct sctp_association *asoc, > @@ -1294,7 +1316,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type, > break; > > case SCTP_CMD_UPDATE_ASSOC: > - sctp_assoc_update(asoc, cmd->obj.asoc); > + sctp_cmd_assoc_update(commands, asoc, cmd->obj.asoc); > break; > > case SCTP_CMD_PURGE_OUTQUEUE: > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html