Commit 34e5b011 authored by Xin Long's avatar Xin Long Committed by David S. Miller

sctp: delay auto_asconf init until binding the first addr

As Or Cohen described:

  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.

This patch is to fix it by moving the auto_asconf init out of
sctp_init_sock(), by which inet_create()/inet6_create() won't
need to operate it in sctp_destroy_sock() when calling
sk_common_release().

It also makes more sense to do auto_asconf init while binding the
first addr, as auto_asconf actually requires an ANY addr bind,
see it in sctp_addr_wq_timeout_handler().

This addresses CVE-2021-23133.

Fixes: 61023658 ("bpf: Add new cgroup attach type to enable sock modifications")
Reported-by: default avatarOr Cohen <orcohen@paloaltonetworks.com>
Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 01bfe5e8
......@@ -357,6 +357,18 @@ static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
return af;
}
static void sctp_auto_asconf_init(struct sctp_sock *sp)
{
struct net *net = sock_net(&sp->inet.sk);
if (net->sctp.default_auto_asconf) {
spin_lock(&net->sctp.addr_wq_lock);
list_add_tail(&sp->auto_asconf_list, &net->sctp.auto_asconf_splist);
spin_unlock(&net->sctp.addr_wq_lock);
sp->do_auto_asconf = 1;
}
}
/* Bind a local address either to an endpoint or to an association. */
static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
{
......@@ -418,8 +430,10 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
return -EADDRINUSE;
/* Refresh ephemeral port. */
if (!bp->port)
if (!bp->port) {
bp->port = inet_sk(sk)->inet_num;
sctp_auto_asconf_init(sp);
}
/* Add the address to the bind address list.
* Use GFP_ATOMIC since BHs will be disabled.
......@@ -4993,19 +5007,6 @@ static int sctp_init_sock(struct sock *sk)
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,
&net->sctp.auto_asconf_splist);
sp->do_auto_asconf = 1;
spin_unlock(&sock_net(sk)->sctp.addr_wq_lock);
} else {
sp->do_auto_asconf = 0;
}
local_bh_enable();
return 0;
......@@ -9401,6 +9402,8 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
return err;
}
sctp_auto_asconf_init(newsp);
/* Move any messages in the old socket's receive queue that are for the
* peeled off association to the new socket's receive queue.
*/
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment