| From 2c2ca2b6a3b5cba3a1b86688315a02bc81c53a0d Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Thu, 12 Aug 2021 13:30:56 +0800 |
| Subject: vsock/virtio: avoid potential deadlock when vsock device remove |
| |
| From: Longpeng(Mike) <longpeng2@huawei.com> |
| |
| [ Upstream commit 49b0b6ffe20c5344f4173f3436298782a08da4f2 ] |
| |
| There's a potential deadlock case when remove the vsock device or |
| process the RESET event: |
| |
| vsock_for_each_connected_socket: |
| spin_lock_bh(&vsock_table_lock) ----------- (1) |
| ... |
| virtio_vsock_reset_sock: |
| lock_sock(sk) --------------------- (2) |
| ... |
| spin_unlock_bh(&vsock_table_lock) |
| |
| lock_sock() may do initiative schedule when the 'sk' is owned by |
| other thread at the same time, we would receivce a warning message |
| that "scheduling while atomic". |
| |
| Even worse, if the next task (selected by the scheduler) try to |
| release a 'sk', it need to request vsock_table_lock and the deadlock |
| occur, cause the system into softlockup state. |
| Call trace: |
| queued_spin_lock_slowpath |
| vsock_remove_bound |
| vsock_remove_sock |
| virtio_transport_release |
| __vsock_release |
| vsock_release |
| __sock_release |
| sock_close |
| __fput |
| ____fput |
| |
| So we should not require sk_lock in this case, just like the behavior |
| in vhost_vsock or vmci. |
| |
| Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko") |
| Cc: Stefan Hajnoczi <stefanha@redhat.com> |
| Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com> |
| Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> |
| Link: https://lore.kernel.org/r/20210812053056.1699-1-longpeng2@huawei.com |
| Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/vmw_vsock/virtio_transport.c | 7 +++++-- |
| 1 file changed, 5 insertions(+), 2 deletions(-) |
| |
| diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c |
| index 2700a63ab095..3a056f8affd1 100644 |
| --- a/net/vmw_vsock/virtio_transport.c |
| +++ b/net/vmw_vsock/virtio_transport.c |
| @@ -356,11 +356,14 @@ static void virtio_vsock_event_fill(struct virtio_vsock *vsock) |
| |
| static void virtio_vsock_reset_sock(struct sock *sk) |
| { |
| - lock_sock(sk); |
| + /* vmci_transport.c doesn't take sk_lock here either. At least we're |
| + * under vsock_table_lock so the sock cannot disappear while we're |
| + * executing. |
| + */ |
| + |
| sk->sk_state = TCP_CLOSE; |
| sk->sk_err = ECONNRESET; |
| sk->sk_error_report(sk); |
| - release_sock(sk); |
| } |
| |
| static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock) |
| -- |
| 2.30.2 |
| |