nbd: restrict sockets to TCP and UDP

[PATCH] nbd: restrict sockets to TCP and UDP

Posted by Eric Dumazet 3 weeks, 2 days ago

Recently, syzbot started to abuse NBD with all kinds of sockets.

Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
made sure the socket supported a shutdown() method.

Explicitely accept TCP and UNIX stream sockets.

Fixes: cf1b2326b734 ("nbd: verify socket is supported during setup")
Reported-by: syzbot+e1cd6bd8493060bd701d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/CANn89iJ+76eE3A_8S_zTpSyW5hvPRn6V57458hCZGY5hbH_bFA@mail.gmail.com/T/#m081036e8747cd7e2626c1da5d78c8b9d1e55b154
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Yu Kuai <yukuai1@huaweicloud.com>
Cc: linux-block@vger.kernel.org
Cc: nbd@other.debian.org
---
 drivers/block/nbd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 6463d0e8d0cef71e73e67fecd16de4dec1c75da7..87b0b78249da3325023949585f4daf40486c9692 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1217,6 +1217,14 @@ static struct socket *nbd_get_socket(struct nbd_device *nbd, unsigned long fd,
 	if (!sock)
 		return NULL;
 
+	if (!sk_is_tcp(sock->sk) &&
+	    !sk_is_stream_unix(sock->sk)) {
+		dev_err(disk_to_dev(nbd->disk), "Unsupported socket: should be TCP or UNIX.\n");
+		*err = -EINVAL;
+		sockfd_put(sock);
+		return NULL;
+	}
+
 	if (sock->ops->shutdown == sock_no_shutdown) {
 		dev_err(disk_to_dev(nbd->disk), "Unsupported socket: shutdown callout must be supported.\n");
 		*err = -EINVAL;
-- 
2.51.0.384.g4c02a37b29-goog

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Richard W.M. Jones 3 weeks, 2 days ago

On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
> Recently, syzbot started to abuse NBD with all kinds of sockets.
> 
> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> made sure the socket supported a shutdown() method.
> 
> Explicitely accept TCP and UNIX stream sockets.

I'm not clear what the actual problem is, but I will say that libnbd &
nbdkit (which are another NBD client & server, interoperable with the
kernel) we support and use NBD over vsock[1].  And we could support
NBD over pretty much any stream socket (Infiniband?) [2].

[1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
    https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
[2] https://libguestfs.org/nbd_connect_socket.3.html

TCP and Unix domain sockets are by far the most widely used, but I
don't think it's fair to exclude other socket types.

Rich.

> Fixes: cf1b2326b734 ("nbd: verify socket is supported during setup")
> Reported-by: syzbot+e1cd6bd8493060bd701d@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/CANn89iJ+76eE3A_8S_zTpSyW5hvPRn6V57458hCZGY5hbH_bFA@mail.gmail.com/T/#m081036e8747cd7e2626c1da5d78c8b9d1e55b154
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Richard W.M. Jones <rjones@redhat.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Yu Kuai <yukuai1@huaweicloud.com>
> Cc: linux-block@vger.kernel.org
> Cc: nbd@other.debian.org
> ---
>  drivers/block/nbd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 6463d0e8d0cef71e73e67fecd16de4dec1c75da7..87b0b78249da3325023949585f4daf40486c9692 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -1217,6 +1217,14 @@ static struct socket *nbd_get_socket(struct nbd_device *nbd, unsigned long fd,
>  	if (!sock)
>  		return NULL;
>  
> +	if (!sk_is_tcp(sock->sk) &&
> +	    !sk_is_stream_unix(sock->sk)) {
> +		dev_err(disk_to_dev(nbd->disk), "Unsupported socket: should be TCP or UNIX.\n");
> +		*err = -EINVAL;
> +		sockfd_put(sock);
> +		return NULL;
> +	}
> +
>  	if (sock->ops->shutdown == sock_no_shutdown) {
>  		dev_err(disk_to_dev(nbd->disk), "Unsupported socket: shutdown callout must be supported.\n");
>  		*err = -EINVAL;
> -- 
> 2.51.0.384.g4c02a37b29-goog

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Eric Dumazet 3 weeks, 2 days ago

On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
>
> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
> > Recently, syzbot started to abuse NBD with all kinds of sockets.
> >
> > Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> > made sure the socket supported a shutdown() method.
> >
> > Explicitely accept TCP and UNIX stream sockets.
>
> I'm not clear what the actual problem is, but I will say that libnbd &
> nbdkit (which are another NBD client & server, interoperable with the
> kernel) we support and use NBD over vsock[1].  And we could support
> NBD over pretty much any stream socket (Infiniband?) [2].
>
> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
> [2] https://libguestfs.org/nbd_connect_socket.3.html
>
> TCP and Unix domain sockets are by far the most widely used, but I
> don't think it's fair to exclude other socket types.

If we have known and supported socket types, please send a patch to add them.

I asked the question last week and got nothing about vsock or other types.

https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/

For sure, we do not want datagram sockets, RAW, netlink, and many others.

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Eric Dumazet 3 weeks, 2 days ago

On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
> >
> > On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
> > > Recently, syzbot started to abuse NBD with all kinds of sockets.
> > >
> > > Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> > > made sure the socket supported a shutdown() method.
> > >
> > > Explicitely accept TCP and UNIX stream sockets.
> >
> > I'm not clear what the actual problem is, but I will say that libnbd &
> > nbdkit (which are another NBD client & server, interoperable with the
> > kernel) we support and use NBD over vsock[1].  And we could support
> > NBD over pretty much any stream socket (Infiniband?) [2].
> >
> > [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
> >     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
> > [2] https://libguestfs.org/nbd_connect_socket.3.html
> >
> > TCP and Unix domain sockets are by far the most widely used, but I
> > don't think it's fair to exclude other socket types.
>
> If we have known and supported socket types, please send a patch to add them.
>
> I asked the question last week and got nothing about vsock or other types.
>
> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
>
> For sure, we do not want datagram sockets, RAW, netlink, and many others.

BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL being used
in net/vmw_vsock/virtio_transport.c

So you will have to fix this.

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Jens Axboe 3 weeks, 2 days ago

On 9/9/25 8:35 AM, Eric Dumazet wrote:
> On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
>>
>> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
>>>
>>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
>>>> Recently, syzbot started to abuse NBD with all kinds of sockets.
>>>>
>>>> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
>>>> made sure the socket supported a shutdown() method.
>>>>
>>>> Explicitely accept TCP and UNIX stream sockets.
>>>
>>> I'm not clear what the actual problem is, but I will say that libnbd &
>>> nbdkit (which are another NBD client & server, interoperable with the
>>> kernel) we support and use NBD over vsock[1].  And we could support
>>> NBD over pretty much any stream socket (Infiniband?) [2].
>>>
>>> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
>>>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
>>> [2] https://libguestfs.org/nbd_connect_socket.3.html
>>>
>>> TCP and Unix domain sockets are by far the most widely used, but I
>>> don't think it's fair to exclude other socket types.
>>
>> If we have known and supported socket types, please send a patch to add them.
>>
>> I asked the question last week and got nothing about vsock or other types.
>>
>> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
>>
>> For sure, we do not want datagram sockets, RAW, netlink, and many others.
> 
> BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL being used
> in net/vmw_vsock/virtio_transport.c
> 
> So you will have to fix this.

Rather than play whack-a-mole with this, would it make sense to mark as
socket as "writeback/reclaim" safe and base the nbd decision on that rather
than attempt to maintain some allow/deny list of sockets?

-- 
Jens Axboe

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Eric Dumazet 3 weeks, 2 days ago

On Tue, Sep 9, 2025 at 7:37 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 9/9/25 8:35 AM, Eric Dumazet wrote:
> > On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
> >>
> >> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
> >>>
> >>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
> >>>> Recently, syzbot started to abuse NBD with all kinds of sockets.
> >>>>
> >>>> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> >>>> made sure the socket supported a shutdown() method.
> >>>>
> >>>> Explicitely accept TCP and UNIX stream sockets.
> >>>
> >>> I'm not clear what the actual problem is, but I will say that libnbd &
> >>> nbdkit (which are another NBD client & server, interoperable with the
> >>> kernel) we support and use NBD over vsock[1].  And we could support
> >>> NBD over pretty much any stream socket (Infiniband?) [2].
> >>>
> >>> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
> >>>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
> >>> [2] https://libguestfs.org/nbd_connect_socket.3.html
> >>>
> >>> TCP and Unix domain sockets are by far the most widely used, but I
> >>> don't think it's fair to exclude other socket types.
> >>
> >> If we have known and supported socket types, please send a patch to add them.
> >>
> >> I asked the question last week and got nothing about vsock or other types.
> >>
> >> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
> >>
> >> For sure, we do not want datagram sockets, RAW, netlink, and many others.
> >
> > BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL being used
> > in net/vmw_vsock/virtio_transport.c
> >
> > So you will have to fix this.
>
> Rather than play whack-a-mole with this, would it make sense to mark as
> socket as "writeback/reclaim" safe and base the nbd decision on that rather
> than attempt to maintain some allow/deny list of sockets?

Even if a socket type was writeback/reclaim safe, probably NBD would not support
arbitrary socket type, like netlink, af_packet, or af_netrom.

An allow list seems safer to me, with commits with a clear owner.

If future syzbot reports are triggered, the bisection will point to
these commits.

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Richard W.M. Jones 3 weeks, 2 days ago

On Tue, Sep 09, 2025 at 07:47:09AM -0700, Eric Dumazet wrote:
> On Tue, Sep 9, 2025 at 7:37 AM Jens Axboe <axboe@kernel.dk> wrote:
> >
> > On 9/9/25 8:35 AM, Eric Dumazet wrote:
> > > On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
> > >>
> > >> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
> > >>>
> > >>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
> > >>>> Recently, syzbot started to abuse NBD with all kinds of sockets.
> > >>>>
> > >>>> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> > >>>> made sure the socket supported a shutdown() method.
> > >>>>
> > >>>> Explicitely accept TCP and UNIX stream sockets.
> > >>>
> > >>> I'm not clear what the actual problem is, but I will say that libnbd &
> > >>> nbdkit (which are another NBD client & server, interoperable with the
> > >>> kernel) we support and use NBD over vsock[1].  And we could support
> > >>> NBD over pretty much any stream socket (Infiniband?) [2].
> > >>>
> > >>> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
> > >>>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
> > >>> [2] https://libguestfs.org/nbd_connect_socket.3.html
> > >>>
> > >>> TCP and Unix domain sockets are by far the most widely used, but I
> > >>> don't think it's fair to exclude other socket types.
> > >>
> > >> If we have known and supported socket types, please send a patch to add them.
> > >>
> > >> I asked the question last week and got nothing about vsock or other types.
> > >>
> > >> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
> > >>
> > >> For sure, we do not want datagram sockets, RAW, netlink, and many others.
> > >
> > > BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL
> > > being used in net/vmw_vsock/virtio_transport.c

CC-ing Stefan & Stefano.  Myself, I'm only using libnbd
(ie. userspace) over vsock, not the kernel client.

> > > So you will have to fix this.
> >
> > Rather than play whack-a-mole with this, would it make sense to mark as
> > socket as "writeback/reclaim" safe and base the nbd decision on that rather
> > than attempt to maintain some allow/deny list of sockets?
> 
> Even if a socket type was writeback/reclaim safe, probably NBD would not support
> arbitrary socket type, like netlink, af_packet, or af_netrom.
> 
> An allow list seems safer to me, with commits with a clear owner.
> 
> If future syzbot reports are triggered, the bisection will point to
> these commits.

From the outside it seems really odd to hard code a list of "good"
socket types into each kernel client that can open a socket.  Normally
if you wanted to restrict socket types wouldn't you do that through
something more flexible like nftables?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Richard W.M. Jones 3 weeks, 2 days ago

So I was playing with this (see commands at end if you want to try)
and it turns out that the nbd-client program doesn't support vsock
anyway.  Of course you could still call the kernel APIs directly to
set up the socket, but it wouldn't be straightforward.

nbd-client did support Sockets Direct Protocol (SDP) but support was
removed in 2023.

The userspace tools like nbdinfo (part of libnbd) work fine, but of
course that's not relevant to the kernel NBD client.

Rich.


Commands to test vsock:

  $ virt-builder fedora-42

  $ nbdkit --vsock memory 1G \
           --run '
      qemu-system-x86_64 -machine accel=kvm:tcg \
                         -cpu host -m 4096 \
                         -drive file=fedora-42.img,format=raw,if=virtio \
                         -device vhost-vsock-pci,guest-cid=3
   '

Inside the guest:

  # dnf install nbdinfo
  # nbdinfo nbd+vsock:///
  (details of the 1G RAM disk will be shown here)

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Eric Dumazet 3 weeks, 2 days ago

On Tue, Sep 9, 2025 at 8:19 AM Richard W.M. Jones <rjones@redhat.com> wrote:
>
> On Tue, Sep 09, 2025 at 07:47:09AM -0700, Eric Dumazet wrote:
> > On Tue, Sep 9, 2025 at 7:37 AM Jens Axboe <axboe@kernel.dk> wrote:
> > >
> > > On 9/9/25 8:35 AM, Eric Dumazet wrote:
> > > > On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
> > > >>
> > > >> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
> > > >>>
> > > >>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
> > > >>>> Recently, syzbot started to abuse NBD with all kinds of sockets.
> > > >>>>
> > > >>>> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> > > >>>> made sure the socket supported a shutdown() method.
> > > >>>>
> > > >>>> Explicitely accept TCP and UNIX stream sockets.
> > > >>>
> > > >>> I'm not clear what the actual problem is, but I will say that libnbd &
> > > >>> nbdkit (which are another NBD client & server, interoperable with the
> > > >>> kernel) we support and use NBD over vsock[1].  And we could support
> > > >>> NBD over pretty much any stream socket (Infiniband?) [2].
> > > >>>
> > > >>> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
> > > >>>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
> > > >>> [2] https://libguestfs.org/nbd_connect_socket.3.html
> > > >>>
> > > >>> TCP and Unix domain sockets are by far the most widely used, but I
> > > >>> don't think it's fair to exclude other socket types.
> > > >>
> > > >> If we have known and supported socket types, please send a patch to add them.
> > > >>
> > > >> I asked the question last week and got nothing about vsock or other types.
> > > >>
> > > >> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
> > > >>
> > > >> For sure, we do not want datagram sockets, RAW, netlink, and many others.
> > > >
> > > > BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL
> > > > being used in net/vmw_vsock/virtio_transport.c
>
> CC-ing Stefan & Stefano.  Myself, I'm only using libnbd
> (ie. userspace) over vsock, not the kernel client.
>
> > > > So you will have to fix this.
> > >
> > > Rather than play whack-a-mole with this, would it make sense to mark as
> > > socket as "writeback/reclaim" safe and base the nbd decision on that rather
> > > than attempt to maintain some allow/deny list of sockets?
> >
> > Even if a socket type was writeback/reclaim safe, probably NBD would not support
> > arbitrary socket type, like netlink, af_packet, or af_netrom.
> >
> > An allow list seems safer to me, with commits with a clear owner.
> >
> > If future syzbot reports are triggered, the bisection will point to
> > these commits.
>
> From the outside it seems really odd to hard code a list of "good"
> socket types into each kernel client that can open a socket.  Normally
> if you wanted to restrict socket types wouldn't you do that through
> something more flexible like nftables?

nftables is user policy.

We need a kernel that will not crash, even if nftables is not
compiled/loaded/used .


>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-p2v converts physical machines to virtual machines.  Boot with a
> live CD or over the network (PXE) and turn machines into KVM guests.
> http://libguestfs.org/virt-v2v
>

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Simon Horman 2 weeks, 6 days ago

On Tue, Sep 09, 2025 at 08:33:27AM -0700, Eric Dumazet wrote:
> On Tue, Sep 9, 2025 at 8:19 AM Richard W.M. Jones <rjones@redhat.com> wrote:
> > On Tue, Sep 09, 2025 at 07:47:09AM -0700, Eric Dumazet wrote:
> > > On Tue, Sep 9, 2025 at 7:37 AM Jens Axboe <axboe@kernel.dk> wrote:
> > > > On 9/9/25 8:35 AM, Eric Dumazet wrote:
> > > > > On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
> > > > >> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
> > > > >>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:

...

> > From the outside it seems really odd to hard code a list of "good"
> > socket types into each kernel client that can open a socket.  Normally
> > if you wanted to restrict socket types wouldn't you do that through
> > something more flexible like nftables?
> 
> nftables is user policy.
> 
> We need a kernel that will not crash, even if nftables is not
> compiled/loaded/used .

Hi Rich, Eric, all,

FWIIW, I think that the kernel maintaining a list of acceptable and
known to work socket types is a reasonable measure. It reduces the
surface where problems can arise - a surface that has real bugs.
And can be expanded as necessary.

For sure it is not perfect. There is a risk of entering wack-a-mole
territory. And a more flexible mechanism may be nice.

But, OTOH, we may be speculating about a problem that doesn't exist.
If, very occasionally, a new socket type comes along and has to be used.
Or perhaps more likely, there is a follow-up to this change for some
cases it missed (i.e. the topic of this thread). And if that is very
occasional. Is there really a problem?

The answer is of course subjective. But I lean towards no.

...

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Stefano Garzarella 3 weeks, 1 day ago

On Tue, Sep 09, 2025 at 08:33:27AM -0700, Eric Dumazet wrote:
>On Tue, Sep 9, 2025 at 8:19 AM Richard W.M. Jones <rjones@redhat.com> wrote:
>>
>> On Tue, Sep 09, 2025 at 07:47:09AM -0700, Eric Dumazet wrote:
>> > On Tue, Sep 9, 2025 at 7:37 AM Jens Axboe <axboe@kernel.dk> wrote:
>> > >
>> > > On 9/9/25 8:35 AM, Eric Dumazet wrote:
>> > > > On Tue, Sep 9, 2025 at 7:04 AM Eric Dumazet <edumazet@google.com> wrote:
>> > > >>
>> > > >> On Tue, Sep 9, 2025 at 6:32 AM Richard W.M. Jones <rjones@redhat.com> wrote:
>> > > >>>
>> > > >>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
>> > > >>>> Recently, syzbot started to abuse NBD with all kinds of sockets.
>> > > >>>>
>> > > >>>> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
>> > > >>>> made sure the socket supported a shutdown() method.
>> > > >>>>
>> > > >>>> Explicitely accept TCP and UNIX stream sockets.
>> > > >>>
>> > > >>> I'm not clear what the actual problem is, but I will say that libnbd &
>> > > >>> nbdkit (which are another NBD client & server, interoperable with the
>> > > >>> kernel) we support and use NBD over vsock[1].  And we could support
>> > > >>> NBD over pretty much any stream socket (Infiniband?) [2].
>> > > >>>
>> > > >>> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
>> > > >>>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
>> > > >>> [2] https://libguestfs.org/nbd_connect_socket.3.html
>> > > >>>
>> > > >>> TCP and Unix domain sockets are by far the most widely used, but I
>> > > >>> don't think it's fair to exclude other socket types.
>> > > >>
>> > > >> If we have known and supported socket types, please send a patch to add them.
>> > > >>
>> > > >> I asked the question last week and got nothing about vsock or other types.
>> > > >>
>> > > >> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
>> > > >>
>> > > >> For sure, we do not want datagram sockets, RAW, netlink, and many others.
>> > > >
>> > > > BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL
>> > > > being used in net/vmw_vsock/virtio_transport.c
>>
>> CC-ing Stefan & Stefano.  Myself, I'm only using libnbd
>> (ie. userspace) over vsock, not the kernel client.

Thanks Rich for cceing me!

>>
>> > > > So you will have to fix this.

How we should fix that?

IIUC GFP_KERNEL in virtio_transport.c is used only by workqueue's 
functions, but we have GFP_ATOMIC in the fast path that can be called 
when the user is sending a packet.

This is basically the driver for the virtio-vsock device that can 
allocates extra buffers to be exposed to the device.
In this case the allocation can happen in virtqueue_add_sgs() for virtio 
indirect buffer, that IIRC virtio-vsock is not using currently (but we 
don't know in the future).

In any case, we use GFP_KERNEL also in virtio_transport_common.c to 
allocate the sk_buff, so that should be the same issue.

Thanks,
Stefano

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Jens Axboe 3 weeks, 2 days ago

On 9/9/25 8:47 AM, Eric Dumazet wrote:
> On Tue, Sep 9, 2025 at 7:37?AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 9/9/25 8:35 AM, Eric Dumazet wrote:
>>> On Tue, Sep 9, 2025 at 7:04?AM Eric Dumazet <edumazet@google.com> wrote:
>>>>
>>>> On Tue, Sep 9, 2025 at 6:32?AM Richard W.M. Jones <rjones@redhat.com> wrote:
>>>>>
>>>>> On Tue, Sep 09, 2025 at 01:22:43PM +0000, Eric Dumazet wrote:
>>>>>> Recently, syzbot started to abuse NBD with all kinds of sockets.
>>>>>>
>>>>>> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
>>>>>> made sure the socket supported a shutdown() method.
>>>>>>
>>>>>> Explicitely accept TCP and UNIX stream sockets.
>>>>>
>>>>> I'm not clear what the actual problem is, but I will say that libnbd &
>>>>> nbdkit (which are another NBD client & server, interoperable with the
>>>>> kernel) we support and use NBD over vsock[1].  And we could support
>>>>> NBD over pretty much any stream socket (Infiniband?) [2].
>>>>>
>>>>> [1] https://libguestfs.org/nbd_aio_connect_vsock.3.html
>>>>>     https://libguestfs.org/nbdkit-service.1.html#AF_VSOCK
>>>>> [2] https://libguestfs.org/nbd_connect_socket.3.html
>>>>>
>>>>> TCP and Unix domain sockets are by far the most widely used, but I
>>>>> don't think it's fair to exclude other socket types.
>>>>
>>>> If we have known and supported socket types, please send a patch to add them.
>>>>
>>>> I asked the question last week and got nothing about vsock or other types.
>>>>
>>>> https://lore.kernel.org/netdev/CANn89iLNFHBMTF2Pb6hHERYpuih9eQZb6A12+ndzBcQs_kZoBA@mail.gmail.com/
>>>>
>>>> For sure, we do not want datagram sockets, RAW, netlink, and many others.
>>>
>>> BTW vsock will probably fire lockdep warnings, I see GFP_KERNEL being used
>>> in net/vmw_vsock/virtio_transport.c
>>>
>>> So you will have to fix this.
>>
>> Rather than play whack-a-mole with this, would it make sense to mark as
>> socket as "writeback/reclaim" safe and base the nbd decision on that rather
>> than attempt to maintain some allow/deny list of sockets?
> 
> Even if a socket type was writeback/reclaim safe, probably NBD would
> not support arbitrary socket type, like netlink, af_packet, or
> af_netrom.
> 
> An allow list seems safer to me, with commits with a clear owner.
> 
> If future syzbot reports are triggered, the bisection will point to
> these commits.

That's fine too, either approach will result in fixups, at the end of
the day. And followup related fixes to solve issues with socket types
that we do deem useful, like the vsock one you already found.

-- 
Jens Axboe

Re: [PATCH] nbd: restrict sockets to TCP and UDP

Posted by Jens Axboe 3 weeks, 2 days ago

On Tue, 09 Sep 2025 13:22:43 +0000, Eric Dumazet wrote:
> Recently, syzbot started to abuse NBD with all kinds of sockets.
> 
> Commit cf1b2326b734 ("nbd: verify socket is supported during setup")
> made sure the socket supported a shutdown() method.
> 
> Explicitely accept TCP and UNIX stream sockets.
> 
> [...]

Applied, thanks!

[1/1] nbd: restrict sockets to TCP and UDP
      commit: 9f7c02e031570e8291a63162c6c046dc15ff85b0

Best regards,
-- 
Jens Axboe