| 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
| ||
|
Message-ID: <2oxz7toygswngn7kfmsrbmpikk5qggwbvk3oxb5ucrbq3pcfff@azc54m4hwlfb>
Date: Mon, 16 Jun 2025 15:59:01 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Xuewei Niu <niuxuewei97@...il.com>
Cc: mst@...hat.com, pabeni@...hat.com, jasowang@...hat.com,
xuanzhuo@...ux.alibaba.com, davem@...emloft.net, netdev@...r.kernel.org, stefanha@...hat.com,
virtualization@...ts.linux.dev, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
fupan.lfp@...group.com, Xuewei Niu <niuxuewei.nxw@...group.com>
Subject: Re: [PATCH net-next v2 3/3] test/vsock: Add ioctl SIOCINQ tests
On Fri, Jun 13, 2025 at 11:11:52AM +0800, Xuewei Niu wrote:
>This patch adds SIOCINQ ioctl tests for both SOCK_STREAM and
>SOCK_SEQPACKET.
>
>The client waits for the server to send data, and checks if the SIOCINQ
>ioctl value matches the data size. After consuming the data, the client
>checks if the SIOCINQ value is 0.
>
>Signed-off-by: Xuewei Niu <niuxuewei.nxw@...group.com>
>---
> tools/testing/vsock/util.c | 36 ++++++++++----
> tools/testing/vsock/util.h | 2 +
> tools/testing/vsock/vsock_test.c | 83 +++++++++++++++++++++++++++++++-
> 3 files changed, 111 insertions(+), 10 deletions(-)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index 0c7e9cbcbc85..472246198966 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -97,28 +97,46 @@ void vsock_wait_remote_close(int fd)
> close(epollfd);
> }
>
>-/* Wait until transport reports no data left to be sent.
>- * Return false if transport does not implement the unsent_bytes() callback.
>+/* Wait until ioctl gives an expected int value.
>+ * Return a negative value if the op is not supported.
> */
>-bool vsock_wait_sent(int fd)
>+int ioctl_int(int fd, unsigned long op, int *actual, int expected)
> {
>- int ret, sock_bytes_unsent;
>+ int ret;
>+ char name[32];
>+
>+ if (!actual) {
>+ fprintf(stderr, "%s requires a non-null pointer\n", __func__);
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ snprintf(name, sizeof(name), "ioctl(%lu)", op);
>
> timeout_begin(TIMEOUT);
> do {
>- ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
>+ ret = ioctl(fd, op, actual);
> if (ret < 0) {
> if (errno == EOPNOTSUPP)
> break;
>
>- perror("ioctl(SIOCOUTQ)");
>+ perror(name);
> exit(EXIT_FAILURE);
> }
>- timeout_check("SIOCOUTQ");
>- } while (sock_bytes_unsent != 0);
>+ timeout_check(name);
>+ } while (*actual != expected);
> timeout_end();
>
>- return !ret;
>+ return ret;
>+}
>+
>+/* Wait until transport reports no data left to be sent.
>+ * Return false if transport does not implement the unsent_bytes() callback.
>+ */
>+bool vsock_wait_sent(int fd)
>+{
>+ int sock_bytes_unsent;
>+
>+ return !(ioctl_int(fd, SIOCOUTQ, &sock_bytes_unsent, 0));
> }
Please split this patch in 2, one where you do the refactoring in
util.c/h and one for the new test.
>
> /* Create socket <type>, bind to <cid, port> and return the file descriptor. */
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index 5e2db67072d5..945c85ff8d22 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -3,6 +3,7 @@
> #define UTIL_H
>
> #include <sys/socket.h>
>+#include <sys/ioctl.h>
Why we need this in util.h?
> #include <linux/vm_sockets.h>
>
> /* Tests can either run as the client or the server */
>@@ -54,6 +55,7 @@ int vsock_stream_listen(unsigned int cid, unsigned int port);
> int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
> struct sockaddr_vm *clientaddrp);
> void vsock_wait_remote_close(int fd);
>+int ioctl_int(int fd, unsigned long op, int *actual, int expected);
> bool vsock_wait_sent(int fd);
> void send_buf(int fd, const void *buf, size_t len, int flags,
> ssize_t expected_ret);
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index f669baaa0dca..43996447f9a2 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -20,7 +20,6 @@
> #include <sys/mman.h>
> #include <poll.h>
> #include <signal.h>
>-#include <sys/ioctl.h>
> #include <linux/time64.h>
>
> #include "vsock_test_zerocopy.h"
>@@ -1305,6 +1304,58 @@ 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 has arrived but has not been read. The expected is
>+ * MSG_BUF_IOCTL_LEN.
>+ */
>+ ret = ioctl_int(fd, TIOCINQ, &sock_bytes_unread, MSG_BUF_IOCTL_LEN);
>+ if (ret) {
>+ fprintf(stderr, "Test skipped, TIOCINQ not supported.\n");
>+ goto out;
>+ }
>+
>+ recv_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>+ // All date has been consumed, so the expected is 0.
>+ ioctl_int(fd, TIOCINQ, &sock_bytes_unread, 0);
>+ 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);
>@@ -1325,6 +1376,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
>@@ -2016,6 +2087,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,
>+ },
I think I already mentioned in the previous version: please add new
tests at the end of the array, so we can preserve test IDs.
Thanks,
Stefano
> {
> .name = "SOCK_STREAM leak accept queue",
> .run_client = test_stream_leak_acceptq_client,
>--
>2.34.1
>
Powered by blists - more mailing lists