Skip to content

Commit 9ad6930

Browse files
committed
Merge branch 'vsock-virtio-fix-data-loss-disclosure-due-to-joining-of-non-linear-skb'
Michal Luczaj says: ==================== vsock/virtio: Fix data loss/disclosure due to joining of non-linear skb Loopback transport coalesces some skbs too eagerly. Handling a zerocopy (non-linear) skb as a linear one leads to skb data loss and kernel memory disclosure. Plug the loss/leak by allowing only linear skb join. Provide a test. ==================== Link: https://patch.msgid.link/20260113-vsock-recv-coalescence-v2-0-552b17837cf4@rbox.co Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents c4efd7a + a63e5fe commit 9ad6930

4 files changed

Lines changed: 86 additions & 2 deletions

File tree

net/vmw_vsock/virtio_transport_common.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,9 +1359,11 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
13591359

13601360
/* Try to copy small packets into the buffer of last packet queued,
13611361
* to avoid wasting memory queueing the entire buffer with a small
1362-
* payload.
1362+
* payload. Skip non-linear (e.g. zerocopy) skbs; these carry payload
1363+
* in skb_shinfo.
13631364
*/
1364-
if (len <= GOOD_COPY_LEN && !skb_queue_empty(&vvs->rx_queue)) {
1365+
if (len <= GOOD_COPY_LEN && !skb_queue_empty(&vvs->rx_queue) &&
1366+
!skb_is_nonlinear(skb)) {
13651367
struct virtio_vsock_hdr *last_hdr;
13661368
struct sk_buff *last_skb;
13671369

tools/testing/vsock/vsock_test.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2403,6 +2403,11 @@ static struct test_case test_cases[] = {
24032403
.run_client = test_stream_accepted_setsockopt_client,
24042404
.run_server = test_stream_accepted_setsockopt_server,
24052405
},
2406+
{
2407+
.name = "SOCK_STREAM virtio MSG_ZEROCOPY coalescence corruption",
2408+
.run_client = test_stream_msgzcopy_mangle_client,
2409+
.run_server = test_stream_msgzcopy_mangle_server,
2410+
},
24062411
{},
24072412
};
24082413

tools/testing/vsock/vsock_test_zerocopy.c

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,18 @@
99
#include <stdio.h>
1010
#include <stdlib.h>
1111
#include <string.h>
12+
#include <sys/ioctl.h>
1213
#include <sys/mman.h>
1314
#include <unistd.h>
1415
#include <poll.h>
1516
#include <linux/errqueue.h>
1617
#include <linux/kernel.h>
18+
#include <linux/sockios.h>
19+
#include <linux/time64.h>
1720
#include <errno.h>
1821

1922
#include "control.h"
23+
#include "timeout.h"
2024
#include "vsock_test_zerocopy.h"
2125
#include "msg_zerocopy_common.h"
2226

@@ -356,3 +360,73 @@ void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts)
356360
control_expectln("DONE");
357361
close(fd);
358362
}
363+
364+
#define GOOD_COPY_LEN 128 /* net/vmw_vsock/virtio_transport_common.c */
365+
366+
void test_stream_msgzcopy_mangle_client(const struct test_opts *opts)
367+
{
368+
char sbuf1[PAGE_SIZE + 1], sbuf2[GOOD_COPY_LEN];
369+
unsigned long hash;
370+
struct pollfd fds;
371+
int fd, i;
372+
373+
fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
374+
if (fd < 0) {
375+
perror("connect");
376+
exit(EXIT_FAILURE);
377+
}
378+
379+
enable_so_zerocopy_check(fd);
380+
381+
memset(sbuf1, 'x', sizeof(sbuf1));
382+
send_buf(fd, sbuf1, sizeof(sbuf1), 0, sizeof(sbuf1));
383+
384+
for (i = 0; i < sizeof(sbuf2); i++)
385+
sbuf2[i] = rand() & 0xff;
386+
387+
send_buf(fd, sbuf2, sizeof(sbuf2), MSG_ZEROCOPY, sizeof(sbuf2));
388+
389+
hash = hash_djb2(sbuf2, sizeof(sbuf2));
390+
control_writeulong(hash);
391+
392+
fds.fd = fd;
393+
fds.events = 0;
394+
395+
if (poll(&fds, 1, TIMEOUT * MSEC_PER_SEC) != 1 ||
396+
!(fds.revents & POLLERR)) {
397+
perror("poll");
398+
exit(EXIT_FAILURE);
399+
}
400+
401+
close(fd);
402+
}
403+
404+
void test_stream_msgzcopy_mangle_server(const struct test_opts *opts)
405+
{
406+
unsigned long local_hash, remote_hash;
407+
char rbuf[PAGE_SIZE + 1];
408+
int fd;
409+
410+
fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
411+
if (fd < 0) {
412+
perror("accept");
413+
exit(EXIT_FAILURE);
414+
}
415+
416+
/* Wait, don't race the (buggy) skbs coalescence. */
417+
vsock_ioctl_int(fd, SIOCINQ, PAGE_SIZE + 1 + GOOD_COPY_LEN);
418+
419+
/* Discard the first packet. */
420+
recv_buf(fd, rbuf, PAGE_SIZE + 1, 0, PAGE_SIZE + 1);
421+
422+
recv_buf(fd, rbuf, GOOD_COPY_LEN, 0, GOOD_COPY_LEN);
423+
remote_hash = control_readulong();
424+
local_hash = hash_djb2(rbuf, GOOD_COPY_LEN);
425+
426+
if (local_hash != remote_hash) {
427+
fprintf(stderr, "Data received corrupted\n");
428+
exit(EXIT_FAILURE);
429+
}
430+
431+
close(fd);
432+
}

tools/testing/vsock/vsock_test_zerocopy.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,7 @@ void test_seqpacket_msgzcopy_server(const struct test_opts *opts);
1212
void test_stream_msgzcopy_empty_errq_client(const struct test_opts *opts);
1313
void test_stream_msgzcopy_empty_errq_server(const struct test_opts *opts);
1414

15+
void test_stream_msgzcopy_mangle_client(const struct test_opts *opts);
16+
void test_stream_msgzcopy_mangle_server(const struct test_opts *opts);
17+
1518
#endif /* VSOCK_TEST_ZEROCOPY_H */

0 commit comments

Comments
 (0)