linux-kernel - Re: [PATCH net-next v2 3/3] test/vsock: Add ioctl SIOCINQ tests

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

Message-ID: <CAGxU2F6FtJT1fyZQ9JTExYF6OsJ6J8HRiOetX09JB6rTzVtVcQ@mail.gmail.com>
Date: Mon, 16 Jun 2025 16:51:34 +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, 
	netdev@...r.kernel.org, niuxuewei.nxw@...group.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 Mon, 16 Jun 2025 at 16:39, Xuewei Niu <niuxuewei97@...il.com> wrote:
>
> > 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.

But we are not using anything in util.c from sys/ioctl.h IIUC, in that
case please move it in util.c where actually is needed.

Thanks,
Stefano