Loopback transport can mangle data in rx queue when a linear skb is
followed by a small MSG_ZEROCOPY packet.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 tools/testing/vsock/vsock_test.c          |  5 +++
 tools/testing/vsock/vsock_test_zerocopy.c | 67 +++++++++++++++++++++++++++++++
 tools/testing/vsock/vsock_test_zerocopy.h |  3 ++
 3 files changed, 75 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index bbe3723babdc..21c8616100f1 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
 		.run_client = test_stream_accepted_setsockopt_client,
 		.run_server = test_stream_accepted_setsockopt_server,
 	},
+	{
+		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
+		.run_client = test_stream_msgzcopy_mangle_client,
+		.run_server = test_stream_msgzcopy_mangle_server,
+	},
 	{},
 };
 
diff --git a/tools/testing/vsock/vsock_test_zerocopy.c b/tools/testing/vsock/vsock_test_zerocopy.c
index 9d9a6cb9614a..6735a9d7525d 100644
--- a/tools/testing/vsock/vsock_test_zerocopy.c
+++ b/tools/testing/vsock/vsock_test_zerocopy.c
@@ -9,11 +9,13 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <unistd.h>
 #include <poll.h>
 #include <linux/errqueue.h>
 #include <linux/kernel.h>
+#include <linux/sockios.h>
 #include <errno.h>
 
 #include "control.h"
@@ -356,3 +358,68 @@ void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts)
 	control_expectln("DONE");
 	close(fd);
 }
+
+#define GOOD_COPY_LEN	128	/* net/vmw_vsock/virtio_transport_common.c */
+
+void test_stream_msgzcopy_mangle_client(const struct test_opts *opts)
+{
+	char sbuf1[PAGE_SIZE + 1], sbuf2[GOOD_COPY_LEN];
+	struct pollfd fds;
+	int fd;
+
+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	enable_so_zerocopy_check(fd);
+
+	memset(sbuf1, '1', sizeof(sbuf1));
+	memset(sbuf2, '2', sizeof(sbuf2));
+
+	send_buf(fd, sbuf1, sizeof(sbuf1), 0, sizeof(sbuf1));
+	send_buf(fd, sbuf2, sizeof(sbuf2), MSG_ZEROCOPY, sizeof(sbuf2));
+
+	fds.fd = fd;
+	fds.events = 0;
+
+	if (poll(&fds, 1, -1) != 1 || !(fds.revents & POLLERR)) {
+		perror("poll");
+		exit(EXIT_FAILURE);
+	}
+
+	close(fd);
+}
+
+static void recv_verify(int fd, char *buf, unsigned int len, char pattern)
+{
+	recv_buf(fd, buf, len, 0, len);
+
+	while (len--) {
+		if (*buf++ != pattern) {
+			fprintf(stderr, "Incorrect data received\n");
+			exit(EXIT_FAILURE);
+		}
+	}
+}
+
+void test_stream_msgzcopy_mangle_server(const struct test_opts *opts)
+{
+	char rbuf[PAGE_SIZE + 1];
+	int fd;
+
+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
+	if (fd < 0) {
+		perror("accept");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Wait, don't race the (buggy) skbs coalescence. */
+	vsock_ioctl_int(fd, SIOCINQ, PAGE_SIZE + 1 + GOOD_COPY_LEN);
+
+	recv_verify(fd, rbuf, PAGE_SIZE + 1, '1');
+	recv_verify(fd, rbuf, GOOD_COPY_LEN, '2');
+
+	close(fd);
+}
diff --git a/tools/testing/vsock/vsock_test_zerocopy.h b/tools/testing/vsock/vsock_test_zerocopy.h
index 3ef2579e024d..d46c91a69f16 100644
--- a/tools/testing/vsock/vsock_test_zerocopy.h
+++ b/tools/testing/vsock/vsock_test_zerocopy.h
@@ -12,4 +12,7 @@ void test_seqpacket_msgzcopy_server(const struct test_opts *opts);
 void test_stream_msgzcopy_empty_errq_client(const struct test_opts *opts);
 void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts);
 
