linux-kernel - Re: [PATCH 2/2] vsock/test: Add test for a linear and non-linear skb getting coalesced

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <76ca0c9f-dcda-4a53-ac1f-c5c28d1ecf44@rbox.co>
Date: Sun, 11 Jan 2026 11:59:54 +0100
From: Michal Luczaj <mhal@...x.co>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, Stefan Hajnoczi <stefanha@...hat.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>,
 Arseniy Krasnov <avkrasnov@...utedevices.com>, kvm@...r.kernel.org,
 virtualization@...ts.linux.dev, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] vsock/test: Add test for a linear and non-linear skb
 getting coalesced

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@...x.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.