lkml.org 
[lkml]   [2015]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.0 09/55] sctp: fix ASCONF list handling
    Date
    4.0-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

    [ Upstream commit 2d45a02d0166caf2627fe91897c6ffc3b19514c4 ]

    ->auto_asconf_splist is per namespace and mangled by functions like
    sctp_setsockopt_auto_asconf() which doesn't guarantee any serialization.

    Also, the call to inet_sk_copy_descendant() was backuping
    ->auto_asconf_list through the copy but was not honoring
    ->do_auto_asconf, which could lead to list corruption if it was
    different between both sockets.

    This commit thus fixes the list handling by using ->addr_wq_lock
    spinlock to protect the list. A special handling is done upon socket
    creation and destruction for that. Error handlig on sctp_init_sock()
    will never return an error after having initialized asconf, so
    sctp_destroy_sock() can be called without addrq_wq_lock. The lock now
    will be take on sctp_close_sock(), before locking the socket, so we
    don't do it in inverse order compared to sctp_addr_wq_timeout_handler().

    Instead of taking the lock on sctp_sock_migrate() for copying and
    restoring the list values, it's preferred to avoid rewritting it by
    implementing sctp_copy_descendant().

    Issue was found with a test application that kept flipping sysctl
    default_auto_asconf on and off, but one could trigger it by issuing
    simultaneous setsockopt() calls on multiple sockets or by
    creating/destroying sockets fast enough. This is only triggerable
    locally.

    Fixes: 9f7d653b67ae ("sctp: Add Auto-ASCONF support (core).")
    Reported-by: Ji Jianwen <jiji@redhat.com>
    Suggested-by: Neil Horman <nhorman@tuxdriver.com>
    Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
    Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
    Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    include/net/netns/sctp.h | 1 +
    include/net/sctp/structs.h | 4 ++++
    net/sctp/socket.c | 43 ++++++++++++++++++++++++++++++++-----------
    3 files changed, 37 insertions(+), 11 deletions(-)

    --- a/include/net/netns/sctp.h
    +++ b/include/net/netns/sctp.h
    @@ -31,6 +31,7 @@ struct netns_sctp {
    struct list_head addr_waitq;
    struct timer_list addr_wq_timer;
    struct list_head auto_asconf_splist;
    + /* Lock that protects both addr_waitq and auto_asconf_splist */
    spinlock_t addr_wq_lock;

    /* Lock that protects the local_addr_list writers */
    --- a/include/net/sctp/structs.h
    +++ b/include/net/sctp/structs.h
    @@ -223,6 +223,10 @@ struct sctp_sock {
    atomic_t pd_mode;
    /* Receive to here while partial delivery is in effect. */
    struct sk_buff_head pd_lobby;
    +
    + /* These must be the last fields, as they will skipped on copies,
    + * like on accept and peeloff operations
    + */
    struct list_head auto_asconf_list;
    int do_auto_asconf;
    };
    --- a/net/sctp/socket.c
    +++ b/net/sctp/socket.c
    @@ -1533,8 +1533,10 @@ static void sctp_close(struct sock *sk,

    /* Supposedly, no process has access to the socket, but
    * the net layers still may.
    + * Also, sctp_destroy_sock() needs to be called with addr_wq_lock
    + * held and that should be grabbed before socket lock.
    */
    - local_bh_disable();
    + spin_lock_bh(&net->sctp.addr_wq_lock);
    bh_lock_sock(sk);

    /* Hold the sock, since sk_common_release() will put sock_put()
    @@ -1544,7 +1546,7 @@ static void sctp_close(struct sock *sk,
    sk_common_release(sk);

    bh_unlock_sock(sk);
    - local_bh_enable();
    + spin_unlock_bh(&net->sctp.addr_wq_lock);

    sock_put(sk);

    @@ -3587,6 +3589,7 @@ static int sctp_setsockopt_auto_asconf(s
    if ((val && sp->do_auto_asconf) || (!val && !sp->do_auto_asconf))
    return 0;

    + spin_lock_bh(&sock_net(sk)->sctp.addr_wq_lock);
    if (val == 0 && sp->do_auto_asconf) {
    list_del(&sp->auto_asconf_list);
    sp->do_auto_asconf = 0;
    @@ -3595,6 +3598,7 @@ static int sctp_setsockopt_auto_asconf(s
    &sock_net(sk)->sctp.auto_asconf_splist);
    sp->do_auto_asconf = 1;
    }
    + spin_unlock_bh(&sock_net(sk)->sctp.addr_wq_lock);
    return 0;
    }

    @@ -4128,18 +4132,28 @@ static int sctp_init_sock(struct sock *s
    local_bh_disable();
    percpu_counter_inc(&sctp_sockets_allocated);
    sock_prot_inuse_add(net, sk->sk_prot, 1);
    +
    + /* Nothing can fail after this block, otherwise
    + * sctp_destroy_sock() will be called without addr_wq_lock held
    + */
    if (net->sctp.default_auto_asconf) {
    + spin_lock(&sock_net(sk)->sctp.addr_wq_lock);
    list_add_tail(&sp->auto_asconf_list,
    &net->sctp.auto_asconf_splist);
    sp->do_auto_asconf = 1;
    - } else
    + spin_unlock(&sock_net(sk)->sctp.addr_wq_lock);
    + } else {
    sp->do_auto_asconf = 0;
    + }
    +
    local_bh_enable();

    return 0;
    }

    -/* Cleanup any SCTP per socket resources. */
    +/* Cleanup any SCTP per socket resources. Must be called with
    + * sock_net(sk)->sctp.addr_wq_lock held if sp->do_auto_asconf is true
    + */
    static void sctp_destroy_sock(struct sock *sk)
    {
    struct sctp_sock *sp;
    @@ -7202,6 +7216,19 @@ void sctp_copy_sock(struct sock *newsk,
    newinet->mc_list = NULL;
    }

    +static inline void sctp_copy_descendant(struct sock *sk_to,
    + const struct sock *sk_from)
    +{
    + int ancestor_size = sizeof(struct inet_sock) +
    + sizeof(struct sctp_sock) -
    + offsetof(struct sctp_sock, auto_asconf_list);
    +
    + if (sk_from->sk_family == PF_INET6)
    + ancestor_size += sizeof(struct ipv6_pinfo);
    +
    + __inet_sk_copy_descendant(sk_to, sk_from, ancestor_size);
    +}
    +
    /* Populate the fields of the newsk from the oldsk and migrate the assoc
    * and its messages to the newsk.
    */
    @@ -7216,7 +7243,6 @@ static void sctp_sock_migrate(struct soc
    struct sk_buff *skb, *tmp;
    struct sctp_ulpevent *event;
    struct sctp_bind_hashbucket *head;
    - struct list_head tmplist;

    /* Migrate socket buffer sizes and all the socket level options to the
    * new socket.
    @@ -7224,12 +7250,7 @@ static void sctp_sock_migrate(struct soc
    newsk->sk_sndbuf = oldsk->sk_sndbuf;
    newsk->sk_rcvbuf = oldsk->sk_rcvbuf;
    /* Brute force copy old sctp opt. */
    - if (oldsp->do_auto_asconf) {
    - memcpy(&tmplist, &newsp->auto_asconf_list, sizeof(tmplist));
    - inet_sk_copy_descendant(newsk, oldsk);
    - memcpy(&newsp->auto_asconf_list, &tmplist, sizeof(tmplist));
    - } else
    - inet_sk_copy_descendant(newsk, oldsk);
    + sctp_copy_descendant(newsk, oldsk);

    /* Restore the ep value that was overwritten with the above structure
    * copy.



    \
     
     \ /
      Last update: 2015-07-08 10:21    [W:4.410 / U:0.400 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site