+void test_stream_msgzcopy_mangle_client(const struct test_opts *opts);
+void test_stream_msgzcopy_mangle_server(const struct test_opts *opts);
+
 #endif /* VSOCK_TEST_ZEROCOPY_H */

-- 
2.52.0

On Thu, Jan 08, 2026 at 10:54:55AM +0100, Michal Luczaj wrote:
>Loopback transport can mangle data in rx queue when a linear skb is
>followed by a small MSG_ZEROCOPY packet.

Can we describe a bit more what the test is doing?

>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> tools/testing/vsock/vsock_test.c          |  5 +++
> tools/testing/vsock/vsock_test_zerocopy.c | 67 +++++++++++++++++++++++++++++++
> tools/testing/vsock/vsock_test_zerocopy.h |  3 ++
> 3 files changed, 75 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index bbe3723babdc..21c8616100f1 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
> 		.run_client = test_stream_accepted_setsockopt_client,
> 		.run_server = test_stream_accepted_setsockopt_server,
> 	},
>+	{
>+		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",

This is essentially a regression test for virtio transport, so I'd add 
virtio in the test name.

>+		.run_client = test_stream_msgzcopy_mangle_client,
>+		.run_server = test_stream_msgzcopy_mangle_server,
>+	},
> 	{},
> };
>
>diff --git a/tools/testing/vsock/vsock_test_zerocopy.c b/tools/testing/vsock/vsock_test_zerocopy.c
>index 9d9a6cb9614a..6735a9d7525d 100644
>--- a/tools/testing/vsock/vsock_test_zerocopy.c
>+++ b/tools/testing/vsock/vsock_test_zerocopy.c
>@@ -9,11 +9,13 @@
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>+#include <sys/ioctl.h>
> #include <sys/mman.h>
> #include <unistd.h>
> #include <poll.h>
> #include <linux/errqueue.h>
> #include <linux/kernel.h>
>+#include <linux/sockios.h>
> #include <errno.h>
>
> #include "control.h"
>@@ -356,3 +358,68 @@ void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts)
> 	control_expectln("DONE");
> 	close(fd);
> }
>+
>+#define GOOD_COPY_LEN	128	/* net/vmw_vsock/virtio_transport_common.c */

I think we don't need this, I mean we can eventually just send a single 
byte, no?

