[PATCH] nbd: do not reinitialize a live socket lock when reclassifying

[Aibing Chen]

d532cddb6c60 ("nbd: Reclassify sockets to avoid lockdep circular
dependency") reclassifies the socket passed via NBD_SET_SOCK (or the
netlink equivalent) with sock_lock_init_class_and_name().  That helper
is meant for freshly created sockets: besides setting the lockdep
class it clears sk_lock.owned, reinitializes the wait queue and
reinitializes sk_lock.slock with spin_lock_init().

The NBD protocol has userspace keep the socket fd open, so the socket
can be in active use while it is being reclassified: an inet_shutdown()
or lock_sock() racing the reclassification can hold sk_lock.slock
across spin_lock_init(), leaving the live lock zeroed.  On
DEBUG_LOCK_ALLOC kernels this surfaces as "BUG: spinlock already
unlocked" (debug_spin_unlock()) from the shutdown/lock_sock path
running in parallel.

Add sock_lock_reclassify_and_name(), a variant that only re-arms the
lockdep class and name of the live sk_lock (plus the module owner
tracking) without touching the lock itself, sk_lock.owned or the wait
queue, and switch nbd_reclassify_socket() to it.  The
sock_allow_reclassification() guard is kept to narrow the window in
which the lock is in use by lock_sock().

Reported-by: syzbot+b29ed72382b0e218323a@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=b29ed72382b0e218323a
Fixes: d532cddb6c60 ("nbd: Reclassify sockets to avoid lockdep circular dependency")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Aibing Chen <chenaibing@xxxxxxxxxxxxxxxxxxxx>
---
 drivers/block/nbd.c |  6 +++---
 include/net/sock.h  | 16 ++++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index ffce519bf..28274a48d 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1251,19 +1251,19 @@ static void nbd_reclassify_socket(struct socket *sock)
 
 	switch (sk->sk_family) {
 	case AF_INET:
-		sock_lock_init_class_and_name(sk, "slock-AF_INET-NBD",
+		sock_lock_reclassify_and_name(sk, "slock-AF_INET-NBD",
 					      &nbd_slock_key[0],
 					      "sk_lock-AF_INET-NBD",
 					      &nbd_key[0]);
 		break;
 	case AF_INET6:
-		sock_lock_init_class_and_name(sk, "slock-AF_INET6-NBD",
+		sock_lock_reclassify_and_name(sk, "slock-AF_INET6-NBD",
 					      &nbd_slock_key[1],
 					      "sk_lock-AF_INET6-NBD",
 					      &nbd_key[1]);
 		break;
 	case AF_UNIX:
-		sock_lock_init_class_and_name(sk, "slock-AF_UNIX-NBD",
+		sock_lock_reclassify_and_name(sk, "slock-AF_UNIX-NBD",
 					      &nbd_slock_key[2],
 					      "sk_lock-AF_UNIX-NBD",
 					      &nbd_key[2]);
diff --git a/include/net/sock.h b/include/net/sock.h
index 3144efad8..2b305777c 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1704,6 +1704,22 @@ do {									\
 	lockdep_init_map(&(sk)->sk_lock.dep_map, (name), (key), 0);	\
 } while (0)
 
+/*
+ * Reclassify the lockdep class of a live socket lock.  Unlike
+ * sock_lock_init_class_and_name() this variant does not touch the
+ * lock itself, sk_lock.owned or the wait queue, so it is safe on a
+ * socket that userspace still holds.  The caller should check
+ * sock_allow_reclassification() first to narrow the window in which
+ * the lock is in use by lock_sock().
+ */
+#define sock_lock_reclassify_and_name(sk, sname, skey, name, key)	\
+do {									\
+	sk_owner_set(sk, THIS_MODULE);					\
+	lockdep_set_class_and_name(&(sk)->sk_lock.slock,		\
+				   (skey), (sname));			\
+	lockdep_init_map(&(sk)->sk_lock.dep_map, (name), (key), 0);	\
+} while (0)
+
 static inline bool lockdep_sock_is_held(const struct sock *sk)
 {
 	return lockdep_is_held(&sk->sk_lock) ||
-- 
2.50.1.windows.1