netdev - 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]

Message-ID: <CA+=dFzguqjjnnqHO53Mx859VB6NU=4MaPWAUzGQB2qtZGfdAUg@mail.gmail.com>
Date:	Thu, 26 Jul 2012 11:12:57 +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/26/12, Vlad Yasevich <vyasevich@...il.com> wrote:
> Xufeng Zhang <xufengzhang.main@...il.com> wrote:
>
>>On 7/26/12, Xufeng Zhang <xufengzhang.main@...il.com> wrote:
>>> On 7/25/12, Vlad Yasevich <vyasevich@...il.com> wrote:
>>>>> And after take a moment to look into the relative codes, I think we
>>>>> can implement it
>>>>> by below way:
>>>>> 1). Add a flag(isi_err_needed) in the embedded struct peer of
>>struct
>>>>> struct sctp_association
>>>>> just like sack_needed flag.
>>>>> 2). When "invalid stream identifier" ERROR happens in
>>sctp_eat_data()
>>>>> function, we just
>>>>> set isi_err_needed flag and don't create ERROR chunk and also don't
>>>>> insert SCTP_CMD_REPLY command.
>>>>> 3). In sctp_gen_sack() function, we create ERROR chunk and also
>>insert
>>>>> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>>>>>
>>>>> Is this way proper?
>>>>>
>>>>
>>>> So, I looked at the code, and it looks very simple to do.  We
>>already
>>>> return a specific status from sctp_eat_data() when the error was
>>>> generated.  All you have to do is take the code that generates the
>>error
>>>> and adds it to the command list and give it its own small function
>>that
>>>> you can then call if SCTP_IERROR_BAD_STREAM error was returned.
>>>
>>> No, it will still has the same problem by just doing this.
>>> SCTP_CMD_GEN_SACK command actually don't enqueue SACK to outqueue,
>>> sctp_gen_sack() do this things when processing SCTP_CMD_GEN_SACK
>>command
>>> in sctp_cmd_interpreter().
>>> So it's not enough if we just insert SCTP_ERROR_INV_STRM command
>>after
>>> sctp_eat_data() return SCTP_IERROR_BAD_STREAM in
>>sctp_sf_eat_data_6_2().
>>
>>Yes, I just tried this way, SACK is still bundled after ERROR chunk.
>>But I think my above method is also not fine if there are multiple
>>error DATA chunks
>>bundled in a packet.
>>Really awesome!
>
> 1. Catch the error return.
> 2.  Set flag indicating error is needed.
> 3.  Queue sack as needed.
> 4.  If error flag set call new function to queue error chunk.

Both step 3 and 4 need lots of changes to make it working since
SACK is ready to queue only at the end of the packet and we also
need to deal with multiple "invalid stream identifier" ERROR chunks
in a single packet.


Thanks,
Xufeng Zhang
>
> That should fix things.  Do this in all callers of sctp_eat_data.
>
> -vlad
>
>>
>>
>>Thanks,
>>Xufeng Zhang
>>
>>>
>>>
>>>
>>> Thanks,
>>> Xufeng Zhang
>>>
>>>>
>>>> -vlad
>>>>
>>>>>
>>>>> Thanks,
>>>>> Xufeng Zhang
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Xufeng Zhang
>>>>>>>
>>>>>>> -vlad
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Xufeng Zhang
>>>>>>>>>
>>>>>>>>> -vlad
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Sent from my Android phone with SkitMail. Please excuse my
>>brevity.
>>>>>>>
>>>>>>
>>>>
>>>>
>>>>
>>>
>
>
> --
> Sent from my Android phone with SkitMail. Please excuse my brevity.
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html