>+
>+void test_stream_msgzcopy_mangle_client(const struct test_opts *opts)
>+{
>+	char sbuf1[PAGE_SIZE + 1], sbuf2[GOOD_COPY_LEN];
>+	struct pollfd fds;
>+	int fd;
>+
>+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	enable_so_zerocopy_check(fd);
>+
>+	memset(sbuf1, '1', sizeof(sbuf1));
>+	memset(sbuf2, '2', sizeof(sbuf2));
>+
>+	send_buf(fd, sbuf1, sizeof(sbuf1), 0, sizeof(sbuf1));
>+	send_buf(fd, sbuf2, sizeof(sbuf2), MSG_ZEROCOPY, sizeof(sbuf2));
>+
>+	fds.fd = fd;
>+	fds.events = 0;
>+
>+	if (poll(&fds, 1, -1) != 1 || !(fds.revents & POLLERR)) {
>+		perror("poll");
>+		exit(EXIT_FAILURE);
>+	}

Should we also call vsock_recv_completion() or we don't care about the 
result?

If we need it, maybe we can factor our the poll + 
vsock_recv_completion().

>+
>+	close(fd);
>+}
>+
>+static void recv_verify(int fd, char *buf, unsigned int len, char pattern)
>+{
>+	recv_buf(fd, buf, len, 0, len);
>+
>+	while (len--) {
>+		if (*buf++ != pattern) {
>+			fprintf(stderr, "Incorrect data received\n");
>+			exit(EXIT_FAILURE);
>+		}
>+	}
>+}
>+
>+void test_stream_msgzcopy_mangle_server(const struct test_opts *opts)
>+{
>+	char rbuf[PAGE_SIZE + 1];
>+	int fd;
>+
>+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>+	if (fd < 0) {
>+		perror("accept");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	/* Wait, don't race the (buggy) skbs coalescence. */
>+	vsock_ioctl_int(fd, SIOCINQ, PAGE_SIZE + 1 + GOOD_COPY_LEN);

This is cool, another option is to add a barrier here (and after the 
poll), but yeah, this should be even better.

Thanks,
Stefano

>+
>+	recv_verify(fd, rbuf, PAGE_SIZE + 1, '1');
>+	recv_verify(fd, rbuf, GOOD_COPY_LEN, '2');
>+
>+	close(fd);
>+}
>diff --git a/tools/testing/vsock/vsock_test_zerocopy.h b/tools/testing/vsock/vsock_test_zerocopy.h
>index 3ef2579e024d..d46c91a69f16 100644
>--- a/tools/testing/vsock/vsock_test_zerocopy.h
>+++ b/tools/testing/vsock/vsock_test_zerocopy.h
>@@ -12,4 +12,7 @@ void test_seqpacket_msgzcopy_server(const struct test_opts *opts);
> void test_stream_msgzcopy_empty_errq_client(const struct test_opts *opts);
> void test_stream_msgzcopy_empty_errq_server(const struct test_opts 
> *opts);
>
>+void test_stream_msgzcopy_mangle_client(const struct test_opts *opts);
>+void test_stream_msgzcopy_mangle_server(const struct test_opts *opts);
>+
> #endif /* VSOCK_TEST_ZEROCOPY_H */
>
>-- 
>2.52.0
>

On 1/9/26 17:32, Stefano Garzarella wrote:
> On Thu, Jan 08, 2026 at 10:54:55AM +0100, Michal Luczaj wrote:
>> Loopback transport can mangle data in rx queue when a linear skb is
>> followed by a small MSG_ZEROCOPY packet.
> 
> Can we describe a bit more what the test is doing?

I've expanded the commit message:

To exercise the logic, send out two packets: a weirdly sized one (to ensure
some spare tail room in the skb) and a zerocopy one that's small enough to
fit in the spare room of its predecessor. Then, wait for both to land in
the rx queue, and check the data received. Faulty packets merger manifests
itself by corrupting payload of the later packet.

>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>> tools/testing/vsock/vsock_test.c          |  5 +++
>> tools/testing/vsock/vsock_test_zerocopy.c | 67 +++++++++++++++++++++++++++++++
>> tools/testing/vsock/vsock_test_zerocopy.h |  3 ++
>> 3 files changed, 75 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index bbe3723babdc..21c8616100f1 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>> 		.run_client = test_stream_accepted_setsockopt_client,
>> 		.run_server = test_stream_accepted_setsockopt_server,
>> 	},
>> +	{
>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
> 
> This is essentially a regression test for virtio transport, so I'd add 
> virtio in the test name.

Isn't virtio transport unaffected? It's about loopback transport (that
shares common code with virtio transport).

>> +		.run_client = test_stream_msgzcopy_mangle_client,
>> +		.run_server = test_stream_msgzcopy_mangle_server,
>> +	},
>> 	{},
>> };
>>
>> diff --git a/tools/testing/vsock/vsock_test_zerocopy.c b/tools/testing/vsock/vsock_test_zerocopy.c
>> index 9d9a6cb9614a..6735a9d7525d 100644
>> --- a/tools/testing/vsock/vsock_test_zerocopy.c
>> +++ b/tools/testing/vsock/vsock_test_zerocopy.c
>> @@ -9,11 +9,13 @@
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> +#include <sys/ioctl.h>
>> #include <sys/mman.h>
>> #include <unistd.h>
>> #include <poll.h>
>> #include <linux/errqueue.h>
>> #include <linux/kernel.h>
>> +#include <linux/sockios.h>
>> #include <errno.h>
>>
>> #include "control.h"
>> @@ -356,3 +358,68 @@ void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts)
>> 	control_expectln("DONE");
>> 	close(fd);
>> }
>> +
>> +#define GOOD_COPY_LEN	128	/* net/vmw_vsock/virtio_transport_common.c */
> 
> I think we don't need this, I mean we can eventually just send a single 
> byte, no?

For a single byte sent, you get a single byte of uninitialized kernel
memory. Uninitialized memory can by anything, in particular it can be
(coincidentally) what you happen to expect. Which would result in a false
positive. So instead of estimating what length sufficiently reduces
probability of such false positive, I just took the upper bound.

