* [PATCH net-next] sctp: handle errors when updating asoc
@ 2017-06-20 8:05 Xin Long
2017-06-20 13:21 ` Neil Horman
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Xin Long @ 2017-06-20 8:05 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
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@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] sctp: handle errors when updating asoc
2017-06-20 8:05 [PATCH net-next] sctp: handle errors when updating asoc Xin Long
@ 2017-06-20 13:21 ` Neil Horman
2017-06-20 15:44 ` Marcelo Ricardo Leitner
2017-06-20 19:33 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Neil Horman @ 2017-06-20 13:21 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem
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@gmail.com>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] sctp: handle errors when updating asoc
2017-06-20 8:05 [PATCH net-next] sctp: handle errors when updating asoc Xin Long
2017-06-20 13:21 ` Neil Horman
@ 2017-06-20 15:44 ` Marcelo Ricardo Leitner
2017-06-20 19:33 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-06-20 15:44 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
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@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] sctp: handle errors when updating asoc
2017-06-20 8:05 [PATCH net-next] sctp: handle errors when updating asoc Xin Long
2017-06-20 13:21 ` Neil Horman
2017-06-20 15:44 ` Marcelo Ricardo Leitner
@ 2017-06-20 19:33 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-06-20 19:33 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 20 Jun 2017 16:05:11 +0800
> 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@gmail.com>
Also applied, thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-06-20 19:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-20 8:05 [PATCH net-next] sctp: handle errors when updating asoc Xin Long
2017-06-20 13:21 ` Neil Horman
2017-06-20 15:44 ` Marcelo Ricardo Leitner
2017-06-20 19:33 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).