From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Subject: Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave Date: Tue, 12 Dec 2017 11:18:53 -0200 Message-ID: <20171212131853.GB3532@localhost.localdomain> References: <807b1707f79b11d5883d71ea771e98e8f7af3ead.1513070662.git.lucien.xin@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: network dev , linux-sctp@vger.kernel.org, Neil Horman , davem@davemloft.net To: Xin Long Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58016 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754837AbdLLNS5 (ORCPT ); Tue, 12 Dec 2017 08:18:57 -0500 Content-Disposition: inline In-Reply-To: <807b1707f79b11d5883d71ea771e98e8f7af3ead.1513070662.git.lucien.xin@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: 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? 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Date: Tue, 12 Dec 2017 13:18:53 +0000 Subject: Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave Message-Id: <20171212131853.GB3532@localhost.localdomain> List-Id: References: <807b1707f79b11d5883d71ea771e98e8f7af3ead.1513070662.git.lucien.xin@gmail.com> In-Reply-To: <807b1707f79b11d5883d71ea771e98e8f7af3ead.1513070662.git.lucien.xin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Xin Long Cc: network dev , linux-sctp@vger.kernel.org, Neil Horman , davem@davemloft.net 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? 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 >