[PATCH net-next 5/5] selftests: net: getsockopt_iter: address review nits

[Breno Leitao <leitao@debian.org>]

Apply two cleanups suggested by Stanislav Fomichev on the original
selftest series:

- Reorder local variable declarations into reverse christmas-tree
  order (longest line first). Because that ordering puts socklen_t
  optlen before the variable whose size it stores, the
  "optlen = sizeof(...)" initializer is moved out of the declaration
  to a plain assignment in the test body, as Stanislav suggested.

- Add ASSERT_EQ(optlen, ...) on every error path so the value the
  kernel writes back to the userspace optlen is pinned down even
  when the syscall returns -1. With do_sock_getsockopt() now writing
  opt->optlen back to userspace unconditionally, asserting that the
  netlink/vsock error paths leave the original input length untouched
  guards against future regressions.

Bobby Eshleman pointed out that
SO_VM_SOCKETS_CONNECT_TIMEOUT_NEW/OLD return a sock_timeval-shaped
payload (16 bytes on 64-bit), which is wider than the u64 case
already covered. Add four tests that exercise this path:

- connect_timeout_new_exact         exact-size buffer
- connect_timeout_new_oversize_clamped  oversize buffer, clamped
- connect_timeout_new_undersize     undersize -> -EINVAL, optlen
                                    untouched
- connect_timeout_old_exact         exact-size buffer for OLD optname

Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
Suggested-by: Bobby Eshleman <bobbyeshleman@meta.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/net/getsockopt_iter.c | 109 +++++++++++++++++++++++---
 1 file changed, 98 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/net/getsockopt_iter.c b/tools/testing/selftests/net/getsockopt_iter.c
index 179f9e84926fd..209569354d0e3 100644
--- a/tools/testing/selftests/net/getsockopt_iter.c
+++ b/tools/testing/selftests/net/getsockopt_iter.c
@@ -22,6 +22,7 @@
 #include <unistd.h>
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
+#include <linux/time_types.h>
 #include <linux/vm_sockets.h>
 #include <sys/socket.h>
 #include "kselftest_harness.h"
