On 2026/8/24 20:16, David Laight wrote:
> On Mon, 24 Aug 2026 17:12:57 +0800
> Rongguang Wei <[email protected]> wrote:
>
>> From: Rongguang Wei <[email protected]>
>>
>> SO_VM_SOCKETS_BUFFER_MIN_SIZE and SO_VM_SOCKETS_BUFFER_MAX_SIZE
>> do not cross-validate against each other, allowing userspace to
>> set buffer_min_size > buffer_max_size.
>> When min > max, buffer_size is silently clamped to an incorrect
>> value. For example, setting min=512KB then max=128 results in
>> buffer_size=128 despite the user requesting much larger buffers
>> via SO_VM_SOCKETS_BUFFER_SIZE.
>>
>> Reproduced with a test program:
>> setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_MIN_SIZE,
>> 512 * 1024, sizeof(int));
>> setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_MAX_SIZE,
>> 128, sizeof(int));
>> // User asked for 1MB but got 128 bytes silently
>> setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_SIZE,
>> 1024 * 1024, sizeof(int));
>>
>> After that use getsockopt to get the buffer_size = 128 and
>> buffer_min_size = 524288, buffer_max_size = 128.
>> The buffer_min_size > buffer_max_size and the kernel accepted
>> contradictory values without error.
>>
>> Add value check to fix this issue. Return -EINVAL to userspace
>> when setting MAX_SIZE to a value smaller than the current MIN_SIZE
>> or setting MIN_SIZE to a value larger than the current MAX_SIZE.
>
> That is going to break userspace that sets the minimum before the maximum
> when the new minimum is larger than the old maximum.
>
> David
>
Hi, David.
Thanks for pointing out the issue.
Here is an alternative approach:
instead of returning -EINVAL, automatically adjust the other value to preserve
buffer_min_size <= buffer_max_size.
The trade-off is that setting one parameter may implicitly adjust the other,
but this is preferable to breaking existing applications.
Does this approach look reasonable?
---
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index a33b2a2d381d..bd4646115af5 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2050,12 +2050,18 @@ static int vsock_connectible_setsockopt(struct socket
*sock,
case SO_VM_SOCKETS_BUFFER_MAX_SIZE:
COPY_IN(val);
+ /* If the new max is less than current min, shrink min to
match. */
+ if (val < vsk->buffer_min_size)
+ vsk->buffer_min_size = val;
vsk->buffer_max_size = val;
vsock_update_buffer_size(vsk, transport, vsk->buffer_size);
break;
case SO_VM_SOCKETS_BUFFER_MIN_SIZE:
COPY_IN(val);
+ /* If the new min is greater than current max, expand max to
match. */
+ if (val > vsk->buffer_max_size)
+ vsk->buffer_max_size = val;
vsk->buffer_min_size = val;
vsock_update_buffer_size(vsk, transport, vsk->buffer_size);
break;
>>
>> Fixes: b9f2b0ffde0c ("vsock: handle buffer_size sockopts in the core")
>> Signed-off-by: Rongguang Wei <[email protected]>
>> ---
>> net/vmw_vsock/af_vsock.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index a33b2a2d381d..5faa30ee8745 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2050,12 +2050,20 @@ static int vsock_connectible_setsockopt(struct
>> socket *sock,
>>
>> case SO_VM_SOCKETS_BUFFER_MAX_SIZE:
>> COPY_IN(val);
>> + if (val < vsk->buffer_min_size) {
>> + err = -EINVAL;
>> + goto exit;
>> + }
>> vsk->buffer_max_size = val;
>> vsock_update_buffer_size(vsk, transport, vsk->buffer_size);
>> break;
>>
>> case SO_VM_SOCKETS_BUFFER_MIN_SIZE:
>> COPY_IN(val);
>> + if (val > vsk->buffer_max_size) {
>> + err = -EINVAL;
>> + goto exit;
>> + }
>> vsk->buffer_min_size = val;
>> vsock_update_buffer_size(vsk, transport, vsk->buffer_size);
>> break;
