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 <[email protected]> >> --- >> 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.
