[PATCH net-next 0/5] net: convert four more protocols to getsockopt_iter

[PATCH net-next 0/5] net: convert four more protocols to getsockopt_iter

[Breno Leitao]

Continue the work to convert protocols to the new getsockopt_iter API.

Convert four additional getsockopt implementations to the new
sockopt_t/getsockopt_iter callback:

  - CAN ISO-TP
  - MCTP
  - CAN J1939
  - LLC

These are mechanical, ABI-preserving conversions following the same
pattern as the previously converted protocols (af_packet, can/raw,
af_netlink, af_vsock): the (char __user *optval, int __user *optlen)
pair is replaced with a single sockopt_t *opt that carries the buffer
length on input and the returned size on output, and exposes an iov_iter
for the copy-out path. put_user()/copy_to_user() pairs are replaced with
a single copy_to_iter() per option, and the wrapper in
do_sock_getsockopt() handles writing optlen back to userspace.

I picked these four because each is small and self-contained, with only
one getsockopt callback and a handful of options, so the conversions are
easy to audit individually.

NOTE: optlen is always updated (returned to userspace) even when optval
fails to copy. I.e, userspace will get the "new" optlen even when the
getsockop() fails. That seems wrong, but, this just preserve the
previous behaviour, not changing it.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (5):
      can: isotp: convert to getsockopt_iter
      can: j1939: convert to getsockopt_iter
      mctp: convert to getsockopt_iter
      llc: convert to getsockopt_iter
      selftests: net: getsockopt_iter: address review nits

 net/can/isotp.c                               |  12 ++-
 net/can/j1939/socket.c                        |  21 +++--
 net/llc/af_llc.c                              |  15 ++--
 net/mctp/af_mctp.c                            |  10 +--
 tools/testing/selftests/net/getsockopt_iter.c | 109 +++++++++++++++++++++++---
 5 files changed, 128 insertions(+), 39 deletions(-)
---
base-commit: c1e5127b577c6b88fa48e532616932ae978528d5
change-id: 20260505-getsock_two-abad19643336

Best regards,
--  
Breno Leitao <leitao@debian.org>

[PATCH net-next 1/5] can: isotp: convert to getsockopt_iter

[Breno Leitao]

Convert CAN ISO-TP socket's getsockopt implementation to use the new
getsockopt_iter callback with sockopt_t.

