| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <HE1PR0702MB38180121DAFDF7D7FEA001D5EC5D9@HE1PR0702MB3818.eurprd07.prod.outlook.com>
Date: Sat, 1 May 2021 16:57:45 +0000
From: "Leppanen, Jere (Nokia - FI/Espoo)" <jere.leppanen@...ia.com>
To: Xin Long <lucien.xin@...il.com>,
network dev <netdev@...r.kernel.org>,
"linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
"Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@...ia.com>
Subject: Re: [PATCHv2 net 3/3] sctp: do asoc update earlier in
sctp_sf_do_dupcook_b
On Fri, 30 Apr 2021, Xin Long wrote:
> The same thing should be done for sctp_sf_do_dupcook_b().
> Meanwhile, SCTP_CMD_UPDATE_ASSOC cmd can be removed.
>
> v1->v2:
> - Fix the return value in sctp_sf_do_assoc_update().
>
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> ---
> include/net/sctp/command.h | 1 -
> net/sctp/sm_sideeffect.c | 26 -------------------------
> net/sctp/sm_statefuns.c | 47 +++++++++++++++++++++++++++++-----------------
> 3 files changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index e8df72e..5e84888 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -68,7 +68,6 @@ enum sctp_verb {
> SCTP_CMD_ASSOC_FAILED, /* Handle association failure. */
> SCTP_CMD_DISCARD_PACKET, /* Discard the whole packet. */
> SCTP_CMD_GEN_SHUTDOWN, /* Generate a SHUTDOWN chunk. */
> - SCTP_CMD_UPDATE_ASSOC, /* Update association information. */
> SCTP_CMD_PURGE_OUTQUEUE, /* Purge all data waiting to be sent. */
> SCTP_CMD_SETUP_T2, /* Hi-level, setup T2-shutdown parms. */
> SCTP_CMD_RTO_PENDING, /* Set transport's rto_pending. */
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 0948f14..ce15d59 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -826,28 +826,6 @@ static void sctp_cmd_setup_t2(struct sctp_cmd_seq *cmds,
> asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = t->rto;
> }
>
> -static void sctp_cmd_assoc_update(struct sctp_cmd_seq *cmds,
> - struct sctp_association *asoc,
> - struct sctp_association *new)
> -{
> - struct net *net = asoc->base.net;
> - struct sctp_chunk *abort;
> -
> - if (!sctp_assoc_update(asoc, new))
> - return;
> -
> - abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
> - 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(struct sctp_cmd_seq *cmds,
> struct sctp_association *asoc,
> @@ -1301,10 +1279,6 @@ static int sctp_cmd_interpreter(enum sctp_event_type event_type,
> sctp_endpoint_add_asoc(ep, asoc);
> break;
>
> - case SCTP_CMD_UPDATE_ASSOC:
> - sctp_cmd_assoc_update(commands, asoc, cmd->obj.asoc);
> - break;
> -
> case SCTP_CMD_PURGE_OUTQUEUE:
> sctp_outq_teardown(&asoc->outqueue);
> break;
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index e8ccc4e..a428449 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1773,6 +1773,30 @@ enum sctp_disposition sctp_sf_do_5_2_3_initack(
> return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
> }
>
> +static int sctp_sf_do_assoc_update(struct sctp_association *asoc,
> + struct sctp_association *new,
> + struct sctp_cmd_seq *cmds)
> +{
> + struct net *net = asoc->base.net;
> + struct sctp_chunk *abort;
> +
> + if (!sctp_assoc_update(asoc, new))
> + return 0;
> +
> + abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
> + 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);
> +
> + return -ENOMEM;
> +}
> +
> /* Unexpected COOKIE-ECHO handler for peer restart (Table 2, action 'A')
> *
> * Section 5.2.4
> @@ -1853,21 +1877,8 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
> sctp_add_cmd_sf(commands, SCTP_CMD_PURGE_ASCONF_QUEUE, SCTP_NULL());
>
> /* Update the content of current association. */
> - if (sctp_assoc_update((struct sctp_association *)asoc, new_asoc)) {
> - struct sctp_chunk *abort;
> -
> - abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
> - if (abort) {
> - sctp_init_cause(abort, SCTP_ERROR_RSRC_LOW, 0);
> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
> - }
> - 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_RSRC_LOW));
> - SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS);
> - SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB);
> + if (sctp_sf_do_assoc_update((struct sctp_association *)asoc, new_asoc, commands))
> goto nomem;
> - }
>
> repl = sctp_make_cookie_ack(asoc, chunk);
> if (!repl)
> @@ -1940,14 +1951,16 @@ static enum sctp_disposition sctp_sf_do_dupcook_b(
> if (!sctp_auth_chunk_verify(net, chunk, new_asoc))
> return SCTP_DISPOSITION_DISCARD;
>
> - /* Update the content of current association. */
> - sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
> sctp_add_cmd_sf(commands, SCTP_CMD_NEW_STATE,
> SCTP_STATE(SCTP_STATE_ESTABLISHED));
> SCTP_INC_STATS(net, SCTP_MIB_CURRESTAB);
> sctp_add_cmd_sf(commands, SCTP_CMD_HB_TIMERS_START, SCTP_NULL());
>
> - repl = sctp_make_cookie_ack(new_asoc, chunk);
> + /* Update the content of current association. */
> + if (sctp_sf_do_assoc_update((struct sctp_association *)asoc, new_asoc, commands))
> + goto nomem;
Wouldn't it be better to do the update before SCTP_CMD_NEW_STATE?
Or do we have some reason to move to SCTP_STATE_ESTABLISHED even
if the update fails?
> +
> + repl = sctp_make_cookie_ack(asoc, chunk);
> if (!repl)
> goto nomem;
>
> --
> 2.1.0
Powered by blists - more mailing lists