Re: [PATCH net v1] vsock: validate buffer min/max size in setsockopt

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;