From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Subject: Re: [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out Date: Thu, 29 Nov 2018 07:49:21 -0500 Message-ID: <20181129124921.GA14550@hmswarspite.think-freely.org> References: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.git.lucien.xin@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: network dev , linux-sctp@vger.kernel.org, davem@davemloft.net, Marcelo Ricardo Leitner To: Xin Long Return-path: Received: from charlotte.tuxdriver.com ([70.61.120.58]:34139 "EHLO smtp.tuxdriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728128AbeK2XzU (ORCPT ); Thu, 29 Nov 2018 18:55:20 -0500 Content-Disposition: inline In-Reply-To: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.git.lucien.xin@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Nov 29, 2018 at 02:42:56PM +0800, Xin Long wrote: > Now when using stream reconfig to add out streams, stream->out > will get re-allocated, and all old streams' information will > be copied to the new ones and the old ones will be freed. > > So without stream->out_curr updated, next time when trying to > send from stream->out_curr stream, a panic would be caused. > > This patch is to check and update stream->out_curr when > allocating stream_out. > > v1->v2: > - define fa_index() to get elem index from stream->out_curr. > > Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations") > Reported-by: Ying Xu > Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com > Signed-off-by: Xin Long > --- > net/sctp/stream.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/net/sctp/stream.c b/net/sctp/stream.c > index 3892e76..30e7809 100644 > --- a/net/sctp/stream.c > +++ b/net/sctp/stream.c > @@ -84,6 +84,19 @@ static void fa_zero(struct flex_array *fa, size_t index, size_t count) > } > } > > +static size_t fa_index(struct flex_array *fa, void *elem, size_t count) > +{ > + size_t index = 0; > + > + while (count--) { > + if (elem == flex_array_get(fa, index)) > + break; > + index++; > + } > + > + return index; > +} > + > /* Migrates chunks from stream queues to new stream queues if needed, > * but not across associations. Also, removes those chunks to streams > * higher than the new max. > @@ -147,6 +160,13 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt, > > if (stream->out) { > fa_copy(out, stream->out, 0, min(outcnt, stream->outcnt)); > + if (stream->out_curr) { > + size_t index = fa_index(stream->out, stream->out_curr, > + stream->outcnt); > + > + BUG_ON(index == stream->outcnt); > + stream->out_curr = flex_array_get(out, index); > + } > fa_free(stream->out); > } > > -- > 2.1.0 > > I'm having a hard time understanding why, as I noted earlier, you don't just write a function in the flex_array code that can resize the number of elements in your array. If you do that, you can avoid both all the copying, and the need to lookup the in-use pointer again Neil From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Date: Thu, 29 Nov 2018 12:49:21 +0000 Subject: Re: [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out Message-Id: <20181129124921.GA14550@hmswarspite.think-freely.org> List-Id: References: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.git.lucien.xin@gmail.com> In-Reply-To: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.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, davem@davemloft.net, Marcelo Ricardo Leitner On Thu, Nov 29, 2018 at 02:42:56PM +0800, Xin Long wrote: > Now when using stream reconfig to add out streams, stream->out > will get re-allocated, and all old streams' information will > be copied to the new ones and the old ones will be freed. > > So without stream->out_curr updated, next time when trying to > send from stream->out_curr stream, a panic would be caused. > > This patch is to check and update stream->out_curr when > allocating stream_out. > > v1->v2: > - define fa_index() to get elem index from stream->out_curr. > > Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations") > Reported-by: Ying Xu > Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com > Signed-off-by: Xin Long > --- > net/sctp/stream.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/net/sctp/stream.c b/net/sctp/stream.c > index 3892e76..30e7809 100644 > --- a/net/sctp/stream.c > +++ b/net/sctp/stream.c > @@ -84,6 +84,19 @@ static void fa_zero(struct flex_array *fa, size_t index, size_t count) > } > } > > +static size_t fa_index(struct flex_array *fa, void *elem, size_t count) > +{ > + size_t index = 0; > + > + while (count--) { > + if (elem = flex_array_get(fa, index)) > + break; > + index++; > + } > + > + return index; > +} > + > /* Migrates chunks from stream queues to new stream queues if needed, > * but not across associations. Also, removes those chunks to streams > * higher than the new max. > @@ -147,6 +160,13 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt, > > if (stream->out) { > fa_copy(out, stream->out, 0, min(outcnt, stream->outcnt)); > + if (stream->out_curr) { > + size_t index = fa_index(stream->out, stream->out_curr, > + stream->outcnt); > + > + BUG_ON(index = stream->outcnt); > + stream->out_curr = flex_array_get(out, index); > + } > fa_free(stream->out); > } > > -- > 2.1.0 > > I'm having a hard time understanding why, as I noted earlier, you don't just write a function in the flex_array code that can resize the number of elements in your array. If you do that, you can avoid both all the copying, and the need to lookup the in-use pointer again Neil