Key changes:
- Replace (char __user *optval, int __user *optlen) with sockopt_t *opt
- Use opt->optlen for buffer length (input) and returned size (output)
- Use copy_to_iter() instead of put_user()/copy_to_user()

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/can/isotp.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index c48b4a818297e..1c33f09fbd338 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1500,7 +1500,7 @@ static int isotp_setsockopt(struct socket *sock, int level, int optname,
 }
 
 static int isotp_getsockopt(struct socket *sock, int level, int optname,
-			    char __user *optval, int __user *optlen)
+			    sockopt_t *opt)
 {
 	struct sock *sk = sock->sk;
 	struct isotp_sock *so = isotp_sk(sk);
@@ -1509,8 +1509,7 @@ static int isotp_getsockopt(struct socket *sock, int level, int optname,
 
 	if (level != SOL_CAN_ISOTP)
 		return -EINVAL;
-	if (get_user(len, optlen))
-		return -EFAULT;
+	len = opt->optlen;
 	if (len < 0)
 		return -EINVAL;
 
@@ -1544,9 +1543,8 @@ static int isotp_getsockopt(struct socket *sock, int level, int optname,
 		return -ENOPROTOOPT;
 	}
 
-	if (put_user(len, optlen))
-		return -EFAULT;
-	if (copy_to_user(optval, val, len))
+	opt->optlen = len;
+	if (copy_to_iter(val, len, &opt->iter_out) != len)
 		return -EFAULT;
 	return 0;
 }
@@ -1718,7 +1716,7 @@ static const struct proto_ops isotp_ops = {
 	.listen = sock_no_listen,
 	.shutdown = sock_no_shutdown,
 	.setsockopt = isotp_setsockopt,
-	.getsockopt = isotp_getsockopt,
+	.getsockopt_iter = isotp_getsockopt,
 	.sendmsg = isotp_sendmsg,
 	.recvmsg = isotp_recvmsg,
 	.mmap = sock_no_mmap,

-- 
2.52.0

[PATCH net-next 2/5] can: j1939: convert to getsockopt_iter

[Breno Leitao]

Convert CAN J1939 socket's getsockopt implementation to use the new
getsockopt_iter callback with sockopt_t.

Key changes:
- Replace (char __user *optval, int __user *optlen) with sockopt_t *opt
- Use opt->optlen for buffer length (input) and returned size (output)
- Use copy_to_iter() instead of copy_to_user()
- Restructure the chained if/else if (which depended on put_user() being
  an expression) into a nested if/else block now that opt->optlen = len
  is a statement
- Add linux/uio.h for copy_to_iter()

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/can/j1939/socket.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 50a598ef5fd4a..d0c6ce607b0dc 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -17,6 +17,7 @@
 #include <linux/can/skb.h>
 #include <linux/errqueue.h>
 #include <linux/if_arp.h>
+#include <linux/uio.h>
 #include <net/can.h>
 
 #include "j1939-priv.h"
@@ -767,7 +768,7 @@ static int j1939_sk_setsockopt(struct socket *sock, int level, int optname,
 }
 
 static int j1939_sk_getsockopt(struct socket *sock, int level, int optname,
-			       char __user *optval, int __user *optlen)
+			       sockopt_t *opt)
 {
 	struct sock *sk = sock->sk;
 	struct j1939_sock *jsk = j1939_sk(sk);
@@ -779,8 +780,7 @@ static int j1939_sk_getsockopt(struct socket *sock, int level, int optname,
 
 	if (level != SOL_CAN_J1939)
 		return -EINVAL;
-	if (get_user(ulen, optlen))
-		return -EFAULT;
+	ulen = opt->optlen;
 	if (ulen < 0)
 		return -EINVAL;
 
@@ -804,11 +804,16 @@ static int j1939_sk_getsockopt(struct socket *sock, int level, int optname,
 	 * but most sockopt's are 'int' properties, and have 'len' & 'val'
 	 * left unchanged, but instead modified 'tmp'
 	 */
-	if (len > ulen)
-		ret = -EFAULT;
-	else if (put_user(len, optlen))
+	if (len > ulen) {
 		ret = -EFAULT;
-	else if (copy_to_user(optval, val, len))
+		goto no_copy;
+	}
+
+	opt->optlen = len;
+	/* Even if the copy below fails, we want to update optlen. This is
+	 * a bit confusing, but, it preserves the original behaviour
+	 */
+	if (copy_to_iter(val, len, &opt->iter_out) != len)
 		ret = -EFAULT;
 	else
 		ret = 0;
@@ -1385,7 +1390,7 @@ static const struct proto_ops j1939_ops = {
 	.listen = sock_no_listen,
 	.shutdown = sock_no_shutdown,
 	.setsockopt = j1939_sk_setsockopt,
-	.getsockopt = j1939_sk_getsockopt,
+	.getsockopt_iter = j1939_sk_getsockopt,
 	.sendmsg = j1939_sk_sendmsg,
 	.recvmsg = j1939_sk_recvmsg,
 	.mmap = sock_no_mmap,

-- 
2.52.0

[PATCH net-next 3/5] mctp: convert to getsockopt_iter

[Breno Leitao]

Convert MCTP socket's getsockopt implementation to use the new
getsockopt_iter callback with sockopt_t.

Key changes:
- Replace (char __user *optval, int __user *optlen) with sockopt_t *opt
- Use opt->optlen for buffer length (input)
- Use copy_to_iter() instead of copy_to_user()
- Add linux/uio.h for copy_to_iter()

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/mctp/af_mctp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index 209a963112e3a..8af5e2b3c8d12 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -12,6 +12,7 @@
 #include <linux/mctp.h>
 #include <linux/module.h>
 #include <linux/socket.h>
+#include <linux/uio.h>
 
 #include <net/mctp.h>
 #include <net/mctpdevice.h>
@@ -405,7 +406,7 @@ static int mctp_setsockopt(struct socket *sock, int level, int optname,
 }
 
 static int mctp_getsockopt(struct socket *sock, int level, int optname,
-			   char __user *optval, int __user *optlen)
+			   sockopt_t *opt)
 {
 	struct mctp_sock *msk = container_of(sock->sk, struct mctp_sock, sk);
 	int len, val;
@@ -413,14 +414,13 @@ static int mctp_getsockopt(struct socket *sock, int level, int optname,
 	if (level != SOL_MCTP)
 		return -EINVAL;
 
-	if (get_user(len, optlen))
-		return -EFAULT;
+	len = opt->optlen;
 
 	if (optname == MCTP_OPT_ADDR_EXT) {
 		if (len != sizeof(int))
 			return -EINVAL;
 		val = !!msk->addr_ext;
-		if (copy_to_user(optval, &val, len))
+		if (copy_to_iter(&val, len, &opt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -639,7 +639,7 @@ static const struct proto_ops mctp_dgram_ops = {
 	.listen		= sock_no_listen,
 	.shutdown	= sock_no_shutdown,
 	.setsockopt	= mctp_setsockopt,
-	.getsockopt	= mctp_getsockopt,
+	.getsockopt_iter = mctp_getsockopt,
 	.sendmsg	= mctp_sendmsg,
 	.recvmsg	= mctp_recvmsg,
 	.mmap		= sock_no_mmap,

-- 
2.52.0

[PATCH net-next 4/5] llc: convert to getsockopt_iter

[Breno Leitao]

Convert LLC socket's getsockopt implementation to use the new
getsockopt_iter callback with sockopt_t.

Key changes:
- Replace (char __user *optval, int __user *optlen) with sockopt_t *opt
- Use opt->optlen for buffer length (input) and returned size (output)
- Use copy_to_iter() instead of put_user()/copy_to_user()
- Add linux/uio.h for copy_to_iter()

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/llc/af_llc.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 1b210db3119e8..7d723f0bd26c2 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -27,6 +27,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/sched/signal.h>
+#include <linux/uio.h>
 
 #include <net/llc.h>
 #include <net/llc_sap.h>
@@ -1172,19 +1173,16 @@ static int llc_ui_setsockopt(struct socket *sock, int level, int optname,
  *	Get connection specific socket information.
  */
 static int llc_ui_getsockopt(struct socket *sock, int level, int optname,
-			     char __user *optval, int __user *optlen)
+			     sockopt_t *opt)
 {
 	struct sock *sk = sock->sk;
 	struct llc_sock *llc = llc_sk(sk);
-	int val = 0, len = 0, rc = -EINVAL;
+	int val = 0, len, rc = -EINVAL;
 
 	lock_sock(sk);
 	if (unlikely(level != SOL_LLC))
 		goto out;
-	rc = get_user(len, optlen);
-	if (rc)
-		goto out;
-	rc = -EINVAL;
+	len = opt->optlen;
 	if (len != sizeof(int))
 		goto out;
 	switch (optname) {
@@ -1212,7 +1210,8 @@ static int llc_ui_getsockopt(struct socket *sock, int level, int optname,
 		goto out;
 	}
 	rc = 0;
-	if (put_user(len, optlen) || copy_to_user(optval, &val, len))
+	opt->optlen = len;
+	if (copy_to_iter(&val, len, &opt->iter_out) != len)
 		rc = -EFAULT;
 out:
 	release_sock(sk);
@@ -1239,7 +1238,7 @@ static const struct proto_ops llc_ui_ops = {
 	.listen      = llc_ui_listen,
 	.shutdown    = llc_ui_shutdown,
 	.setsockopt  = llc_ui_setsockopt,
-	.getsockopt  = llc_ui_getsockopt,
+	.getsockopt_iter = llc_ui_getsockopt,
 	.sendmsg     = llc_ui_sendmsg,
 	.recvmsg     = llc_ui_recvmsg,
 	.mmap	     = sock_no_mmap,

-- 
2.52.0

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

[Breno Leitao]

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