| 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: <20250616143901.1187273-1-niuxuewei.nxw@antgroup.com>
Date: Mon, 16 Jun 2025 22:39:01 +0800
From: Xuewei Niu <niuxuewei97@...il.com>
To: sgarzare@...hat.com
Cc: davem@...emloft.net,
fupan.lfp@...group.com,
jasowang@...hat.com,
kvm@...r.kernel.org,
linux-kernel@...r.kernel.org,
mst@...hat.com,
netdev@...r.kernel.org,
niuxuewei.nxw@...group.com,
niuxuewei97@...il.com,
pabeni@...hat.com,
stefanha@...hat.com,
virtualization@...ts.linux.dev,
xuanzhuo@...ux.alibaba.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.
Will do.
> > /* 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?
We call `ioctl()` in the util.c. And we use `TIOCINQ` in the vsock_test.c,
where includes "util.h". So including `sys/ioctl.h` in util.h is needed.
> > #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.
My bad. Sorry. Will do.
Thanks,
Xuewei
> Thanks,
> Stefano
>
> > {
> > .name = "SOCK_STREAM leak accept queue",
> > .run_client = test_stream_leak_acceptq_client,
> >--
> >2.34.1
> >
Powered by blists - more mailing lists