Re: [BUG] vsock: refcount_t saturation and OOM via buffer size invariant inversion

[Stefano Garzarella <sgarzare@redhat.com>]

On Mon, Apr 06, 2026 at 08:36:16PM +0200, Norbert Szetei wrote:
>Hi Stefano,
>
>I like option 1 the most, as it is the most straightforward way to fix
>the issue. I am including the patch below. This fixes the clamping
>mismatch, but as you pointed out, it won't solve the root problem
>regarding the issue to arbitrarily set a maximum buffer value.
>
>Since VSOCK uses a unified buffer size rather than separating read and
>write buffers like the core stack, introducing a vsock-specific sysctl
>(e.g., net.vmw_vsock.buffer_max_size) seems the cleanest approach to me.
>I was also considering net.core.wmem_max/rmem_max, but mapping to those
>feels less natural.

I would prefer to stop to add custom sysctl/sockopt for vsock and start 
to reuse the common ones, so if net.core.wmem_max/rmem_max can be used 
in some way, maybe we can try that path first and fallback to a custom 
one if we see it isn't doable.

>
>If you agree with the vsock-specific sysctl, or have a different
>suggestion, let me know and I will send a follow-up patch for that.
>Thanks.
>
>Best, Norbert
>
>-- >8 --
>From f5d160167c862c7f2ad6e6a1d4181d01997b683a Mon Sep 17 00:00:00 2001
>From: Norbert Szetei <norbert@doyensec.com>
>Date: Mon, 6 Apr 2026 19:52:52 +0200
>Subject: [PATCH] vsock: fix buffer size clamping order
>
>In vsock_update_buffer_size(), the buffer size was being clamped to the
>maximum first, and then to the minimum. If a user sets a minimum buffer
>size larger than the maximum, the minimum check overrides the maximum
>check, inverting the constraint.
>
>This breaks the intended socket memory boundaries by allowing the
>vsk->buffer_size to grow beyond the configured vsk->buffer_max_size.
>
>Fix this by checking the minimum first, and then the maximum. This
>ensures the buffer size never exceeds the buffer_max_size.

Please add a Fixes tag here, that should be:

Fixes: b9f2b0ffde0c ("vsock: handle buffer_size sockopts in the core")

For virtio transport it was pre-existing of that commit IIUC, but 
doesn't metter since these changes will not apply without that commit, 
so should be fine.

Please add also a Suggested-by.

>
>Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>---
> net/vmw_vsock/af_vsock.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index d912ed2f012a..08f4dfb9782c 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1951,12 +1951,12 @@ static void vsock_update_buffer_size(struct vsock_sock *vsk,
> 				    const struct vsock_transport *transport,
> 				    u64 val)
> {
>-	if (val > vsk->buffer_max_size)
>-		val = vsk->buffer_max_size;
>-
> 	if (val < vsk->buffer_min_size)
> 		val = vsk->buffer_min_size;
>
>+	if (val > vsk->buffer_max_size)
>+		val = vsk->buffer_max_size;
>+

The patch itself LGTM!

Thanks,
Stefano