From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xin Long Subject: Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave Date: Tue, 12 Dec 2017 21:31:36 +0800 Message-ID: References: <807b1707f79b11d5883d71ea771e98e8f7af3ead.1513070662.git.lucien.xin@gmail.com> <20171212131853.GB3532@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: network dev , linux-sctp@vger.kernel.org, Neil Horman , davem To: Marcelo Ricardo Leitner Return-path: Received: from mail-qt0-f194.google.com ([209.85.216.194]:44906 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754156AbdLLNbh (ORCPT ); Tue, 12 Dec 2017 08:31:37 -0500 In-Reply-To: <20171212131853.GB3532@localhost.localdomain> Sender: netdev-owner@vger.kernel.org List-ID: On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner wrote: > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote: > ... >> --- a/net/sctp/sm_statetable.c >> +++ b/net/sctp/sm_statetable.c >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup( >> return &chunk_event_table[cid][state]; >> >> if (net->sctp.prsctp_enable) { >> - if (cid == SCTP_CID_FWD_TSN) >> + if (cid == SCTP_CID_FWD_TSN || >> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN)) > > We don't really need to check intl_enable here, do we? We think net->sctp.xxx_enable as a main switch here, that means it only accepts SCTP_CID_I_FWD_TSN when this switch is open. As for the check in validate_ftsn(), it follows the RFC checking on asoc's intl_enable. does it make sense from this perspective ? > Because a) it should actually look like: > - if (cid == SCTP_CID_FWD_TSN) > + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) || > + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN)) > but b) we will validate the chunk format/feature later with > validate_ftsn(), similarly to what happens with data chunks, so it > seems the check on intl_enable here is not necessary. > > Same happens with data chunks, btw. > >> return &prsctp_chunk_event_table[0][state]; >> } >> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c >> index 2ead372..cc4a5e3 100644 >> --- a/net/sctp/stream_interleave.c >> +++ b/net/sctp/stream_interleave.c >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn) >> } >> } >> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \ >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \ >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++) >> + >> +#define sctp_walk_ifwdtsn(pos, ch) \ >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \ >> + sizeof(struct sctp_ifwdtsn_chunk)) >> + >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk) >> +{ >> + struct sctp_fwdtsn_skip *skip; >> + __u16 incnt; >> + >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN) >> + return false; >> + >> + incnt = chunk->asoc->stream.incnt; >> + sctp_walk_fwdtsn(skip, chunk) >> + if (ntohs(skip->stream) >= incnt) >> + return false; >> + >> + return true; >> +} >> + >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk) >> +{ >> + struct sctp_ifwdtsn_skip *skip; >> + __u16 incnt; >> + >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN) >> + return false; >> + >> + incnt = chunk->asoc->stream.incnt; >> + sctp_walk_ifwdtsn(skip, chunk) >> + if (ntohs(skip->stream) >= incnt) >> + return false; >> + >> + return true; >> +} >> + >> static struct sctp_stream_interleave sctp_stream_interleave_0 = { >> .data_chunk_len = sizeof(struct sctp_data_chunk), >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk), >> /* DATA process functions */ >> .make_datafrag = sctp_make_datafrag_empty, >> .assign_number = sctp_chunk_assign_ssn, >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = { >> .abort_pd = sctp_ulpq_abort_pd, >> /* FORWARD-TSN process functions */ >> .generate_ftsn = sctp_generate_fwdtsn, >> + .validate_ftsn = sctp_validate_fwdtsn, >> }; >> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = { >> .data_chunk_len = sizeof(struct sctp_idata_chunk), >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk), >> /* I-DATA process functions */ >> .make_datafrag = sctp_make_idatafrag_empty, >> .assign_number = sctp_chunk_assign_mid, >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = { >> .abort_pd = sctp_intl_abort_pd, >> /* I-FORWARD-TSN process functions */ >> .generate_ftsn = sctp_generate_iftsn, >> + .validate_ftsn = sctp_validate_iftsn, >> }; >> >> void sctp_stream_interleave_init(struct sctp_stream *stream) >> -- >> 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 >>