| From b166a20b07382b8bc1dcee2a448715c9c2c81b5b Mon Sep 17 00:00:00 2001 |
| From: Or Cohen <orcohen@paloaltonetworks.com> |
| Date: Tue, 13 Apr 2021 21:10:31 +0300 |
| Subject: net/sctp: fix race condition in sctp_destroy_sock |
| |
| 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 |
| @@ -1520,11 +1520,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. |
| @@ -1533,7 +1531,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); |
| |
| @@ -4939,9 +4937,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, |
| @@ -4976,7 +4971,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(); |