Thread (5 messages) flat view 5 messages, 4 authors, 1d ago

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

From: weirongguang <hidden>
Date: 2026-09-07 07:46:03
Also in: virtualization
Subsystem: networking [general], the rest, vm sockets (af_vsock) · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Stefano Garzarella


On 2026/8/24 20:16, David Laight wrote:
On Mon, 24 Aug 2026 17:12:57 +0800
Rongguang Wei [off-list ref] wrote:
quoted
From: Rongguang Wei <redacted>

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;                                                           
quoted
Fixes: b9f2b0ffde0c ("vsock: handle buffer_size sockopts in the core")
Signed-off-by: Rongguang Wei <redacted>
---
 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;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help