linux-kernel - Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 30 Jul 2012 12:58:01 +0800
From:	Xufeng Zhang <xufengzhang.main@...il.com>
To:	Vlad Yasevich <vyasevich@...il.com>
Cc:	Neil Horman <nhorman@...driver.com>,
	xufeng zhang <xufeng.zhang@...driver.com>, sri@...ibm.com,
	davem@...emloft.net, linux-sctp@...r.kernel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK
 when bundling

On 7/28/12, Vlad Yasevich <vyasevich@...il.com> wrote:
> here is an untested prototype of what I was talking about.  This should
> handle multiple data chunks.

Yes, it works if only the end of the DATA chunk in a packet has
invalid stream identifier
and I have verified this patch by my test case, but what happens if
there are multiple
DATA chunks which have invalid stream identifier in a packet?

Consider the below example:
A packet has several chunks bundling together: "COOKIE_ECHO DATA DATA", both
of the two DATA chunks have invalid stream identifier, then the
response will be
"COOKIE_ACK ERROR SACK ERROR", right?



Thanks,
Xufeng Zhang

>
> -vlad
>
> ---
>   include/net/sctp/command.h |    1 +
>   net/sctp/sm_sideeffect.c   |   22 ++++++++++++++++++++++
>   net/sctp/sm_statefuns.c    |   18 ++++++++++--------
>   3 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index 712b3be..4043445 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -110,6 +110,7 @@ typedef enum {
>   	SCTP_CMD_SEND_NEXT_ASCONF, /* Send the next ASCONF after ACK */
>   	SCTP_CMD_PURGE_ASCONF_QUEUE, /* Purge all asconf queues.*/
>   	SCTP_CMD_SET_ASOC,	 /* Restore association context */
> +	SCTP_CMD_GEN_BAD_STREAM, /* Issue an Invalid Stream error */
>   	SCTP_CMD_LAST
>   } sctp_verb_t;
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 1ff51c9..c5a1322 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1036,6 +1036,22 @@ static void sctp_cmd_send_asconf(struct
> sctp_association *asoc)
>   	}
>   }
>
> +static void sctp_cmd_make_inv_stream_err(sctp_cmd_seq_t *commands,
> +					 struct sctp_association *asoc,
> +					 struct sctp_chunk *chunk,
> +					 struct sctp_datahdr *data_hdr)
> +{
> +	struct sctp_chunk *err;
> +
> +	err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM,
> +				 &data_hdr->stream,
> +				 sizeof(data_hdr->stream),
> +				 sizeof(u16));
> +	if (err)
> +		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> +					SCTP_CHUNK(err));
> +}
> +
>
>   /* These three macros allow us to pull the debugging code out of the
>    * main flow of sctp_do_sm() to keep attention focused on the real
> @@ -1700,6 +1716,12 @@ static int sctp_cmd_interpreter(sctp_event_t
> event_type,
>   			asoc = cmd->obj.asoc;
>   			break;
>
> +		case SCTP_CMD_GEN_BAD_STREAM:
> +			sctp_cmd_make_inv_stream_err(commands,
> +					 asoc, chunk,
> +					 (struct sctp_datahdr *)cmd->obj.ptr);
> +			break;
> +
>   		default:
>   			pr_warn("Impossible command: %u, %p\n",
>   				cmd->verb, cmd->obj.ptr);
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 891f5db..57532e3 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -2972,6 +2972,12 @@ discard_noforce:
>   	if (chunk->end_of_packet)
>   		sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
>
> +	/* Queue the INVALID STREAM error after the SACK if one is needed. */
> +	if (SCTP_IERROR_BAD_STREAM == error) {
> +		sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
> +				SCTP_PTR(chunk->subh.data_hdr));
> +	}
> +
>   	return SCTP_DISPOSITION_DISCARD;
>   consume:
>   	return SCTP_DISPOSITION_CONSUME;
> @@ -3044,6 +3050,10 @@ sctp_disposition_t
> sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep,
>   		 */
>   		sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN, SCTP_NULL());
>   		sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE());
> +		if (SCTP_IERROR_BAD_STREAM == error) {
> +			sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
> +					SCTP_PTR(chunk->subh.data_hdr));
> +		}
>   		sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
>   				SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
>   	}
> @@ -6140,14 +6150,6 @@ static int sctp_eat_data(const struct
> sctp_association *asoc,
>   	if (sid >= asoc->c.sinit_max_instreams) {
>   		/* Mark tsn as received even though we drop it */
>   		sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn));
> -
> -		err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM,
> -					 &data_hdr->stream,
> -					 sizeof(data_hdr->stream),
> -					 sizeof(u16));
> -		if (err)
> -			sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> -					SCTP_CHUNK(err));
>   		return SCTP_IERROR_BAD_STREAM;
>   	}
>
> -- 1.7.7.6
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/