All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Borkmann <dborkman@redhat.com>
To: vyasevic@redhat.com
Cc: David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
	tgraf@suug.ch, nhorman@tuxdriver.com
Subject: Re: [PATCH net v2] net: sctp: wake up all assocs if sndbuf policy is per socket
Date: Tue, 08 Apr 2014 20:50:31 +0000	[thread overview]
Message-ID: <53446117.7010701@redhat.com> (raw)
In-Reply-To: <53445005.9000707@redhat.com>

On 04/08/2014 09:37 PM, Vlad Yasevich wrote:
> On 04/08/2014 03:04 PM, Daniel Borkmann wrote:
>> On 04/08/2014 08:46 PM, David Miller wrote:
>>>
>>> Daniel and Vlad, I'm about to send Linus a pull request.
>>>
>>> I know that you still need to fixup this SCTP change and it'll be
>>> in there, but I really need to get the changes in my tree staged
>>> so that I can do a set of -stable submissions.
>>>
>>> So please don't freak out, I know that this change still needs work
>>> and shouldn't go to -stable just yet :-)
>>
>> Noted, thanks. I think the issue is that in sctp_association_free()
>> we do a list_del(&asoc->asocs) and then flush sctp_outq_free() which
>> will then access on sctp_wfree() a poisoned entry. I think this
>> should be list_del_init() instead.
>
> Switching to list_del_init() will solve the crash, but will not address
> the issue.  You've just removed an association and need to notify others
> of available space.  You can't do that since you've been unlinked.
>
> We either need a rcu_style unlink, or detect the delete case and loop
> from the beginning.
>
> You can do #2 easily enough by looking at asoc->base.dead to decide
> where to start looping.

Agreed, I think #2 is better, so we can simply call and return with
sctp_write_space() if we see that the assoc is dead; I think SCTP is
doing too much deferring to RCU anyway. ;)

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Borkmann <dborkman@redhat.com>
To: vyasevic@redhat.com
Cc: David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
	tgraf@suug.ch, nhorman@tuxdriver.com
Subject: Re: [PATCH net v2] net: sctp: wake up all assocs if sndbuf policy is per socket
Date: Tue, 08 Apr 2014 22:50:31 +0200	[thread overview]
Message-ID: <53446117.7010701@redhat.com> (raw)
In-Reply-To: <53445005.9000707@redhat.com>

On 04/08/2014 09:37 PM, Vlad Yasevich wrote:
> On 04/08/2014 03:04 PM, Daniel Borkmann wrote:
>> On 04/08/2014 08:46 PM, David Miller wrote:
>>>
>>> Daniel and Vlad, I'm about to send Linus a pull request.
>>>
>>> I know that you still need to fixup this SCTP change and it'll be
>>> in there, but I really need to get the changes in my tree staged
>>> so that I can do a set of -stable submissions.
>>>
>>> So please don't freak out, I know that this change still needs work
>>> and shouldn't go to -stable just yet :-)
>>
>> Noted, thanks. I think the issue is that in sctp_association_free()
>> we do a list_del(&asoc->asocs) and then flush sctp_outq_free() which
>> will then access on sctp_wfree() a poisoned entry. I think this
>> should be list_del_init() instead.
>
> Switching to list_del_init() will solve the crash, but will not address
> the issue.  You've just removed an association and need to notify others
> of available space.  You can't do that since you've been unlinked.
>
> We either need a rcu_style unlink, or detect the delete case and loop
> from the beginning.
>
> You can do #2 easily enough by looking at asoc->base.dead to decide
> where to start looping.

Agreed, I think #2 is better, so we can simply call and return with
sctp_write_space() if we see that the assoc is dead; I think SCTP is
doing too much deferring to RCU anyway. ;)

  reply	other threads:[~2014-04-08 20:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-08 15:26 [PATCH net v2] net: sctp: wake up all assocs if sndbuf policy is per socket Daniel Borkmann
2014-04-08 15:26 ` Daniel Borkmann
2014-04-08 15:28 ` Vlad Yasevich
2014-04-08 15:28   ` Vlad Yasevich
2014-04-08 15:37 ` Neil Horman
2014-04-08 15:37   ` Neil Horman
2014-04-08 16:57 ` Daniel Borkmann
2014-04-08 16:57   ` Daniel Borkmann
2014-04-08 17:18   ` David Miller
2014-04-08 17:18     ` David Miller
2014-04-08 17:20     ` Daniel Borkmann
2014-04-08 17:20       ` Daniel Borkmann
2014-04-08 18:19       ` David Miller
2014-04-08 18:19         ` David Miller
2014-04-08 18:46         ` David Miller
2014-04-08 18:46           ` David Miller
2014-04-08 19:04           ` Daniel Borkmann
2014-04-08 19:04             ` Daniel Borkmann
2014-04-08 19:37             ` Vlad Yasevich
2014-04-08 19:37               ` Vlad Yasevich
2014-04-08 20:50               ` Daniel Borkmann [this message]
2014-04-08 20:50                 ` Daniel Borkmann
2014-04-08 17:08 ` David Miller
2014-04-08 17:08   ` David Miller

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=53446117.7010701@redhat.com \
    --to=dborkman@redhat.com \
    --cc=davem@davemloft.net \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=tgraf@suug.ch \
    --cc=vyasevic@redhat.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.