Skip to content

Commit 3c1f070

Browse files
dcuigregkh
authored andcommitted
vsock: Fix a lockdep warning in __vsock_release()
[ Upstream commit 0d9138f ] Lockdep is unhappy if two locks from the same class are held. Fix the below warning for hyperv and virtio sockets (vmci socket code doesn't have the issue) by using lock_sock_nested() when __vsock_release() is called recursively: ============================================ WARNING: possible recursive locking detected 5.3.0+ #1 Not tainted -------------------------------------------- server/1795 is trying to acquire lock: ffff8880c5158990 (sk_lock-AF_VSOCK){+.+.}, at: hvs_release+0x10/0x120 [hv_sock] but task is already holding lock: ffff8880c5158150 (sk_lock-AF_VSOCK){+.+.}, at: __vsock_release+0x2e/0xf0 [vsock] other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(sk_lock-AF_VSOCK); lock(sk_lock-AF_VSOCK); *** DEADLOCK *** May be due to missing lock nesting notation 2 locks held by server/1795: #0: ffff8880c5d05ff8 (&sb->s_type->i_mutex_key#10){+.+.}, at: __sock_release+0x2d/0xa0 #1: ffff8880c5158150 (sk_lock-AF_VSOCK){+.+.}, at: __vsock_release+0x2e/0xf0 [vsock] stack backtrace: CPU: 5 PID: 1795 Comm: server Not tainted 5.3.0+ #1 Call Trace: dump_stack+0x67/0x90 __lock_acquire.cold.67+0xd2/0x20b lock_acquire+0xb5/0x1c0 lock_sock_nested+0x6d/0x90 hvs_release+0x10/0x120 [hv_sock] __vsock_release+0x24/0xf0 [vsock] __vsock_release+0xa0/0xf0 [vsock] vsock_release+0x12/0x30 [vsock] __sock_release+0x37/0xa0 sock_close+0x14/0x20 __fput+0xc1/0x250 task_work_run+0x98/0xc0 do_exit+0x344/0xc60 do_group_exit+0x47/0xb0 get_signal+0x15c/0xc50 do_signal+0x30/0x720 exit_to_usermode_loop+0x50/0xa0 do_syscall_64+0x24e/0x270 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x7f4184e85f31 Tested-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Dexuan Cui <decui@microsoft.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 544aee5 commit 3c1f070

3 files changed

Lines changed: 14 additions & 6 deletions

File tree

net/vmw_vsock/af_vsock.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ struct sock *__vsock_create(struct net *net,
641641
}
642642
EXPORT_SYMBOL_GPL(__vsock_create);
643643

644-
static void __vsock_release(struct sock *sk)
644+
static void __vsock_release(struct sock *sk, int level)
645645
{
646646
if (sk) {
647647
struct sk_buff *skb;
@@ -651,9 +651,17 @@ static void __vsock_release(struct sock *sk)
651651
vsk = vsock_sk(sk);
652652
pending = NULL; /* Compiler warning. */
653653

654+
/* The release call is supposed to use lock_sock_nested()
655+
* rather than lock_sock(), if a sock lock should be acquired.
656+
*/
654657
transport->release(vsk);
655658

656-
lock_sock(sk);
659+
/* When "level" is SINGLE_DEPTH_NESTING, use the nested
660+
* version to avoid the warning "possible recursive locking
661+
* detected". When "level" is 0, lock_sock_nested(sk, level)
662+
* is the same as lock_sock(sk).
663+
*/
664+
lock_sock_nested(sk, level);
657665
sock_orphan(sk);
658666
sk->sk_shutdown = SHUTDOWN_MASK;
659667

@@ -662,7 +670,7 @@ static void __vsock_release(struct sock *sk)
662670

663671
/* Clean up any sockets that never were accepted. */
664672
while ((pending = vsock_dequeue_accept(sk)) != NULL) {
665-
__vsock_release(pending);
673+
__vsock_release(pending, SINGLE_DEPTH_NESTING);
666674
sock_put(pending);
667675
}
668676

@@ -711,7 +719,7 @@ EXPORT_SYMBOL_GPL(vsock_stream_has_space);
711719

712720
static int vsock_release(struct socket *sock)
713721
{
714-
__vsock_release(sock->sk);
722+
__vsock_release(sock->sk, 0);
715723
sock->sk = NULL;
716724
sock->state = SS_FREE;
717725

net/vmw_vsock/hyperv_transport.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ static void hvs_release(struct vsock_sock *vsk)
538538
struct sock *sk = sk_vsock(vsk);
539539
bool remove_sock;
540540

541-
lock_sock(sk);
541+
lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
542542
remove_sock = hvs_close_lock_held(vsk);
543543
release_sock(sk);
544544
if (remove_sock)

net/vmw_vsock/virtio_transport_common.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,7 @@ void virtio_transport_release(struct vsock_sock *vsk)
791791
struct sock *sk = &vsk->sk;
792792
bool remove_sock = true;
793793

794-
lock_sock(sk);
794+
lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
795795
if (sk->sk_type == SOCK_STREAM)
796796
remove_sock = virtio_transport_close(vsk);
797797

0 commit comments

Comments
 (0)