BTW, I've realized recv_verify() is reinventing the wheel. How about
dropping it in favour of what test_seqpacket_msg_bounds_client() does, i.e.
calc the hash of payload and send it over the control channel for verification?

>> +
>> +void test_stream_msgzcopy_mangle_client(const struct test_opts *opts)
>> +{
>> +	char sbuf1[PAGE_SIZE + 1], sbuf2[GOOD_COPY_LEN];
>> +	struct pollfd fds;
>> +	int fd;
>> +
>> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>> +	if (fd < 0) {
>> +		perror("connect");
>> +		exit(EXIT_FAILURE);
>> +	}
>> +
>> +	enable_so_zerocopy_check(fd);
>> +
>> +	memset(sbuf1, '1', sizeof(sbuf1));
>> +	memset(sbuf2, '2', sizeof(sbuf2));
>> +
>> +	send_buf(fd, sbuf1, sizeof(sbuf1), 0, sizeof(sbuf1));
>> +	send_buf(fd, sbuf2, sizeof(sbuf2), MSG_ZEROCOPY, sizeof(sbuf2));
>> +
>> +	fds.fd = fd;
>> +	fds.events = 0;
>> +
>> +	if (poll(&fds, 1, -1) != 1 || !(fds.revents & POLLERR)) {
>> +		perror("poll");
>> +		exit(EXIT_FAILURE);
>> +	}
> 
> Should we also call vsock_recv_completion() or we don't care about the 
> result?
> 
> If we need it, maybe we can factor our the poll + 
> vsock_recv_completion().

Nope, we don't care about the result.

On Sun, Jan 11, 2026 at 11:59:54AM +0100, Michal Luczaj wrote:
>On 1/9/26 17:32, Stefano Garzarella wrote:
>> On Thu, Jan 08, 2026 at 10:54:55AM +0100, Michal Luczaj wrote:
>>> Loopback transport can mangle data in rx queue when a linear skb is
>>> followed by a small MSG_ZEROCOPY packet.
>>
>> Can we describe a bit more what the test is doing?
>
>I've expanded the commit message:
>
>To exercise the logic, send out two packets: a weirdly sized one (to ensure
>some spare tail room in the skb) and a zerocopy one that's small enough to
>fit in the spare room of its predecessor. Then, wait for both to land in
>the rx queue, and check the data received. Faulty packets merger manifests
>itself by corrupting payload of the later packet.

Thanks! LGTM!

>
>>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>>> ---
>>> tools/testing/vsock/vsock_test.c          |  5 +++
>>> tools/testing/vsock/vsock_test_zerocopy.c | 67 +++++++++++++++++++++++++++++++
>>> tools/testing/vsock/vsock_test_zerocopy.h |  3 ++
>>> 3 files changed, 75 insertions(+)
>>>
>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>> index bbe3723babdc..21c8616100f1 100644
>>> --- a/tools/testing/vsock/vsock_test.c
>>> +++ b/tools/testing/vsock/vsock_test.c
>>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>>> 		.run_client = test_stream_accepted_setsockopt_client,
>>> 		.run_server = test_stream_accepted_setsockopt_server,
>>> 	},
>>> +	{
>>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
>>
>> This is essentially a regression test for virtio transport, so I'd add
>> virtio in the test name.
>
>Isn't virtio transport unaffected? It's about loopback transport (that
>shares common code with virtio transport).

Why virtio transport is not affected?

>
>>> +		.run_client = test_stream_msgzcopy_mangle_client,
>>> +		.run_server = test_stream_msgzcopy_mangle_server,
>>> +	},
>>> 	{},
>>> };
>>>
>>> diff --git a/tools/testing/vsock/vsock_test_zerocopy.c b/tools/testing/vsock/vsock_test_zerocopy.c
>>> index 9d9a6cb9614a..6735a9d7525d 100644
>>> --- a/tools/testing/vsock/vsock_test_zerocopy.c
>>> +++ b/tools/testing/vsock/vsock_test_zerocopy.c
>>> @@ -9,11 +9,13 @@
>>> #include <stdio.h>
>>> #include <stdlib.h>
>>> #include <string.h>
>>> +#include <sys/ioctl.h>
>>> #include <sys/mman.h>
>>> #include <unistd.h>
>>> #include <poll.h>
>>> #include <linux/errqueue.h>
>>> #include <linux/kernel.h>
>>> +#include <linux/sockios.h>
>>> #include <errno.h>
>>>
>>> #include "control.h"
>>> @@ -356,3 +358,68 @@ void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts)
>>> 	control_expectln("DONE");
>>> 	close(fd);
>>> }
>>> +
>>> +#define GOOD_COPY_LEN	128	/* net/vmw_vsock/virtio_transport_common.c */
>>
>> I think we don't need this, I mean we can eventually just send a single
>> byte, no?
>
>For a single byte sent, you get a single byte of uninitialized kernel
>memory. Uninitialized memory can by anything, in particular it can be
>(coincidentally) what you happen to expect. Which would result in a false
>positive. So instead of estimating what length sufficiently reduces
>probability of such false positive, I just took the upper bound.

