netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
To: mleitner@redhat.com
Cc: Neil Horman <nhorman@tuxdriver.com>,
	netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
	Daniel Borkmann <daniel@iogearbox.net>,
	Vlad Yasevich <vyasevich@gmail.com>,
	Michio Honda <micchie@sfc.wide.ad.jp>
Subject: Re: [PATCH v3 2/2] sctp: fix ASCONF list handling
Date: Mon, 08 Jun 2015 22:09:50 +0200	[thread overview]
Message-ID: <1433794190.4616.8.camel@stressinduktion.org> (raw)
In-Reply-To: <b319ab74178e7f186081db90cc24c229127e42ad.1433524062.git.marcelo.leitner@gmail.com>

On Fr, 2015-06-05 at 14:08 -0300, mleitner@redhat.com wrote:
>       if (sp->do_auto_asconf) {
> +             spin_lock_bh(&sock_net(sk)->sctp.addr_wq_lock);
>               sp->do_auto_asconf = 0;
> -             list_del(&sp->auto_asconf_list);
> +             list_del_rcu(&sp->auto_asconf_list);
> +             spin_unlock_bh(&sock_net(sk)->sctp.addr_wq_lock);
>       }

This also looks a bit unsafe to me:

My proposal would be to sock_hold/sock_put the sockets when pushing them
onto the auto_asconf_list and defer the modifications on the list until
we don't need to hold socket lock anymore (in syscalls we do have a reference
anyway).

addr_wq_lock then is only used either without lock_sock at all or only
in order addr_wq_lock -> lock_sock, which does not cause any locking
ordering issues.

Bye,
Hannes

  reply	other threads:[~2015-06-08 20:09 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-28  0:52 [PATCH] sctp: fix ASCONF list handling mleitner
2015-05-28 10:15 ` Neil Horman
2015-05-28 11:17   ` Marcelo Ricardo Leitner
2015-05-28 13:27     ` Marcelo Ricardo Leitner
2015-05-28 14:31       ` Neil Horman
2015-05-28 14:46       ` Marcelo Ricardo Leitner
2015-05-29 13:17         ` Neil Horman
2015-05-29 16:50           ` Marcelo Ricardo Leitner
2015-06-01 14:00             ` Neil Horman
2015-06-01 22:28               ` Marcelo Ricardo Leitner
2015-06-02 13:48                 ` Neil Horman
2015-06-03 16:54                   ` [PATCH v2 1/2] sctp: rcu-ify addr_waitq mleitner
2015-06-03 17:19                     ` Marcelo Ricardo Leitner
2015-06-04 14:27                     ` Neil Horman
2015-06-05 14:18                       ` Marcelo Ricardo Leitner
2015-06-05 17:08                       ` [PATCH v3 " mleitner
2015-06-08 13:58                         ` Neil Horman
2015-06-08 14:46                         ` Hannes Frederic Sowa
2015-06-08 14:59                           ` Hannes Frederic Sowa
2015-06-08 15:19                             ` Neil Horman
2015-06-08 15:37                               ` Hannes Frederic Sowa
2015-06-09 11:36                                 ` Neil Horman
2015-06-09 19:32                                   ` Marcelo Ricardo Leitner
2015-06-10 13:31                                     ` Marcelo Ricardo Leitner
2015-06-10 19:14                                       ` Neil Horman
2015-06-11 14:30                                         ` [PATCH v4] sctp: fix ASCONF list handling mleitner
2015-06-11 23:31                                           ` David Miller
2015-06-12 13:16                                             ` [PATCH v5] " marcelo.leitner
2015-06-14 19:56                                               ` David Miller
2015-06-05 17:08                       ` [PATCH v3 2/2] " mleitner
2015-06-08 20:09                         ` Hannes Frederic Sowa [this message]
2015-06-09 15:37                           ` Marcelo Ricardo Leitner
2015-06-09 17:04                             ` Neil Horman
2015-06-03 16:54                   ` [PATCH v2 " mleitner

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=1433794190.4616.8.camel@stressinduktion.org \
    --to=hannes@stressinduktion.org \
    --cc=daniel@iogearbox.net \
    --cc=linux-sctp@vger.kernel.org \
    --cc=micchie@sfc.wide.ad.jp \
    --cc=mleitner@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=vyasevich@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).