Hi, v2: fix indentation/whitespace issues in the previous patch.
-- >8 -- From f5d160167c862c7f2ad6e6a1d4181d01997b683a Mon Sep 17 00:00:00 2001 From: Norbert Szetei <[email protected]> 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. Signed-off-by: Norbert Szetei <[email protected]> --- 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; + if (val != vsk->buffer_size && transport && transport->notify_buffer_size) transport->notify_buffer_size(vsk, &val); -- 2.53.0 > On Apr 6, 2026, at 20:41, Norbert Szetei <[email protected]> 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. > > 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 <[email protected]> > 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. > > Signed-off-by: Norbert Szetei <[email protected]> > --- > 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; > + > if (val != vsk->buffer_size && > transport && transport->notify_buffer_size) > transport->notify_buffer_size(vsk, &val); > -- > 2.53.0 > >> On Mar 31, 2026, at 14:37, Stefano Garzarella <[email protected]> wrote: >> >> On Tue, Mar 24, 2026 at 06:28:12PM +0100, Norbert Szetei wrote: >>> Hello, >>> >>> we have discovered a bug in AF_VSOCK where an unprivileged user can bypass >>> socket >>> memory constraints. This leads to refcount_t saturation and OOM. While >>> refcount_t prevents a true UAF by saturating, the resulting state triggers >>> kernel warnings and kernel panic, depending on the setup. >>> >>> In vsock_connectible_setsockopt(), the SO_VM_SOCKETS_BUFFER_MIN_SIZE and >>> SO_VM_SOCKETS_BUFFER_MAX_SIZE options are used to update the buffer's >>> minimum >>> and maximum values independently. >>> >>> The vsock_update_buffer_size() function clamps the buffer size to the >>> maximum >>> first, then the minimum: >>> >>> // >>> https://github.com/torvalds/linux/blob/c369299895a591d96745d6492d4888259b004a9e/net/vmw_vsock/af_vsock.c#L1950 >>> if (val > vsk->buffer_max_size) >>> val = vsk->buffer_max_size; >>> >>> if (val < vsk->buffer_min_size) >>> val = vsk->buffer_min_size; >>> >>> vsk->buffer_size = val; >>> >>> By setting buffer_min_size to a large value, the second clamp overrides the >>> first, forcing vsk->buffer_size to exceed the intended maximum. The >>> transport >>> layer then uses this value, allowing unbounded SKB allocation that >>> saturates the >>> 32-bit sk_wmem_alloc refcount. >>> >>> The fix should ensure that SO_VM_SOCKETS_BUFFER_MIN_SIZE cannot be used to >>> set a >>> value higher than the current buffer_max_size. Conversely, >>> SO_VM_SOCKETS_BUFFER_MAX_SIZE should not be allowed to be set lower than the >>> current buffer_min_size. >> >> Okay, but that wouldn't change much. As long as the user sets the maximum to >> match the minimum you set in the POC, it behaves exactly the same way, right? >> >> Maybe we should add a sysctl to set a global upper bound, but this is >> another problem, I agree that we should improve the kernel behavior around >> min/max. I see 3 options: >> >> 1. Just invert the checks, fist check for min, then for max. >> >> 2. Simply adjust the min and max values so that they make sense. For >> example, if the minimum value being set is greater than the maximum, the >> kernel could adjust the maximum to the same value. However, this would not >> change the behavior of your POC. >> >> 3. Force the minimum to be less than or equal to the maximum. This, however, >> would require a certain order when setting the minimum and maximum, >> especially relative to the default. For example, if you increase the minimum >> beyond the default maximum, you must adjust the maximum first; conversely, >> if you want to set the maximum below the default minimum, you must adjust >> the minimum first. >> >> I'm more into 1 or 2. 3 IMO is too much. >> >> Do you want to send a patch? >> >> Thanks, >> Stefano > >