I see, makes sense to me.

>
>BTW, I've realized recv_verify() is reinventing the wheel. How about
>dropping it in favour of what test_seqpacket_msg_bounds_client() does, i.e.
>calc the hash of payload and send it over the control channel for verification?

Yeah, strongly agree on that.

>
>>> +
>>> +void test_stream_msgzcopy_mangle_client(const struct test_opts *opts)
>>> +{
>>> +	char sbuf1[PAGE_SIZE + 1], sbuf2[GOOD_COPY_LEN];
>>> +	struct pollfd fds;
>>> +	int fd;
>>> +
>>> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>>> +	if (fd < 0) {
>>> +		perror("connect");
>>> +		exit(EXIT_FAILURE);
>>> +	}
>>> +
>>> +	enable_so_zerocopy_check(fd);
>>> +
>>> +	memset(sbuf1, '1', sizeof(sbuf1));
>>> +	memset(sbuf2, '2', sizeof(sbuf2));
>>> +
>>> +	send_buf(fd, sbuf1, sizeof(sbuf1), 0, sizeof(sbuf1));
>>> +	send_buf(fd, sbuf2, sizeof(sbuf2), MSG_ZEROCOPY, sizeof(sbuf2));
>>> +
>>> +	fds.fd = fd;
>>> +	fds.events = 0;
>>> +
>>> +	if (poll(&fds, 1, -1) != 1 || !(fds.revents & POLLERR)) {
>>> +		perror("poll");
>>> +		exit(EXIT_FAILURE);
>>> +	}
>>
>> Should we also call vsock_recv_completion() or we don't care about the
>> result?
>>
>> If we need it, maybe we can factor our the poll +
>> vsock_recv_completion().
>
>Nope, we don't care about the result.
>

Okay, I see.

Thanks,
Stefano

On 1/12/26 14:44, Stefano Garzarella wrote:
> On Sun, Jan 11, 2026 at 11:59:54AM +0100, Michal Luczaj wrote:
>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>> index bbe3723babdc..21c8616100f1 100644
>>>> --- a/tools/testing/vsock/vsock_test.c
>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>>>> 		.run_client = test_stream_accepted_setsockopt_client,
>>>> 		.run_server = test_stream_accepted_setsockopt_server,
>>>> 	},
>>>> +	{
>>>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
>>>
>>> This is essentially a regression test for virtio transport, so I'd add
>>> virtio in the test name.
>>
>> Isn't virtio transport unaffected? It's about loopback transport (that
>> shares common code with virtio transport).
> 
> Why virtio transport is not affected?

With the usual caveat that I may be completely missing something, aren't
all virtio-transport's rx skbs linear? See virtio_vsock_alloc_linear_skb()
in virtio_vsock_rx_fill().

On Mon, Jan 12, 2026 at 04:52:02PM +0100, Michal Luczaj wrote:
>On 1/12/26 14:44, Stefano Garzarella wrote:
>> On Sun, Jan 11, 2026 at 11:59:54AM +0100, Michal Luczaj wrote:
>>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>>> index bbe3723babdc..21c8616100f1 100644
>>>>> --- a/tools/testing/vsock/vsock_test.c
>>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>>>>> 		.run_client = test_stream_accepted_setsockopt_client,
>>>>> 		.run_server = test_stream_accepted_setsockopt_server,
>>>>> 	},
>>>>> +	{
>>>>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
>>>>
>>>> This is essentially a regression test for virtio transport, so I'd add
>>>> virtio in the test name.
>>>
>>> Isn't virtio transport unaffected? It's about loopback transport (that
>>> shares common code with virtio transport).
>>
>> Why virtio transport is not affected?
>
>With the usual caveat that I may be completely missing something, aren't
>all virtio-transport's rx skbs linear? See virtio_vsock_alloc_linear_skb()
>in virtio_vsock_rx_fill().
>

True, but what about drivers/vhost/vsock.c ?

IIUC in vhost_vsock_handle_tx_kick() we call vhost_vsock_alloc_skb(), 
that calls virtio_vsock_alloc_skb() and pass that skb to 
virtio_transport_recv_pkt(). So, it's also affected right?

BTW in general we consider loopback as one of virtio devices since it 
really shares with them most of the code.

That said, now I'm thinking more about Fixes tag.
Before commit 6693731487a8 ("vsock/virtio: Allocate nonlinear SKBs for 
handling large transmit buffers") was that a real issue?

Thanks,
Stefano

On 1/12/26 17:48, Stefano Garzarella wrote:
>>>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>>>> index bbe3723babdc..21c8616100f1 100644
>>>>>> --- a/tools/testing/vsock/vsock_test.c
>>>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>>>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>>>>>> 		.run_client = test_stream_accepted_setsockopt_client,
>>>>>> 		.run_server = test_stream_accepted_setsockopt_server,
>>>>>> 	},
>>>>>> +	{
>>>>>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
>>>>>
>>>>> This is essentially a regression test for virtio transport, so I'd add
>>>>> virtio in the test name.
>>>>
>>>> Isn't virtio transport unaffected? It's about loopback transport (that
>>>> shares common code with virtio transport).
>>>
>>> Why virtio transport is not affected?
>>
>> With the usual caveat that I may be completely missing something, aren't
>> all virtio-transport's rx skbs linear? See virtio_vsock_alloc_linear_skb()
>> in virtio_vsock_rx_fill().
>>
> 
> True, but what about drivers/vhost/vsock.c ?
> 
> IIUC in vhost_vsock_handle_tx_kick() we call vhost_vsock_alloc_skb(), 
> that calls virtio_vsock_alloc_skb() and pass that skb to 
> virtio_transport_recv_pkt(). So, it's also affected right?

virtio_vsock_alloc_skb() returns a non-linear skb only if size >
SKB_WITH_OVERHEAD(PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)). And that is way
more than GOOD_COPY_LEN, so we're good.

At least until someone increases GOOD_COPY_LEN and/or reduces the size
condition for non-linear allocation. So, yeah, a bit brittle.

> BTW in general we consider loopback as one of virtio devices since it 
> really shares with them most of the code.

Fair enough, I'll add "virtio" to the test name.

> That said, now I'm thinking more about Fixes tag.
> Before commit 6693731487a8 ("vsock/virtio: Allocate nonlinear SKBs for 
> handling large transmit buffers") was that a real issue?

I don't really think that commit changes anything for the zerocopy case. It
only makes some big (>GOOD_COPY_LEN) non-ZC skbs turn non-linear.

On Mon, Jan 12, 2026 at 10:20:50PM +0100, Michal Luczaj wrote:
>On 1/12/26 17:48, Stefano Garzarella wrote:
>>>>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>>>>> index bbe3723babdc..21c8616100f1 100644
>>>>>>> --- a/tools/testing/vsock/vsock_test.c
>>>>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>>>>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>>>>>>> 		.run_client = test_stream_accepted_setsockopt_client,
>>>>>>> 		.run_server = test_stream_accepted_setsockopt_server,
>>>>>>> 	},
>>>>>>> +	{
>>>>>>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
>>>>>>
>>>>>> This is essentially a regression test for virtio transport, so I'd add
>>>>>> virtio in the test name.
>>>>>
>>>>> Isn't virtio transport unaffected? It's about loopback transport (that
>>>>> shares common code with virtio transport).
>>>>
>>>> Why virtio transport is not affected?
>>>
>>> With the usual caveat that I may be completely missing something, aren't
>>> all virtio-transport's rx skbs linear? See virtio_vsock_alloc_linear_skb()
>>> in virtio_vsock_rx_fill().
>>>
>>
>> True, but what about drivers/vhost/vsock.c ?
>>
>> IIUC in vhost_vsock_handle_tx_kick() we call vhost_vsock_alloc_skb(),
>> that calls virtio_vsock_alloc_skb() and pass that skb to
>> virtio_transport_recv_pkt(). So, it's also affected right?
>
>virtio_vsock_alloc_skb() returns a non-linear skb only if size >
>SKB_WITH_OVERHEAD(PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)). And that is way
>more than GOOD_COPY_LEN, so we're good.
>
>At least until someone increases GOOD_COPY_LEN and/or reduces the size
>condition for non-linear allocation. So, yeah, a bit brittle.

I see, thanks for clarify. So please add all of this conclusions in the 
patch 1 description to make it clear that only loopback is affected, so 
no guest/host attack is possible. (not really severe CVE)

>
>> BTW in general we consider loopback as one of virtio devices since it
>> really shares with them most of the code.
>
>Fair enough, I'll add "virtio" to the test name.

Thanks.

>
>> That said, now I'm thinking more about Fixes tag.
>> Before commit 6693731487a8 ("vsock/virtio: Allocate nonlinear SKBs for
>> handling large transmit buffers") was that a real issue?
>
>I don't really think that commit changes anything for the zerocopy case. It
>only makes some big (>GOOD_COPY_LEN) non-ZC skbs turn non-linear.
>

I see.

Stefano

On 1/13/26 10:36, Stefano Garzarella wrote:
> On Mon, Jan 12, 2026 at 10:20:50PM +0100, Michal Luczaj wrote:
>> On 1/12/26 17:48, Stefano Garzarella wrote:
>>>>>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>>>>>> index bbe3723babdc..21c8616100f1 100644
>>>>>>>> --- a/tools/testing/vsock/vsock_test.c
>>>>>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>>>>>> @@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
>>>>>>>> 		.run_client = test_stream_accepted_setsockopt_client,
>>>>>>>> 		.run_server = test_stream_accepted_setsockopt_server,
>>>>>>>> 	},
>>>>>>>> +	{
>>>>>>>> +		.name = "SOCK_STREAM MSG_ZEROCOPY coalescence corruption",
>>>>>>>
>>>>>>> This is essentially a regression test for virtio transport, so I'd add
>>>>>>> virtio in the test name.
>>>>>>
>>>>>> Isn't virtio transport unaffected? It's about loopback transport (that
>>>>>> shares common code with virtio transport).
>>>>>
>>>>> Why virtio transport is not affected?
>>>>
>>>> With the usual caveat that I may be completely missing something, aren't
>>>> all virtio-transport's rx skbs linear? See virtio_vsock_alloc_linear_skb()
>>>> in virtio_vsock_rx_fill().
>>>>
>>>
>>> True, but what about drivers/vhost/vsock.c ?
>>>
>>> IIUC in vhost_vsock_handle_tx_kick() we call vhost_vsock_alloc_skb(),
>>> that calls virtio_vsock_alloc_skb() and pass that skb to
>>> virtio_transport_recv_pkt(). So, it's also affected right?
>>
>> virtio_vsock_alloc_skb() returns a non-linear skb only if size >
>> SKB_WITH_OVERHEAD(PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)). And that is way
>> more than GOOD_COPY_LEN, so we're good.
>>
>> At least until someone increases GOOD_COPY_LEN and/or reduces the size
>> condition for non-linear allocation. So, yeah, a bit brittle.
> 
> I see, thanks for clarify. So please add all of this conclusions in the 
> patch 1 description to make it clear that only loopback is affected, so 
> no guest/host attack is possible. (not really severe CVE)

OK, here's v2:
https://lore.kernel.org/netdev/20260113-vsock-recv-coalescence-v2-0-552b17837cf4@rbox.co/