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: <ppjcnfwpedahsx4id3r2ctnj2hfi2alh4ctvsptk23ujzoz5xc@elm6l5hpu55d>
Date: Mon, 16 Dec 2024 15:37:55 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Michal Luczaj <mhal@...x.co>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 6/6] vsock/test: Add test for MSG_ZEROCOPY
 completion memory leak

On Mon, Dec 16, 2024 at 01:01:02PM +0100, Michal Luczaj wrote:
>Exercise the ENOMEM error path by attempting to hit net.core.optmem_max
>limit on send().
>
>Test aims to create a memory leak, kmemleak should be employed.
>
>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 | 152 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 152 insertions(+)

Reviewed-by: Stefano Garzarella <sgarzare@...hat.com>

>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index d2970198967e3a2d02ac461921b946e3b0498837..8f145b6db77d904f3f888ec8fbe76298e7dd91de 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1560,6 +1560,153 @@ static void test_stream_msgzcopy_leak_errq_server(const struct test_opts *opts)
> 	close(fd);
> }
>
>+/* Test msgzcopy_leak_zcskb is meant to exercise sendmsg() error handling path,
>+ * that might leak an skb. The idea is to fail virtio_transport_init_zcopy_skb()
>+ * by hitting net.core.optmem_max limit in sock_omalloc(), specifically
>+ *
>+ *   vsock_connectible_sendmsg
>+ *     virtio_transport_stream_enqueue
>+ *       virtio_transport_send_pkt_info
>+ *         virtio_transport_init_zcopy_skb
>+ *         . msg_zerocopy_realloc
>+ *         .   msg_zerocopy_alloc
>+ *         .     sock_omalloc
>+ *         .       sk_omem_alloc + size > sysctl_optmem_max
>+ *         return -ENOMEM
>+ *
>+ * We abuse the implementation detail of net/socket.c:____sys_sendmsg().
>+ * sk_omem_alloc can be precisely bumped by sock_kmalloc(), as it is used to
>+ * fetch user-provided control data.
>+ *
>+ * While this approach works for now, it relies on assumptions regarding the
>+ * implementation and configuration (for example, order of net.core.optmem_max
>+ * can not exceed MAX_PAGE_ORDER), which may not hold in the future. A more
>+ * resilient testing could be implemented by leveraging the Fault injection
>+ * framework (CONFIG_FAULT_INJECTION), e.g.
>+ *
>+ *   client# echo N > /sys/kernel/debug/failslab/ignore-gfp-wait
>+ *   client# echo 0 > /sys/kernel/debug/failslab/verbose
>+ *
>+ *   void client(const struct test_opts *opts)
>+ *   {
>+ *       char buf[16];
>+ *       int f, s, i;
>+ *
>+ *       f = open("/proc/self/fail-nth", O_WRONLY);
>+ *
>+ *       for (i = 1; i < 32; i++) {
>+ *           control_writeulong(CONTINUE);
>+ *
>+ *           s = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>+ *           enable_so_zerocopy_check(s);
>+ *
>+ *           sprintf(buf, "%d", i);
>+ *           write(f, buf, strlen(buf));
>+ *
>+ *           send(s, &(char){ 0 }, 1, MSG_ZEROCOPY);
>+ *
>+ *           write(f, "0", 1);
>+ *           close(s);
>+ *       }
>+ *
>+ *       control_writeulong(DONE);
>+ *       close(f);
>+ *   }
>+ *
>+ *   void server(const struct test_opts *opts)
>+ *   {
>+ *       int fd;
>+ *
>+ *       while (control_readulong() == CONTINUE) {
>+ *           fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>+ *           vsock_wait_remote_close(fd);
>+ *           close(fd);
>+ *       }
>+ *   }
>+ *
>+ * Refer to Documentation/fault-injection/fault-injection.rst.
>+ */
>+#define MAX_PAGE_ORDER	10	/* usually */
>+#define PAGE_SIZE	4096
>+
>+/* Test for a memory leak. User is expected to run kmemleak scan, see README. */
>+static void test_stream_msgzcopy_leak_zcskb_client(const struct test_opts *opts)
>+{
>+	size_t optmem_max, ctl_len, chunk_size;
>+	struct msghdr msg = { 0 };
>+	struct iovec iov;
>+	char *chunk;
>+	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, "%zu", &optmem_max) != 1) {
>+		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);
>+
>+	ctl_len = optmem_max - 1;
>+	if (ctl_len > PAGE_SIZE << MAX_PAGE_ORDER) {
>+		fprintf(stderr, "Try with net.core.optmem_max = 100000\n");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	chunk_size = CMSG_SPACE(ctl_len);
>+	chunk = malloc(chunk_size);
>+	if (!chunk) {
>+		perror("malloc");
>+		exit(EXIT_FAILURE);
>+	}
>+	memset(chunk, 0, chunk_size);
>+
>+	iov.iov_base = &(char){ 0 };
>+	iov.iov_len = 1;
>+
>+	msg.msg_iov = &iov;
>+	msg.msg_iovlen = 1;
>+	msg.msg_control = chunk;
>+	msg.msg_controllen = ctl_len;
>+
>+	errno = 0;
>+	res = sendmsg(fd, &msg, MSG_ZEROCOPY);
>+	if (res >= 0 || errno != ENOMEM) {
>+		fprintf(stderr, "Expected ENOMEM, got errno=%d res=%d\n",
>+			errno, res);
>+		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",
>@@ -1700,6 +1847,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
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