All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: network dev <netdev@vger.kernel.org>,
	linux-sctp@vger.kernel.org, davem@davemloft.net,
	Neil Horman <nhorman@tuxdriver.com>
Subject: Re: [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out
Date: Tue, 29 Jan 2019 12:05:32 +0000	[thread overview]
Message-ID: <20190129120531.GC10665@localhost.localdomain> (raw)
In-Reply-To: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.git.lucien.xin@gmail.com>

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 <yinxu@redhat.com>
> Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

We are sort of mixing things up here. We have a bug on SCTP stack that
triggers panics. As good practices recommends, the code should be as
generic as possible and the SCTP-only was dropped in favor of a more
generic one, fixing rhashtables instead. Okay. But then we discovered
rhashtables are going away and we are now waiting on a restructing
to fix the panic. That's not good, especially because it cannot and
should not be backported into -stable trees.

That said, we should not wait for the restructuring to _implicitly_
fix the bug. We should pursuit both fixes here:
- Apply this patch, to fix SCTP stack and allow it to be easily
  backportable.
- Apply the generic fix, which is the restructuring, whenever it
  actually lands.

Thoughts?

Thanks,
Marcelo

WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: network dev <netdev@vger.kernel.org>,
	linux-sctp@vger.kernel.org, davem@davemloft.net,
	Neil Horman <nhorman@tuxdriver.com>
Subject: Re: [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out
Date: Tue, 29 Jan 2019 10:05:32 -0200	[thread overview]
Message-ID: <20190129120531.GC10665@localhost.localdomain> (raw)
In-Reply-To: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.git.lucien.xin@gmail.com>

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 <yinxu@redhat.com>
> Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

We are sort of mixing things up here. We have a bug on SCTP stack that
triggers panics. As good practices recommends, the code should be as
generic as possible and the SCTP-only was dropped in favor of a more
generic one, fixing rhashtables instead. Okay. But then we discovered
rhashtables are going away and we are now waiting on a restructing
to fix the panic. That's not good, especially because it cannot and
should not be backported into -stable trees.

That said, we should not wait for the restructuring to _implicitly_
fix the bug. We should pursuit both fixes here:
- Apply this patch, to fix SCTP stack and allow it to be easily
  backportable.
- Apply the generic fix, which is the restructuring, whenever it
  actually lands.

Thoughts?

Thanks,
Marcelo

  parent reply	other threads:[~2019-01-29 12:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-29  6:42 [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out Xin Long
2018-11-29  6:42 ` Xin Long
2018-11-29 12:49 ` Neil Horman
2018-11-29 12:49   ` Neil Horman
2018-11-29 14:31   ` Xin Long
2018-11-29 14:31     ` Xin Long
2018-11-29 14:38 ` Neil Horman
2018-11-29 14:38   ` Neil Horman
2018-11-30  6:22   ` Xin Long
2018-11-30  6:22     ` Xin Long
2018-11-30 12:20     ` Neil Horman
2018-11-30 12:20       ` Neil Horman
2018-11-30 13:48       ` Xin Long
2018-11-30 13:48         ` Xin Long
2018-11-30 15:22         ` Neil Horman
2018-11-30 15:22           ` Neil Horman
2018-11-30 18:53           ` Xin Long
2018-11-30 18:53             ` Xin Long
2018-11-30 19:35             ` Neil Horman
2018-11-30 19:35               ` Neil Horman
2018-11-30 19:40             ` Neil Horman
2018-11-30 19:40               ` Neil Horman
2019-01-29 12:05 ` Marcelo Ricardo Leitner [this message]
2019-01-29 12:05   ` Marcelo Ricardo Leitner
2019-01-29 18:58   ` Tuxdriver
2019-01-29 18:58     ` Tuxdriver
2019-02-01  0:39     ` Marcelo Ricardo Leitner
2019-02-01  0:39       ` Marcelo Ricardo Leitner
2019-02-01 12:31       ` Neil Horman
2019-02-01 12:31         ` Neil Horman
2019-02-01 18:38         ` David Miller
2019-02-01 18:38           ` David Miller
2019-02-03 19:28           ` Xin Long
2019-02-03 19:28             ` Xin Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190129120531.GC10665@localhost.localdomain \
    --to=marcelo.leitner@gmail.com \
    --cc=davem@davemloft.net \
    --cc=linux-sctp@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.