| 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: <501B3643.5000605@windriver.com>
Date: Fri, 3 Aug 2012 10:24:03 +0800
From: xufeng zhang <xufeng.zhang@...driver.com>
To: Vlad Yasevich <vyasevich@...il.com>
CC: Xufeng Zhang <xufengzhang.main@...il.com>,
Neil Horman <nhorman@...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 08/03/2012 05:17 AM, Vlad Yasevich wrote:
> On 07/31/2012 02:51 AM, xufeng zhang wrote:
>> Sorry, please ignore the above patch, there was an paste error.
>> Please check the following patch.
>> ============================================
>> I'm wondering if the below solution is fine to you which is based on
>> your changes.
>> BTW, I have verified this patch and it works ok for all the situation,
>> but only one problem persists:
>> there is a potential that commands will exceeds SCTP_MAX_NUM_COMMANDS
>> which happens during sending lots of small error DATA chunks.
>>
>
> I started thinking along the same vein, but was thinking that maybe it
> makes sense to make error list more generic. I need to check the spec
> on the ordering of ERROR chunks. If they are always after other
> control chunks, then maybe make an error list and queue all errors
> there. Then when sending control chunks, drain the control queue
> first, then the error queue, and finally the data queue.
I didn't find the explicitly bundling order description in spec for
other ERROR chunks,
but it's reasonable to do this.
Anyway, I'll wait for your final solution.
Thanks,
Xufeng Zhang
>
> BTW, the patch below doesn't include the code to queue the error chunk
> onto the new error queue.
>
> -vlad
>
>> Thanks,
>> Xufeng Zhang
>>
>> ---
>> include/net/sctp/command.h | 1 +
>> include/net/sctp/structs.h | 3 +++
>> net/sctp/outqueue.c | 7 +++++++
>> net/sctp/sm_sideeffect.c | 16 ++++++++++++++++
>> net/sctp/sm_statefuns.c | 17 ++++++++++++++---
>> 5 files changed, 41 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
>> index 712b3be..62c34f5 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, /* Invalid Stream errors happened
>> command */
>> SCTP_CMD_LAST
>> } sctp_verb_t;
>>
>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>> index fc5e600..3d218e0 100644
>> --- a/include/net/sctp/structs.h
>> +++ b/include/net/sctp/structs.h
>> @@ -1183,6 +1183,9 @@ struct sctp_outq {
>> */
>> struct list_head abandoned;
>>
>> + /* Put Invalid Stream error chunks on this list */
>> + struct list_head bad_stream_err;
>> +
>> /* How many unackd bytes do we have in-flight? */
>> __u32 outstanding_bytes;
>>
>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>> index e7aa177..1e87b0b 100644
>> --- a/net/sctp/outqueue.c
>> +++ b/net/sctp/outqueue.c
>> @@ -211,6 +211,7 @@ void sctp_outq_init(struct sctp_association *asoc,
>> struct sctp_outq *q)
>> INIT_LIST_HEAD(&q->retransmit);
>> INIT_LIST_HEAD(&q->sacked);
>> INIT_LIST_HEAD(&q->abandoned);
>> + INIT_LIST_HEAD(&q->bad_stream_err);
>>
>> q->fast_rtx = 0;
>> q->outstanding_bytes = 0;
>> @@ -283,6 +284,12 @@ void sctp_outq_teardown(struct sctp_outq *q)
>> list_del_init(&chunk->list);
>> sctp_chunk_free(chunk);
>> }
>> +
>> + /* Throw away any pending Invalid Stream error chunks */
>> + list_for_each_entry_safe(chunk, tmp,&q->bad_stream_err, list) {
>> + list_del_init(&chunk->list);
>> + sctp_chunk_free(chunk);
>> + }
>> }
>>
>> /* Free the outqueue structure and any related pending chunks. */
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index fe99628..4698593 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -1060,6 +1060,18 @@ 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 *err, *tmp;
>> + struct sctp_outq *q =&asoc->outqueue;
>> +
>> + list_for_each_entry_safe(err, tmp,&q->bad_stream_err, list) {
>> + list_del_init(&err->list);
>> + 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
>> @@ -1724,6 +1736,10 @@ 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);
>> + 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 9fca103..1c1bcd9 100644
>> --- a/net/sctp/sm_statefuns.c
>> +++ b/net/sctp/sm_statefuns.c
>> @@ -2967,8 +2967,14 @@ discard_force:
>> return SCTP_DISPOSITION_DISCARD;
>>
>> discard_noforce:
>> - if (chunk->end_of_packet)
>> + if (chunk->end_of_packet) {
>> + struct sctp_outq *q =&asoc->outqueue;
>> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
>> + /* Queue the INVALID STREAM error after the SACK if one
>> is needed. */
>> + if (!list_empty(&q->bad_stream_err))
>> + sctp_add_cmd_sf(commands,
>> SCTP_CMD_GEN_BAD_STREAM,
>> + SCTP_NULL());
>> + }
>>
>> return SCTP_DISPOSITION_DISCARD;
>> consume:
>> @@ -3037,11 +3043,16 @@ sctp_disposition_t
>> sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep,
>> * with a SACK, a SHUTDOWN chunk, and restart the T2-shutdown
>> timer
>> */
>> if (chunk->end_of_packet) {
>> + struct sctp_outq *q =&asoc->outqueue;
>> /* We must delay the chunk creation since the
>> cumulative
>> * TSN has not been updated yet.
>> */
>> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN,
>> SCTP_NULL());
>> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK,
>> SCTP_FORCE());
>> + /* Queue the INVALID STREAM error after the SACK if one
>> is needed. */
>> + if (!list_empty(&q->bad_stream_err))
>> + sctp_add_cmd_sf(commands,
>> SCTP_CMD_GEN_BAD_STREAM,
>> + SCTP_NULL());
>> sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
>>
>> SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
>> }
>> @@ -6136,6 +6147,7 @@ static int sctp_eat_data(const struct
>> sctp_association *asoc,
>> */
>> sid = ntohs(data_hdr->stream);
>> if (sid>= asoc->c.sinit_max_instreams) {
>> + struct sctp_outq *q =&asoc->outqueue;
>> /* Mark tsn as received even though we drop it */
>> sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN,
>> SCTP_U32(tsn));
>>
>> @@ -6144,8 +6156,7 @@ static int sctp_eat_data(const struct
>> sctp_association *asoc,
>> sizeof(data_hdr->stream),
>> sizeof(u16));
>> if (err)
>> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
>> - SCTP_CHUNK(err));
>> + list_add_tail(&err->list,&q->bad_stream_err);
>> return SCTP_IERROR_BAD_STREAM;
>> }
>>
>
>
--
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/
Powered by blists - more mailing lists