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: <k6vzt5brosmomqzb7ibripcxh4pauz4juwqvhv77sv7vbywglq@m4bpb3hpoc3n>
Date: Wed, 21 May 2025 11:00:50 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Xuewei Niu <niuxuewei97@...il.com>
Cc: davem@...emloft.net, fupan.lfp@...group.com, jasowang@...hat.com, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, mst@...hat.com, 
	niuxuewei.nxw@...group.com, pabeni@...hat.com, stefanha@...hat.com, 
	virtualization@...ts.linux.dev, xuanzhuo@...ux.alibaba.com
Subject: Re: [PATCH 3/3] test/vsock: Add ioctl SIOCINQ tests

On Wed, May 21, 2025 at 10:15:47AM +0800, Xuewei Niu wrote:
>> On Mon, May 19, 2025 at 03:06:49PM +0800, Xuewei Niu wrote:
>> >This patch adds two tests for ioctl SIOCINQ for SOCK_STREAM and
>> >SOCK_SEQPACKET. The client waits for the server to send data, and checks if
>> >the return value of the SIOCINQ is the size of the data. Then, consumes the
>> >data and checks if the value is 0.
>>
>> We recently fixed the SIOCOUTQ test, see:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7fd7ad6f36af36f30a06d165eff3780cb139fa79
>>
>> Should we do the same here?
>
>Yeah! Indeed, we have recognized this issue before. I think it is better to
>wrap this ioctl operation in a function in "tools/testing/vsock/util.c" and
>make it reusable.

Yep, if that helps to reduce code duplication, it's fine.

Thanks,
Stefano

