mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
synced 2025-01-10 15:58:47 +00:00
bpf, sock_map: Move cancel_work_sync() out of sock lock
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: 799aa7f98d53 ("skmsg: Avoid lock_sock() in sk_psock_backlog()") Reported-by: Stanislav Fomichev <sdf@google.com> Signed-off-by: Cong Wang <cong.wang@bytedance.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Tested-by: Jakub Sitnicki <jakub@cloudflare.com> Acked-by: John Fastabend <john.fastabend@gmail.com> Acked-by: Jakub Sitnicki <jakub@cloudflare.com> Link: https://lore.kernel.org/bpf/20221102043417.279409-1-xiyou.wangcong@gmail.com
This commit is contained in:
parent
a778f5d46b
commit
8bbabb3fdd
@ -376,7 +376,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);
|
||||
|
||||
#if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
|
||||
int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock);
|
||||
|
@ -803,16 +803,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_done_strp(struct sk_psock *psock);
|
||||
@ -850,7 +847,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);
|
||||
|
||||
INIT_RCU_WORK(&psock->rwork, sk_psock_destroy);
|
||||
queue_rcu_work(system_wq, &psock->rwork);
|
||||
|
@ -1596,7 +1596,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);
|
||||
}
|
||||
@ -1619,9 +1619,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);
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(sock_map_close);
|
||||
|
Loading…
x
Reference in New Issue
Block a user