netdev - Re: [PATCH net-next 4/4] vsock/test: Add test for MSG

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <oipmjpvmvbksopq6ugfmad2bd6k6mkj34q3jef5fvz72f3xfow@ve7lrp5gx37c>
Date: Tue, 10 Dec 2024 16:36:23 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Michal Luczaj <mhal@...x.co>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next 4/4] vsock/test: Add test for MSG_ZEROCOPY
 completion memory leak

On Fri, Dec 06, 2024 at 07:34:54PM +0100, Michal Luczaj wrote:
>Exercise the ENOMEM error path by attempting to hit net.core.optmem_max
>limit on send().
>
>Fixed by commit 60cf6206a1f5 ("virtio/vsock: Improve MSG_ZEROCOPY error
>handling").
>
>Signed-off-by: Michal Luczaj <mhal@...x.co>
>---
> tools/testing/vsock/vsock_test.c | 66 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index f92c62b25a25d35ae63a77a0122a194051719169..6973e681490b363e3b9cedcf195844ba56da6f1d 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1552,6 +1552,67 @@ static void test_stream_msgzcopy_leak_errq_server(const struct test_opts *opts)
> 	close(fd);
> }
>
>+static void test_stream_msgzcopy_leak_zcskb_client(const struct test_opts *opts)
>+{
>+	char buf[1024] = { 0 };
>+	ssize_t optmem_max;
>+	int fd, res;
>+	FILE *f;
>+
>+	f = fopen("/proc/sys/net/core/optmem_max", "r");
>+	if (!f) {
>+		perror("fopen(optmem_max)");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (fscanf(f, "%zd", &optmem_max) != 1 || optmem_max > ~0U / 2) {
>+		fprintf(stderr, "fscanf(optmem_max) failed\n");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	fclose(f);
>+
>+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	enable_so_zerocopy_check(fd);
>+
>+	/* The idea is to fail virtio_transport_init_zcopy_skb() by hitting
>+	 * core.sysctl_optmem_max (sysctl net.core.optmem_max) limit check in
>+	 * sock_omalloc().
>+	 */
>+	optmem_max *= 2;
>+	errno = 0;
>+	do {
>+		res = send(fd, buf, sizeof(buf), MSG_ZEROCOPY);
>+		optmem_max -= res;
>+	} while (res > 0 && optmem_max > 0);
>+
>+	if (errno != ENOMEM) {
>+		fprintf(stderr, "expected ENOMEM on send()\n");

This test is failing in my suite with this message (I added errno
in the message that maybe we can add to understand better what we
expect, and what we saw):

28 - SOCK_STREAM MSG_ZEROCOPY leak completion skb...expected ENOMEM on send() - errno 0

my env:

host (L0) -> vsockvm0 (L1) -> vsockvm1 (L2)

L2 is a nested guest run by L1.
L1 and L2 runs v6.13-rc2 plus some commits
(7cb1b466315004af98f6ba6c2546bb713ca3c237)

vsockvm0$ cat /proc/sys/net/core/optmem_max
81920

vsockvm1$ cat /proc/sys/net/core/optmem_max
81920

If the server is running on vsockvm0 (host for the test POV, using
vhost-vsock transport), the test passes, but if the server is running on
vsockvm1 (guest, virtio-vsock transport) the test fails:

vsockvm1$ vsock_test --mode=server --control-port=12345 --peer-cid=2

vsockvm0$ vsock_test --mode=client --control-host=192.168.133.3 \
     --control-port=12345 --peer-cid=4
...
26 - SOCK_STREAM leak accept queue...ok
27 - SOCK_STREAM MSG_ZEROCOPY leak MSG_ERRQUEUE...ok
28 - SOCK_STREAM MSG_ZEROCOPY leak completion skb...expected ENOMEM on send() - errno 0

Maybe because virtio_transport_init_zcopy_skb() in the vhost-vsock case
is called in the context of vhost kernel task created by
vhost_task_create()?

Thanks,
Stefano

>+		exit(EXIT_FAILURE);
>+	}
>+
>+	close(fd);
>+}
>+
>+static void test_stream_msgzcopy_leak_zcskb_server(const struct test_opts *opts)
>+{
>+	int fd;
>+
>+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>+	if (fd < 0) {
>+		perror("accept");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	vsock_wait_remote_close(fd);
>+	close(fd);
>+}
>+
> static struct test_case test_cases[] = {
> 	{
> 		.name = "SOCK_STREAM connection reset",
>@@ -1692,6 +1753,11 @@ static struct test_case test_cases[] = {
> 		.run_client = test_stream_msgzcopy_leak_errq_client,
> 		.run_server = test_stream_msgzcopy_leak_errq_server,
> 	},
>+	{
>+		.name = "SOCK_STREAM MSG_ZEROCOPY leak completion skb",
>+		.run_client = test_stream_msgzcopy_leak_zcskb_client,
>+		.run_server = test_stream_msgzcopy_leak_zcskb_server,
>+	},
> 	{},
> };
>
>
>-- 
>2.47.1
>