Commit a4aa9897 authored by Cong Wang's avatar Cong Wang Committed by Liu Jian
Browse files

bpf, sock_map: Move cancel_work_sync() out of sock lock 

mainline inclusion
from mainline-v6.1-rc5
commit 8bbabb3f
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/I65HYE
CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8bbabb3fddcd0f858be69ed5abc9b470a239d6f2



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

Stanislav reported a lockdep warning, which is caused by the
cancel_work_sync() called inside sock_map_close(), as analyzed
below by Jakub:

psock->work.func = sk_psock_backlog()
  ACQUIRE psock->work_mutex
    sk_psock_handle_skb()
      skb_send_sock()
        __skb_send_sock()
          sendpage_unlocked()
            kernel_sendpage()
              sock->ops->sendpage = inet_sendpage()
                sk->sk_prot->sendpage = tcp_sendpage()
                  ACQUIRE sk->sk_lock
                    tcp_sendpage_locked()
                  RELEASE sk->sk_lock
  RELEASE psock->work_mutex

sock_map_close()
  ACQUIRE sk->sk_lock
  sk_psock_stop()
    sk_psock_clear_state(psock, SK_PSOCK_TX_ENABLED)
    cancel_work_sync()
      __cancel_work_timer()
        __flush_work()
          // wait for psock->work to finish
  RELEASE sk->sk_lock

We can move the cancel_work_sync() out of the sock lock protection,
but still before saved_close() was called.

Fixes: 799aa7f9 ("skmsg: Avoid lock_sock() in sk_psock_backlog()")
Reported-by: default avatarStanislav Fomichev <sdf@google.com>
Signed-off-by: default avatarCong Wang <cong.wang@bytedance.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Tested-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Acked-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
Link: https://lore.kernel.org/bpf/20221102043417.279409-1-xiyou.wangcong@gmail.com


(cherry picked from commit 8bbabb3f)
Signed-off-by: default avatarLiu Jian <liujian56@huawei.com>

Conflicts:
	net/core/skmsg.c
parent 86404036

include/linux/skmsg.h

+1 −1
Original line number Diff line number Diff line
@@ -393,7 +393,7 @@ static inline void sk_psock_report_error(struct sk_psock *psock, int err) 
}



struct sk_psock *sk_psock_init(struct sock *sk, int node);

void sk_psock_stop(struct sk_psock *psock, bool wait);

void sk_psock_stop(struct sk_psock *psock);



int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock);

void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock);

net/core/skmsg.c

+2 −5
Original line number Diff line number Diff line
@@ -695,16 +695,13 @@ static void sk_psock_link_destroy(struct sk_psock *psock) 
	}

}



void sk_psock_stop(struct sk_psock *psock, bool wait)

void sk_psock_stop(struct sk_psock *psock)

{

	spin_lock_bh(&psock->ingress_lock);

	sk_psock_clear_state(psock, SK_PSOCK_TX_ENABLED);

	sk_psock_cork_free(psock);

	__sk_psock_zap_ingress(psock);

	spin_unlock_bh(&psock->ingress_lock);



	if (wait)

		cancel_work_sync(&psock->work);

}



static void sk_psock_destroy_deferred(struct work_struct *gc)
@@ -750,7 +747,7 @@ void sk_psock_drop(struct sock *sk, struct sk_psock *psock) 
		sk_psock_stop_verdict(sk, psock);

	write_unlock_bh(&sk->sk_callback_lock);



	sk_psock_stop(psock, false);

	sk_psock_stop(psock);

	call_rcu(&psock->rcu, sk_psock_destroy);

}

EXPORT_SYMBOL_GPL(sk_psock_drop);

net/core/sock_map.c

+4 −3
Original line number Diff line number Diff line
@@ -1651,7 +1651,7 @@ void sock_map_destroy(struct sock *sk) 
	saved_destroy = psock->saved_destroy;

	sock_map_remove_links(sk, psock);

	rcu_read_unlock();

	sk_psock_stop(psock, false);

	sk_psock_stop(psock);

	sk_psock_put(sk, psock);

	saved_destroy(sk);

}
@@ -1674,9 +1674,10 @@ void sock_map_close(struct sock *sk, long timeout) 
	saved_close = psock->saved_close;

	sock_map_remove_links(sk, psock);

	rcu_read_unlock();

	sk_psock_stop(psock, true);

	sk_psock_put(sk, psock);

	sk_psock_stop(psock);

	release_sock(sk);

	cancel_work_sync(&psock->work);

	sk_psock_put(sk, psock);

	saved_close(sk, timeout);

}