Re: [PATCH] nbd: do not reclassify userspace sockets
Li Zao <[email protected]>
| Newsgroups | org.kernel.vger.linux-block,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAAwbnpfikLQUap0MLYFpk83BahLaiYZWP50hV1J6YTftkeGT2Q@mail.gmail.com> |
Hello, I'm just following up on this patch to see if you've had a chance to review it, or if there is any additional information I should provide. Thank you for your time and help. Best regards, Haocheng On Mon, Jul 13, 2026 at 8:03 PM <[email protected]> wrote: > > From: Haocheng Yu <[email protected]> > > A Syzkaller-based tool we developed reported a pvqspinlock warning when > one thread passes a TCP socket to NBD_SET_SOCK while another thread > concurrently calls setsockopt() on the same socket: > > pvqspinlock: lock ... has corrupted value 0x0 > sockopt_lock_sock() > do_ip_setsockopt() > __sys_setsockopt() > > NBD gets the socket with sockfd_lookup(), so the socket remains a normal > userspace-visible file descriptor. Under CONFIG_DEBUG_LOCK_ALLOC, > nbd_add_socket() calls nbd_reclassify_socket(), which in turn calls > sock_lock_init_class_and_name(). This function resets sk_lock.owned, > reinitializes sk_lock.wq, and calls spin_lock_init() on sk_lock.slock. > > The sock_allow_reclassification() check only samples the current socket > lock state. It does not prevent a concurrent syscall from acquiring or > using the socket lock immediately after the check. Reinitializing the > socket lock while setsockopt() is using it can therefore clear the > queued spinlock state and make the later unlock trigger the corrupted-lock > warning. > > Only reclassify sockets that are private to the kernel before they are > published. NBD accepts sockets supplied by userspace and cannot make > that guarantee, so remove the NBD-specific reclassification. > > Signed-off-by: Haocheng Yu <[email protected]> > --- > drivers/block/nbd.c | 37 ------------------------------------- > 1 file changed, 37 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 8f10762e90ef..e2fe9e3308fc 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -1238,42 +1238,6 @@ static struct socket *nbd_get_socket(struct nbd_device *nbd, unsigned long fd, > return sock; > } > > -#ifdef CONFIG_DEBUG_LOCK_ALLOC > -static struct lock_class_key nbd_key[3]; > -static struct lock_class_key nbd_slock_key[3]; > - > -static void nbd_reclassify_socket(struct socket *sock) > -{ > - struct sock *sk = sock->sk; > - > - if (!sock_allow_reclassification(sk)) > - return; > - > - switch (sk->sk_family) { > - case AF_INET: > - sock_lock_init_class_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", > - &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", > - &nbd_slock_key[2], > - "sk_lock-AF_UNIX-NBD", > - &nbd_key[2]); > - break; > - } > -} > -#else > -static inline void nbd_reclassify_socket(struct socket *sock) {} > -#endif > - > static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, > bool netlink) > { > @@ -1290,7 +1254,6 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg, > sock = nbd_get_socket(nbd, arg, &err); > if (!sock) > return err; > - nbd_reclassify_socket(sock); > > /* > * We need to make sure we don't get any errant requests while we're > > base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa > -- > 2.51.0 >