>
>Will do.
>
>> >
>> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@...group.com>
>> >---
>> > tools/testing/vsock/vsock_test.c | 102 +++++++++++++++++++++++++++++++
>> > 1 file changed, 102 insertions(+)
>> >
>> >diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> >index d0f6d253ac72..8b3fb88e2877 100644
>> >--- a/tools/testing/vsock/vsock_test.c
>> >+++ b/tools/testing/vsock/vsock_test.c
>> >@@ -1282,6 +1282,78 @@ static void test_unsent_bytes_client(const struct test_opts *opts, int type)
>> > 	close(fd);
>> > }
>> >
>> >+static void test_unread_bytes_server(const struct test_opts *opts, int type)
>> >+{
>> >+	unsigned char buf[MSG_BUF_IOCTL_LEN];
>> >+	int client_fd;
>> >+
>> >+	client_fd = vsock_accept(VMADDR_CID_ANY, opts->peer_port, NULL, type);
>> >+	if (client_fd < 0) {
>> >+		perror("accept");
>> >+		exit(EXIT_FAILURE);
>> >+	}
>> >+
>> >+	for (int i = 0; i < sizeof(buf); i++)
>> >+		buf[i] = rand() & 0xFF;
>> >+
>> >+	send_buf(client_fd, buf, sizeof(buf), 0, sizeof(buf));
>> >+	control_writeln("SENT");
>> >+	control_expectln("RECEIVED");
>> >+
>> >+	close(client_fd);
>> >+}
>> >+
>> >+static void test_unread_bytes_client(const struct test_opts *opts, int type)
>> >+{
>> >+	unsigned char buf[MSG_BUF_IOCTL_LEN];
>> >+	int ret, fd;
>> >+	int sock_bytes_unread;
>> >+
>> >+	fd = vsock_connect(opts->peer_cid, opts->peer_port, type);
>> >+	if (fd < 0) {
>> >+		perror("connect");
>> >+		exit(EXIT_FAILURE);
>> >+	}
>> >+
>> >+	control_expectln("SENT");
>> >+	// The data have come in but is not read, the expected value is
>> >+	// MSG_BUF_IOCTL_LEN.
>> >+	ret = ioctl(fd, SIOCINQ, &sock_bytes_unread);
>> >+	if (ret < 0) {
>> >+		if (errno == EOPNOTSUPP) {
>> >+			fprintf(stderr,
>> >+				"Test skipped, SIOCINQ not supported.\n");
>> >+			goto out;
>> >+		} else {
>> >+			perror("ioctl");
>> >+			exit(EXIT_FAILURE);
>> >+		}
>> >+	} else if (ret == 0 && sock_bytes_unread != MSG_BUF_IOCTL_LEN) {
>> >+		fprintf(stderr,
>> >+			"Unexpected 'SIOCOUTQ' value, expected %d, got %i\n",
>> >+			MSG_BUF_IOCTL_LEN, sock_bytes_unread);
>> >+		exit(EXIT_FAILURE);
>> >+	}
>> >+
>> >+	recv_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>> >+	// The data is consumed, so the expected is 0.
>> >+	ret = ioctl(fd, SIOCINQ, &sock_bytes_unread);
>> >+	if (ret < 0) {
>> >+		// Don't ignore EOPNOTSUPP since we have already checked it!
>> >+		perror("ioctl");
>> >+		exit(EXIT_FAILURE);
>> >+	} else if (ret == 0 && sock_bytes_unread != 0) {
>> >+		fprintf(stderr,
>> >+			"Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
>> >+			sock_bytes_unread);
>> >+		exit(EXIT_FAILURE);
>> >+	}
>> >+	control_writeln("RECEIVED");
>> >+
>> >+out:
>> >+	close(fd);
>> >+}
>> >+
>> > static void test_stream_unsent_bytes_client(const struct test_opts *opts)
>> > {
>> > 	test_unsent_bytes_client(opts, SOCK_STREAM);
>> >@@ -1302,6 +1374,26 @@ static void test_seqpacket_unsent_bytes_server(const struct test_opts *opts)
>> > 	test_unsent_bytes_server(opts, SOCK_SEQPACKET);
>> > }
>> >
>> >+static void test_stream_unread_bytes_client(const struct test_opts *opts)
>> >+{
>> >+	test_unread_bytes_client(opts, SOCK_STREAM);
>> >+}
>> >+
>> >+static void test_stream_unread_bytes_server(const struct test_opts *opts)
>> >+{
>> >+	test_unread_bytes_server(opts, SOCK_STREAM);
>> >+}
>> >+
>> >+static void test_seqpacket_unread_bytes_client(const struct test_opts *opts)
>> >+{
>> >+	test_unread_bytes_client(opts, SOCK_SEQPACKET);
>> >+}
>> >+
>> >+static void test_seqpacket_unread_bytes_server(const struct test_opts *opts)
>> >+{
>> >+	test_unread_bytes_server(opts, SOCK_SEQPACKET);
>> >+}
>> >+
>> > #define RCVLOWAT_CREDIT_UPD_BUF_SIZE	(1024 * 128)
>> > /* This define is the same as in 'include/linux/virtio_vsock.h':
>> >  * it is used to decide when to send credit update message during
>> >@@ -1954,6 +2046,16 @@ static struct test_case test_cases[] = {
>> > 		.run_client = test_seqpacket_unsent_bytes_client,
>> > 		.run_server = test_seqpacket_unsent_bytes_server,
>> > 	},
>> >+	{
>> >+		.name = "SOCK_STREAM ioctl(SIOCINQ) functionality",
>> >+		.run_client = test_stream_unread_bytes_client,
>> >+		.run_server = test_stream_unread_bytes_server,
>> >+	},
>> >+	{
>> >+		.name = "SOCK_SEQPACKET ioctl(SIOCINQ) functionality",
>> >+		.run_client = test_seqpacket_unread_bytes_client,
>> >+		.run_server = test_seqpacket_unread_bytes_server,
>> >+	},
>>
>> Please, append new test at the end, so we will not change test IDs.
>>
>> Thanks,
>> Stefano
>
>Will do.
>
>Thanks,
>Xuewei
>
>> > 	{
>> > 		.name = "SOCK_STREAM leak accept queue",
>> > 		.run_client = test_stream_leak_acceptq_client,
>> >--
>> >2.34.1
>> >
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