@@ -61,8 +62,10 @@ FIXTURE_TEARDOWN(netlink)
 
 TEST_F(netlink, pktinfo_exact)
 {
+	socklen_t optlen;
 	int val = -1;
-	socklen_t optlen = sizeof(val);
+
+	optlen = sizeof(val);
 
 	ASSERT_EQ(0, getsockopt(self->fd, SOL_NETLINK, NETLINK_PKTINFO,
 				&val, &optlen));
@@ -73,7 +76,9 @@ TEST_F(netlink, pktinfo_exact)
 TEST_F(netlink, pktinfo_oversize_clamped)
 {
 	char buf[16] = {};
-	socklen_t optlen = sizeof(buf);
+	socklen_t optlen;
+
+	optlen = sizeof(buf);
 
 	ASSERT_EQ(0, getsockopt(self->fd, SOL_NETLINK, NETLINK_PKTINFO,
 				buf, &optlen));
@@ -83,11 +88,14 @@ TEST_F(netlink, pktinfo_oversize_clamped)
 TEST_F(netlink, pktinfo_undersize)
 {
 	char buf[2] = {};
-	socklen_t optlen = sizeof(buf);
+	socklen_t optlen;
+
+	optlen = sizeof(buf);
 
 	ASSERT_EQ(-1, getsockopt(self->fd, SOL_NETLINK, NETLINK_PKTINFO,
 				 buf, &optlen));
 	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(sizeof(buf), optlen);
 }
 
 TEST_F(netlink, list_memberships_size_discovery)
@@ -105,7 +113,9 @@ TEST_F(netlink, list_memberships_size_discovery)
 TEST_F(netlink, list_memberships_full_read)
 {
 	__u32 buf[64] = {};
-	socklen_t optlen = sizeof(buf);
+	socklen_t optlen;
+
+	optlen = sizeof(buf);
 
 	ASSERT_EQ(0, getsockopt(self->fd, SOL_NETLINK,
 				NETLINK_LIST_MEMBERSHIPS,
@@ -117,22 +127,28 @@ TEST_F(netlink, list_memberships_full_read)
 
 TEST_F(netlink, bad_level)
 {
+	socklen_t optlen;
 	int val;
-	socklen_t optlen = sizeof(val);
+
+	optlen = sizeof(val);
 
 	ASSERT_EQ(-1, getsockopt(self->fd, SOL_SOCKET + 1, NETLINK_PKTINFO,
 				 &val, &optlen));
 	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(val), optlen);
 }
 
 TEST_F(netlink, bad_optname)
 {
+	socklen_t optlen;
 	int val;
-	socklen_t optlen = sizeof(val);
+
+	optlen = sizeof(val);
 
 	ASSERT_EQ(-1, getsockopt(self->fd, SOL_NETLINK, 0x7fff,
 				 &val, &optlen));
 	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(val), optlen);
 }
 
 /* ---------- vsock ---------- */
@@ -157,8 +173,10 @@ FIXTURE_TEARDOWN(vsock)
 
 TEST_F(vsock, buffer_size_exact)
 {
+	socklen_t optlen;
 	uint64_t val = 0;
-	socklen_t optlen = sizeof(val);
+
+	optlen = sizeof(val);
 
 	ASSERT_EQ(0, getsockopt(self->fd, AF_VSOCK,
 				SO_VM_SOCKETS_BUFFER_SIZE,
@@ -170,7 +188,9 @@ TEST_F(vsock, buffer_size_exact)
 TEST_F(vsock, buffer_size_oversize_clamped)
 {
 	char buf[16] = {};
-	socklen_t optlen = sizeof(buf);
+	socklen_t optlen;
+
+	optlen = sizeof(buf);
 
 	ASSERT_EQ(0, getsockopt(self->fd, AF_VSOCK,
 				SO_VM_SOCKETS_BUFFER_SIZE,
@@ -181,33 +201,100 @@ TEST_F(vsock, buffer_size_oversize_clamped)
 TEST_F(vsock, buffer_size_undersize)
 {
 	char buf[4] = {};
-	socklen_t optlen = sizeof(buf);
+	socklen_t optlen;
+
+	optlen = sizeof(buf);
 
 	ASSERT_EQ(-1, getsockopt(self->fd, AF_VSOCK,
 				 SO_VM_SOCKETS_BUFFER_SIZE,
 				 buf, &optlen));
 	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(sizeof(buf), optlen);
 }
 
 TEST_F(vsock, bad_level)
 {
+	socklen_t optlen;
 	uint64_t val;
-	socklen_t optlen = sizeof(val);
+
+	optlen = sizeof(val);
 
 	ASSERT_EQ(-1, getsockopt(self->fd, SOL_SOCKET + 1,
 				 SO_VM_SOCKETS_BUFFER_SIZE,
 				 &val, &optlen));
 	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(val), optlen);
 }
 
 TEST_F(vsock, bad_optname)
 {
+	socklen_t optlen;
 	uint64_t val;
-	socklen_t optlen = sizeof(val);
+
+	optlen = sizeof(val);
 
 	ASSERT_EQ(-1, getsockopt(self->fd, AF_VSOCK, 0x7fff,
 				 &val, &optlen));
 	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(val), optlen);
+}
+
+/* SO_VM_SOCKETS_CONNECT_TIMEOUT_{NEW,OLD} return a sock_timeval-shaped
+ * payload, which is wider than u64 on 64-bit. They exercise the path
+ * where the protocol's reported lv (16 bytes) is larger than the
+ * common 8-byte u64 case covered above.
+ */
+TEST_F(vsock, connect_timeout_new_exact)
+{
+	struct __kernel_sock_timeval tv = {};
+	socklen_t optlen;
+
+	optlen = sizeof(tv);
+
+	ASSERT_EQ(0, getsockopt(self->fd, AF_VSOCK,
+				SO_VM_SOCKETS_CONNECT_TIMEOUT_NEW,
+				&tv, &optlen));
+	ASSERT_EQ(sizeof(tv), optlen);
+}
+
+TEST_F(vsock, connect_timeout_new_oversize_clamped)
+{
+	char buf[sizeof(struct __kernel_sock_timeval) * 2] = {};
+	socklen_t optlen;
+
+	optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, AF_VSOCK,
+				SO_VM_SOCKETS_CONNECT_TIMEOUT_NEW,
+				buf, &optlen));
+	ASSERT_EQ(sizeof(struct __kernel_sock_timeval), optlen);
+}
+
+TEST_F(vsock, connect_timeout_new_undersize)
+{
+	socklen_t optlen;
+	uint64_t val;
+
+	optlen = sizeof(val);
+
+	ASSERT_EQ(-1, getsockopt(self->fd, AF_VSOCK,
+				 SO_VM_SOCKETS_CONNECT_TIMEOUT_NEW,
+				 &val, &optlen));
+	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(sizeof(val), optlen);
+}
+
+TEST_F(vsock, connect_timeout_old_exact)
+{
+	struct __kernel_old_timeval tv = {};
+	socklen_t optlen;
+
+	optlen = sizeof(tv);
+
+	ASSERT_EQ(0, getsockopt(self->fd, AF_VSOCK,
+				SO_VM_SOCKETS_CONNECT_TIMEOUT_OLD,
+				&tv, &optlen));
+	ASSERT_EQ(sizeof(tv), optlen);
 }
 
 TEST_HARNESS_MAIN

-- 
2.52.0