lkml.org 
[lkml]   [2021]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 01/37] net/sctp: fix race condition in sctp_destroy_sock
    Date
    From: Or Cohen <orcohen@paloaltonetworks.com>

    commit b166a20b07382b8bc1dcee2a448715c9c2c81b5b upstream.

    If sctp_destroy_sock is called without sock_net(sk)->sctp.addr_wq_lock
    held and sp->do_auto_asconf is true, then an element is removed
    from the auto_asconf_splist without any proper locking.

    This can happen in the following functions:
    1. In sctp_accept, if sctp_sock_migrate fails.
    2. In inet_create or inet6_create, if there is a bpf program
    attached to BPF_CGROUP_INET_SOCK_CREATE which denies
    creation of the sctp socket.

    The bug is fixed by acquiring addr_wq_lock in sctp_destroy_sock
    instead of sctp_close.

    This addresses CVE-2021-23133.

    Reported-by: Or Cohen <orcohen@paloaltonetworks.com>
    Reviewed-by: Xin Long <lucien.xin@gmail.com>
    Fixes: 610236587600 ("bpf: Add new cgroup attach type to enable sock modifications")
    Signed-off-by: Or Cohen <orcohen@paloaltonetworks.com>
    Acked-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>
    ---
    net/sctp/socket.c | 13 +++++--------
    1 file changed, 5 insertions(+), 8 deletions(-)

    --- a/net/sctp/socket.c
    +++ b/net/sctp/socket.c
    @@ -1582,11 +1582,9 @@ 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.
    */
    - spin_lock_bh(&net->sctp.addr_wq_lock);
    - bh_lock_sock_nested(sk);
    + local_bh_disable();
    + bh_lock_sock(sk);

    /* Hold the sock, since sk_common_release() will put sock_put()
    * and we have just a little more cleanup.
    @@ -1595,7 +1593,7 @@ static void sctp_close(struct sock *sk,
    sk_common_release(sk);

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

    sock_put(sk);

    @@ -4262,9 +4260,6 @@ static int sctp_init_sock(struct sock *s
    sk_sockets_allocated_inc(sk);
    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,
    @@ -4299,7 +4294,9 @@ static void sctp_destroy_sock(struct soc

    if (sp->do_auto_asconf) {
    sp->do_auto_asconf = 0;
    + spin_lock_bh(&sock_net(sk)->sctp.addr_wq_lock);
    list_del(&sp->auto_asconf_list);
    + spin_unlock_bh(&sock_net(sk)->sctp.addr_wq_lock);
    }
    sctp_endpoint_free(sp->ep);
    local_bh_disable();

    \
     
     \ /
      Last update: 2021-04-26 09:32    [W:4.097 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site