[PATCH] nbd: don't take cpu_hotplug_lock under tx_lock on reconnect

[PATCH] nbd: don't take cpu_hotplug_lock under tx_lock on reconnect

[Nguyen Ngoc Thang]

nbd_genl_reconnect_sock_cb() calls sk_set_memalloc() on the new socket
while holding nsock->tx_lock. sk_set_memalloc() bumps the
memalloc_socks_key static key, which takes cpu_hotplug_lock.

tx_lock is taken in the I/O path under cmd->lock and set->srcu, which
lockdep already orders after fs_reclaim through the queue freeze and
elevator locks. Once cpu_hotplug_lock hangs off tx_lock, any path that
allocates memory while holding a lock taken under cpu_hotplug_lock
closes a cycle:

  WARNING: possible circular locking dependency detected
  Chain exists of:
    fs_reclaim --> cpu_hotplug_lock --> uevent_sock_mutex

  -> #6 (cpu_hotplug_lock){++++}-{0:0}:
         cpus_read_lock+0x54/0x1c8 kernel/cpu.c:490
         static_key_slow_inc+0x1c/0x40 kernel/jump_label.c:190
         sk_set_memalloc+0x6c/0xa0 net/core/sock.c:307
         nbd_genl_reconnect_sock_cb+0x32c/0x72c drivers/block/nbd.c:1377
  -> #5 (&nsock->tx_lock){+.+.}-{4:4}:
         nbd_handle_cmd drivers/block/nbd.c:1143 [inline]
         nbd_queue_rq+0x1f4/0xbac drivers/block/nbd.c:1207

The new socket isn't visible to anyone until it is installed in nsock,
so it doesn't need tx_lock. Mark it memalloc before looking for a dead
connection, as nbd_start_device() does, and undo it if there is no
dead connection to replace.

Reported-by: syzbot+a246ab421a728c78b4ad@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a246ab421a728c78b4ad
Fixes: b7aa3d39385d ("nbd: add a reconfigure netlink command")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Tested in x86_64 QEMU (KASAN + PROVE_LOCKING) with the syzbot C
reproducer. Before: the circular dependency with
&nsock->tx_lock -> cpu_hotplug_lock is reported on every run. After:
that chain is gone. Also exercised the no-dead-connection path
(sk_clear_memalloc()) with a second NBD_CMD_RECONFIGURE.

With this applied, the reproducer then hits an unrelated lockdep false
positive (reconnected sockets are not reclassified); I'm sending a
separate fix for that.

 drivers/block/nbd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 218b99f9e1ef..a567c0bce90e 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1364,6 +1364,9 @@ static int nbd_genl_reconnect_sock_cb(struct nbd_device *nbd, unsigned long arg)
 		return -ENOMEM;
 	}
 
+	/* Takes cpu_hotplug_lock, so it must not nest inside tx_lock. */
+	sk_set_memalloc(sock->sk);
+
 	for (i = 0; i < config->num_connections; i++) {
 		struct nbd_sock *nsock = config->socks[i];
 
@@ -1375,7 +1378,6 @@ static int nbd_genl_reconnect_sock_cb(struct nbd_device *nbd, unsigned long arg)
 			mutex_unlock(&nsock->tx_lock);
 			continue;
 		}
-		sk_set_memalloc(sock->sk);
 		if (nbd->tag_set.timeout)
 			sock->sk->sk_sndtimeo = nbd->tag_set.timeout;
 		atomic_inc(&config->recv_threads);
@@ -1404,6 +1406,7 @@ static int nbd_genl_reconnect_sock_cb(struct nbd_device *nbd, unsigned long arg)
 		dev_info(nbd_to_dev(nbd), "reconnected socket\n");
 		return 0;
 	}
+	sk_clear_memalloc(sock->sk);
 	sockfd_put(sock);
 	kfree(args);
 	return 1;
-- 
2.43.0