[PATCH net-next 0/6] ipv4,ipv6: convert the getsockopt switches to sockopt_t

[PATCH net-next 0/6] ipv4,ipv6: convert the getsockopt switches to sockopt_t

[Breno Leitao]

This series continues the migration to sockopt_t, as described in [1].
do_ip_getsockopt() and do_ipv6_getsockopt() are the last two switches
still taking a sockptr_t pair, and everything they forward to now takes
a sockopt_t, so convert them both.

Patch 1 first rejects a negative optlen on ipv6, which do_ip_getsockopt()
already does. len is an int, but every consumer compares it unsigned, so
-1 reads as a huge value and each site clamps to its own reply size. The
conversion cannot carry that.

MCAST_MSFILTER has to move before the switches above it. It is the last
option with the quirk sockopt_expand_out() was added for [2]: optlen only
covers the fixed part, and the real reply size comes from the gf_numsrc
field inside it. Patches 2 and 3 convert it on both families.

Patches 4 and 5 convert the switches themselves. Each wrapper keeps its
__user prototype and builds the sockopt_t with sockopt_init_user(), so
the proto layer is untouched. Patch 6 grows the selftest an ip and an
ipv6 fixture, pinning the returned length and errno.

Link: https://lore.kernel.org/all/20260401-getsockopt-v2-0-611df6771aff@debian.org/ [1]
Link: https://lore.kernel.org/all/20260914-getsockopt_phase6-v2-0-e48befc9602e@debian.org/ [2]

To: David Ahern <dsahern@kernel.org>
To: Ido Schimmel <idosch@nvidia.com>
To: David S. Miller <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Simon Horman <horms@kernel.org>
To: Alexei Starovoitov <ast@kernel.org>
To: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andrii@kernel.org>
To: Eduard Zingerman <eddyz87@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: Martin KaFai Lau <martin.lau@linux.dev>
To: Song Liu <song@kernel.org>
To: Yonghong Song <yonghong.song@linux.dev>
To: Jiri Olsa <jolsa@kernel.org>
To: Emil Tsalapatis <emil@etsalapatis.com>
To: Ihor Solodrai <ihor.solodrai@linux.dev>
To: John Fastabend <john.fastabend@gmail.com>
To: Stanislav Fomichev <sdf@fomichev.me>
To: Shuah Khan <shuah@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: bpf@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
To: Stanislav Fomichev <sdf@fomichev.me>
Cc: david.laight.linux@gmail.com

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (6):
      ipv6: reject a negative optlen in do_ipv6_getsockopt()
      ipv6: mcast: convert ip6_mc_msfget() to sockopt_t
      ipv4: igmp: convert ip_mc_gsfget() to sockopt_t
      ipv4: convert do_ip_getsockopt() to sockopt_t
      ipv6: convert do_ipv6_getsockopt() to sockopt_t
      selftests: net: getsockopt_iter: cover ip and ipv6

 include/linux/igmp.h                          |   2 +-
 include/linux/mroute.h                        |   4 +-
 include/linux/mroute6.h                       |   5 +-
 include/net/ip.h                              |   3 +-
 include/net/ipv6.h                            |   4 +-
 net/core/filter.c                             |  36 ++-
 net/ipv4/igmp.c                               |  23 +-
 net/ipv4/ip_sockglue.c                        | 118 +++++-----
 net/ipv4/ipmr.c                               |  11 +-
 net/ipv6/ip6mr.c                              |  13 +-
 net/ipv6/ipv6_sockglue.c                      | 111 +++++-----
 net/ipv6/mcast.c                              |  19 +-
 tools/testing/selftests/net/getsockopt_iter.c | 302 ++++++++++++++++++++++++++
 13 files changed, 494 insertions(+), 157 deletions(-)
---
base-commit: 42a9fb3382fc2573e92f41d203b095d9a372cfc9
change-id: 20260924-sockopt_expand_out_v2-ee4bc7ba4cea

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

[PATCH net-next 1/6] ipv6: reject a negative optlen in do_ipv6_getsockopt()

[Breno Leitao]

IPv4's do_ip_getsockopt() rejects a negative optlen right after reading
it. do_ipv6_getsockopt() never has, and nothing downstream treats it as
an error either: len is an int, but every consumer compares it unsigned,
so -1 behaves as a huge value and each site clamps to its own reply
size.

	len = min_t(unsigned int, sizeof(int), len);

So getsockopt(fd, SOL_IPV6, IPV6_TCLASS, buf, &len) with len set to -1
answers 4 bytes and reports 4, rather than failing.

This is a bug ready to bite us in the near future, let's get this fixed.

I've found this because testing the rest of the patch was returning
inconsistency when optlen = -1.

Later patches make the check unreachable, since sockopt_init_user() and
sockptr_to_sockopt() both reject a negative length before the switch
runs. Keep it as a guard; the fix stands on its own here.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/ipv6/ipv6_sockglue.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 4b3536571c9804..5c6a0819a2aaff 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -1002,6 +1002,8 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 
 	if (copy_from_sockptr(&len, optlen, sizeof(int)))
 		return -EFAULT;
+	if (len < 0)
+		return -EINVAL;
 	switch (optname) {
 	case MCAST_MSFILTER:
 		if (in_compat_syscall())

-- 
2.53.0-Meta

[PATCH net-next 2/6] ipv6: mcast: convert ip6_mc_msfget() to sockopt_t

[Breno Leitao]

MCAST_MSFILTER reads its reply through ip6_mc_msfget(), reached from
do_ipv6_getsockopt() and from nowhere else. Convert it, and build the
sockopt_t at the call site for as long as the caller still carries a
sockptr_t pair.

optlen only has to cover the fixed part, and the real reply size comes
from the gf_numsrc field inside it. Userspace relies on that, so
sockopt_expand_out() grows optval past optlen, for a user address only
and only far enough for the sources the socket has.

ip6_mc_msfget() now advances over the fixed part and writes the source
list through iter_out. Its callers rewind by the reply length they
already compute, which is exactly what the callee consumed, and land
back where they used to write: offset 0 for the native reply, gf_fmode
for the compat one.

The *optlen store moves out to the call site, guarded by !err so the
-EINVAL, -EADDRNOTAVAIL and -EFAULT returns still leave the caller's
optlen word untouched.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/net/ipv6.h       |  2 +-
 net/ipv6/ipv6_sockglue.c | 63 ++++++++++++++++++++++++++++++++----------------
 net/ipv6/mcast.c         | 19 ++++++++++++---
 3 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 3de07e738538f7..9bb68d75890364 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1195,7 +1195,7 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
 int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
 		  struct sockaddr_storage *list);
 int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
-		  sockptr_t optval, size_t ss_offset);
+		  sockopt_t *opt, size_t ss_offset);
 
 #ifdef CONFIG_PROC_FS
 int ac6_proc_init(struct net *net);
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 5c6a0819a2aaff..1bdb3e001e4fe6 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -922,48 +922,51 @@ static int ipv6_getsockopt_sticky(struct sock *sk, struct ipv6_txoptions *opt,
 	return len;
 }
 
-static int ipv6_get_msfilter(struct sock *sk, sockptr_t optval,
-			     sockptr_t optlen, int len)
+static int ipv6_get_msfilter(struct sock *sk, sockopt_t *opt)
 {
 	const int size0 = offsetof(struct group_filter, gf_slist_flex);
 	struct group_filter gsf;
-	int num;
+	int num, len;
 	int err;
 
-	if (len < size0)
+	if (opt->optlen < size0)
 		return -EINVAL;
-	if (copy_from_sockptr(&gsf, optval, size0))
+	if (copy_from_iter(&gsf, size0, &opt->iter_in) != size0)
 		return -EFAULT;
 	if (gsf.gf_group.ss_family != AF_INET6)
 		return -EADDRNOTAVAIL;
 	num = gsf.gf_numsrc;
 	sockopt_lock_sock(sk);
-	err = ip6_mc_msfget(sk, &gsf, optval, size0);
+	err = ip6_mc_msfget(sk, &gsf, opt, size0);
 	if (!err) {
 		if (num > gsf.gf_numsrc)
 			num = gsf.gf_numsrc;
 		len = GROUP_FILTER_SIZE(num);
-		if (copy_to_sockptr(optlen, &len, sizeof(int)) ||
-		    copy_to_sockptr(optval, &gsf, size0))
+		opt->optlen = len;
+
+		/* ip6_mc_msfget() consumed the whole reply; rewind to the
+		 * fixed part.
+		 */
+		iov_iter_revert(&opt->iter_out, len);
+		if (copy_to_iter(&gsf, size0, &opt->iter_out) != size0)
 			err = -EFAULT;
 	}
 	sockopt_release_sock(sk);
 	return err;
 }
 
-static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval,
-				    sockptr_t optlen, int len)
+static int compat_ipv6_get_msfilter(struct sock *sk, sockopt_t *opt)
 {
 	const int size0 = offsetof(struct compat_group_filter, gf_slist_flex);
 	struct compat_group_filter gf32;
 	struct group_filter gf;
 	int err;
-	int num;
+	int num, len;
 
-	if (len < size0)
+	if (opt->optlen < size0)
 		return -EINVAL;
 
-	if (copy_from_sockptr(&gf32, optval, size0))
+	if (copy_from_iter(&gf32, size0, &opt->iter_in) != size0)
 		return -EFAULT;
 	gf.gf_interface = gf32.gf_interface;
 	gf.gf_fmode = gf32.gf_fmode;
@@ -974,18 +977,22 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval,
 		return -EADDRNOTAVAIL;
 
 	sockopt_lock_sock(sk);
-	err = ip6_mc_msfget(sk, &gf, optval, size0);
+	err = ip6_mc_msfget(sk, &gf, opt, size0);
 	sockopt_release_sock(sk);
 	if (err)
 		return err;
 	if (num > gf.gf_numsrc)
 		num = gf.gf_numsrc;
 	len = GROUP_FILTER_SIZE(num) - (sizeof(gf)-sizeof(gf32));
-	if (copy_to_sockptr(optlen, &len, sizeof(int)) ||
-	    copy_to_sockptr_offset(optval, offsetof(struct compat_group_filter, gf_fmode),
-				   &gf.gf_fmode, sizeof(gf32.gf_fmode)) ||
-	    copy_to_sockptr_offset(optval, offsetof(struct compat_group_filter, gf_numsrc),
-				   &gf.gf_numsrc, sizeof(gf32.gf_numsrc)))
+	opt->optlen = len;
+
+	/* Rewind to gf_fmode, which gf_numsrc follows. */
+	iov_iter_revert(&opt->iter_out,
+			len - offsetof(struct compat_group_filter, gf_fmode));
+	if (copy_to_iter(&gf.gf_fmode, sizeof(gf32.gf_fmode),
+			 &opt->iter_out) != sizeof(gf32.gf_fmode) ||
+	    copy_to_iter(&gf.gf_numsrc, sizeof(gf32.gf_numsrc),
+			 &opt->iter_out) != sizeof(gf32.gf_numsrc))
 		return -EFAULT;
 	return 0;
 }
@@ -1006,9 +1013,23 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		return -EINVAL;
 	switch (optname) {
 	case MCAST_MSFILTER:
+	{
+		struct kvec kvec;
+		sockopt_t opt;
+		int err;
+
+		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
+		if (err)
+			return err;
+
 		if (in_compat_syscall())
-			return compat_ipv6_get_msfilter(sk, optval, optlen, len);
-		return ipv6_get_msfilter(sk, optval, optlen, len);
+			err = compat_ipv6_get_msfilter(sk, &opt);
+		else
+			err = ipv6_get_msfilter(sk, &opt);
+		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
+			err = -EFAULT;
+		return err;
+	}
 	case IPV6_2292PKTOPTIONS:
 	{
 		struct msghdr msg;
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index ecef55f261890c..4ca2d77811f4eb 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -600,14 +600,14 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
 }
 
 int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
-		  sockptr_t optval, size_t ss_offset)
+		  sockopt_t *opt, size_t ss_offset)
 {
 	struct ipv6_pinfo *inet6 = inet6_sk(sk);
 	const struct in6_addr *group;
 	struct ipv6_mc_socklist *pmc;
 	struct ip6_sf_socklist *psl;
+	int i, copycount, err;
 	unsigned int count;
-	int i, copycount;
 
 	group = &((struct sockaddr_in6 *)&gsf->gf_group)->sin6_addr;
 
@@ -629,6 +629,18 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
 
 	copycount = min(count, gsf->gf_numsrc);
 	gsf->gf_numsrc = count;
+
+	/* The source list is sized by the gf_numsrc the caller left in optval,
+	 * not by optlen, which only has to cover the fixed part.
+	 */
+	err = sockopt_expand_out(opt, ss_offset +
+				 copycount * sizeof(struct sockaddr_storage));
+	if (err)
+		return err;
+
+	/* The caller fills the fixed part in once it knows gf_numsrc. */
+	iov_iter_advance(&opt->iter_out, ss_offset);
+
 	for (i = 0; i < copycount; i++) {
 		struct sockaddr_in6 *psin6;
 		struct sockaddr_storage ss;
@@ -637,9 +649,8 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
 		memset(&ss, 0, sizeof(ss));
 		psin6->sin6_family = AF_INET6;
 		psin6->sin6_addr = psl->sl_addr[i];
-		if (copy_to_sockptr_offset(optval, ss_offset, &ss, sizeof(ss)))
+		if (copy_to_iter(&ss, sizeof(ss), &opt->iter_out) != sizeof(ss))
 			return -EFAULT;
-		ss_offset += sizeof(ss);
 	}
 	return 0;
 }

-- 
2.53.0-Meta

[PATCH net-next 3/6] ipv4: igmp: convert ip_mc_gsfget() to sockopt_t

[Breno Leitao]

MCAST_MSFILTER reads its reply through ip_mc_gsfget(), reached from
do_ip_getsockopt() and from nowhere else. Convert it the same way as
the ipv6 side, and build the sockopt_t at the call site for as long as
the caller still carries a sockptr_t pair.

optlen here only has to cover the fixed part, and the real reply size
comes from the gf_numsrc field inside it. This is nasty, but userspace
relies on it, so sockopt_expand_out() preserves the same mechanism: it
grows optval only for a user address, and only far enough for the
sources the socket has.

ip_mc_gsfget() writes the source list, and its two callers write the
fixed part afterwards, at the head of optval. iter_out only moves
forward, so the callee advances over the fixed part rather than
addressing each source by offset.

The callers then rewind. The reply length they already compute is
exactly what the callee consumed, ss_offset plus the sources it wrote,
so both land back where they used to write: offset 0 for the native
reply, gf_fmode for the compat one. The bytes are the same.

The *optlen store moves out to the call site, guarded by !err so the
-EINVAL, -EADDRNOTAVAIL and -EFAULT returns still leave the caller's
optlen word untouched.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/igmp.h   |  2 +-
 net/ipv4/igmp.c        | 20 +++++++++++++-----
 net/ipv4/ip_sockglue.c | 57 +++++++++++++++++++++++++++++++-------------------
 3 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index e075611344ef3b..0a1abf3552d2bb 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -276,7 +276,7 @@ extern int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf,int ifindex);
 extern int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 			sockopt_t *opt);
 extern int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
-			sockptr_t optval, size_t offset);
+			sockopt_t *opt, size_t offset);
 extern int ip_mc_sf_allow(const struct sock *sk, __be32 local, __be32 rmt,
 			  int dif, int sdif);
 extern void ip_mc_init_dev(struct in_device *);
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 144fca158adcb0..d573c5bf8f038b 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2775,9 +2775,9 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf, sockopt_t *opt)
 }
 
 int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
-		 sockptr_t optval, size_t ss_offset)
+		 sockopt_t *opt, size_t ss_offset)
 {
-	int i, count, copycount;
+	int i, count, copycount, err;
 	struct sockaddr_in *psin;
 	__be32 addr;
 	struct ip_mc_socklist *pmc;
@@ -2805,6 +2805,18 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 	count = psl ? psl->sl_count : 0;
 	copycount = count < gsf->gf_numsrc ? count : gsf->gf_numsrc;
 	gsf->gf_numsrc = count;
+
+	/* The source list is sized by the gf_numsrc the caller left in optval,
+	 * not by optlen, which only has to cover the fixed part.
+	 */
+	err = sockopt_expand_out(opt, ss_offset +
+				 copycount * sizeof(struct sockaddr_storage));
+	if (err)
+		return err;
+
+	/* The caller fills the fixed part in once it knows gf_numsrc. */
+	iov_iter_advance(&opt->iter_out, ss_offset);
+
 	for (i = 0; i < copycount; i++) {
 		struct sockaddr_storage ss;
 
@@ -2812,10 +2824,8 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 		memset(&ss, 0, sizeof(ss));
 		psin->sin_family = AF_INET;
 		psin->sin_addr.s_addr = psl->sl_addr[i];
-		if (copy_to_sockptr_offset(optval, ss_offset,
-					   &ss, sizeof(ss)))
+		if (copy_to_iter(&ss, sizeof(ss), &opt->iter_out) != sizeof(ss))
 			return -EFAULT;
-		ss_offset += sizeof(ss);
 	}
 	return 0;
 }
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index e06c1f48ecad6e..1f452b6ea86e9d 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1442,45 +1442,46 @@ static bool getsockopt_needs_rtnl(int optname)
 	return false;
 }
 
-static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
-				 sockptr_t optlen, int len)
+static int ip_get_mcast_msfilter(struct sock *sk, sockopt_t *opt)
 {
 	const int size0 = offsetof(struct group_filter, gf_slist_flex);
 	struct group_filter gsf;
 	int num, gsf_size;
 	int err;
 
-	if (len < size0)
+	if (opt->optlen < size0)
 		return -EINVAL;
-	if (copy_from_sockptr(&gsf, optval, size0))
+	if (copy_from_iter(&gsf, size0, &opt->iter_in) != size0)
 		return -EFAULT;
 
 	num = gsf.gf_numsrc;
-	err = ip_mc_gsfget(sk, &gsf, optval,
+	err = ip_mc_gsfget(sk, &gsf, opt,
 			   offsetof(struct group_filter, gf_slist_flex));
 	if (err)
 		return err;
 	if (gsf.gf_numsrc < num)
 		num = gsf.gf_numsrc;
 	gsf_size = GROUP_FILTER_SIZE(num);
-	if (copy_to_sockptr(optlen, &gsf_size, sizeof(int)) ||
-	    copy_to_sockptr(optval, &gsf, size0))
+	opt->optlen = gsf_size;
+
+	/* ip_mc_gsfget() consumed the whole reply; rewind to the fixed part. */
+	iov_iter_revert(&opt->iter_out, gsf_size);
+	if (copy_to_iter(&gsf, size0, &opt->iter_out) != size0)
 		return -EFAULT;
 	return 0;
 }
 
-static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
-					sockptr_t optlen, int len)
+static int compat_ip_get_mcast_msfilter(struct sock *sk, sockopt_t *opt)
 {
 	const int size0 = offsetof(struct compat_group_filter, gf_slist_flex);
 	struct compat_group_filter gf32;
 	struct group_filter gf;
-	int num;
+	int num, len;
 	int err;
 
-	if (len < size0)
+	if (opt->optlen < size0)
 		return -EINVAL;
-	if (copy_from_sockptr(&gf32, optval, size0))
+	if (copy_from_iter(&gf32, size0, &opt->iter_in) != size0)
 		return -EFAULT;
 
 	gf.gf_interface = gf32.gf_interface;
@@ -1488,18 +1489,22 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
 	num = gf.gf_numsrc = gf32.gf_numsrc;
 	gf.gf_group = gf32.gf_group;
 
-	err = ip_mc_gsfget(sk, &gf, optval,
+	err = ip_mc_gsfget(sk, &gf, opt,
 			   offsetof(struct compat_group_filter, gf_slist_flex));
 	if (err)
 		return err;
 	if (gf.gf_numsrc < num)
 		num = gf.gf_numsrc;
 	len = GROUP_FILTER_SIZE(num) - (sizeof(gf) - sizeof(gf32));
-	if (copy_to_sockptr(optlen, &len, sizeof(int)) ||
-	    copy_to_sockptr_offset(optval, offsetof(struct compat_group_filter, gf_fmode),
-				   &gf.gf_fmode, sizeof(gf.gf_fmode)) ||
-	    copy_to_sockptr_offset(optval, offsetof(struct compat_group_filter, gf_numsrc),
-				   &gf.gf_numsrc, sizeof(gf.gf_numsrc)))
+	opt->optlen = len;
+
+	/* Rewind to gf_fmode, which gf_numsrc follows. */
+	iov_iter_revert(&opt->iter_out,
+			len - offsetof(struct compat_group_filter, gf_fmode));
+	if (copy_to_iter(&gf.gf_fmode, sizeof(gf32.gf_fmode),
+			 &opt->iter_out) != sizeof(gf32.gf_fmode) ||
+	    copy_to_iter(&gf.gf_numsrc, sizeof(gf32.gf_numsrc),
+			 &opt->iter_out) != sizeof(gf32.gf_numsrc))
 		return -EFAULT;
 	return 0;
 }
@@ -1727,12 +1732,22 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		goto out;
 	}
 	case MCAST_MSFILTER:
+	{
+		struct kvec kvec;
+		sockopt_t opt;
+
+		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
+		if (err)
+			goto out;
+
 		if (in_compat_syscall())
-			err = compat_ip_get_mcast_msfilter(sk, optval, optlen,
-							   len);
+			err = compat_ip_get_mcast_msfilter(sk, &opt);
 		else
-			err = ip_get_mcast_msfilter(sk, optval, optlen, len);
+			err = ip_get_mcast_msfilter(sk, &opt);
+		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
+			err = -EFAULT;
 		goto out;
+	}
 	case IP_PROTOCOL:
 		val = inet_sk(sk)->inet_num;
 		break;

-- 
2.53.0-Meta

[PATCH net-next 4/6] ipv4: convert do_ip_getsockopt() to sockopt_t

[Breno Leitao]

Convert the SOL_IP switch and ip_mroute_getsockopt(), its last leaf on
a sockptr_t pair. The IP_MSFILTER and MCAST_MSFILTER bridges go away.

ip_getsockopt() builds the sockopt_t with sockopt_init_user() and
writes optlen back unconditionally, because IP_OPTIONS and others
report a length then fail; ip_mc_msfget() therefore stores its length
last. IP_PKTOPTIONS takes its buffer off iter_out. The argument is
sopt, since IP_OPTIONS already has an ip_options local named opt.

Validating optlen now precedes the level check and the MRT_* dispatch,
so a negative optlen wins over -EOPNOTSUPP there.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/mroute.h |  4 +--
 include/net/ip.h       |  3 +-
 net/core/filter.c      | 18 ++++++++---
 net/ipv4/igmp.c        |  3 +-
 net/ipv4/ip_sockglue.c | 87 +++++++++++++++++++-------------------------------
 net/ipv4/ipmr.c        | 11 +++----
 6 files changed, 56 insertions(+), 70 deletions(-)

diff --git a/include/linux/mroute.h b/include/linux/mroute.h
index 4c5003afee6c51..c1c21e61c81c37 100644
--- a/include/linux/mroute.h
+++ b/include/linux/mroute.h
@@ -17,7 +17,7 @@ static inline int ip_mroute_opt(int opt)
 }
 
 int ip_mroute_setsockopt(struct sock *, int, sockptr_t, unsigned int);
-int ip_mroute_getsockopt(struct sock *, int, sockptr_t, sockptr_t);
+int ip_mroute_getsockopt(struct sock *sk, int optname, sockopt_t *opt);
 int ipmr_ioctl(struct sock *sk, int cmd, void *arg);
 int ipmr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
 int ip_mr_init(void);
@@ -31,7 +31,7 @@ static inline int ip_mroute_setsockopt(struct sock *sock, int optname,
 }
 
 static inline int ip_mroute_getsockopt(struct sock *sk, int optname,
-				       sockptr_t optval, sockptr_t optlen)
+				       sockopt_t *opt)
 {
 	return -ENOPROTOOPT;
 }
diff --git a/include/net/ip.h b/include/net/ip.h
index 6f602df72ee621..81aef4b98430bf 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -823,8 +823,7 @@ int do_ip_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
 		     unsigned int optlen);
 int ip_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
 		  unsigned int optlen);
-int do_ip_getsockopt(struct sock *sk, int level, int optname,
-		     sockptr_t optval, sockptr_t optlen);
+int do_ip_getsockopt(struct sock *sk, int level, int optname, sockopt_t *sopt);
 int ip_getsockopt(struct sock *sk, int level, int optname, char __user *optval,
 		  int __user *optlen);
 int ip_ra_control(struct sock *sk, unsigned char on,
diff --git a/net/core/filter.c b/net/core/filter.c
index 70dc621672f2e9..9a706ec1f7f122 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5737,10 +5737,20 @@ static int sol_ip_sockopt(struct sock *sk, int optname,
 		return -EINVAL;
 	}
 
-	if (getopt)
-		return do_ip_getsockopt(sk, SOL_IP, optname,
-					KERNEL_SOCKPTR(optval),
-					KERNEL_SOCKPTR(optlen));
+	if (getopt) {
+		struct kvec kvec;
+		sockopt_t opt;
+		int err;
+
+		err = sockptr_to_sockopt(&opt, KERNEL_SOCKPTR(optval),
+					 KERNEL_SOCKPTR(optlen), &kvec);
+		if (err)
+			return err;
+
+		err = do_ip_getsockopt(sk, SOL_IP, optname, &opt);
+		*optlen = opt.optlen;
+		return err;
+	}
 
 	return do_ip_setsockopt(sk, SOL_IP, optname,
 				KERNEL_SOCKPTR(optval), *optlen);
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d573c5bf8f038b..bf00810cdd7a13 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2763,12 +2763,13 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf, sockopt_t *opt)
 	if (err)
 		return err;
 
-	opt->optlen = msf_size;
 	if (copy_to_iter(msf, IP_MSFILTER_SIZE(0), &opt->iter_out) !=
 	    IP_MSFILTER_SIZE(0))
 		return -EFAULT;
 	if (len && copy_to_iter(psl->sl_addr, len, &opt->iter_out) != len)
 		return -EFAULT;
+
+	opt->optlen = msf_size;
 	return 0;
 done:
 	return err;
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 1f452b6ea86e9d..2e78b11d294a9f 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1509,8 +1509,7 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockopt_t *opt)
 	return 0;
 }
 
-int do_ip_getsockopt(struct sock *sk, int level, int optname,
-		     sockptr_t optval, sockptr_t optlen)
+int do_ip_getsockopt(struct sock *sk, int level, int optname, sockopt_t *sopt)
 {
 	struct inet_sock *inet = inet_sk(sk);
 	bool needs_rtnl = getsockopt_needs_rtnl(optname);
@@ -1521,10 +1520,9 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		return -EOPNOTSUPP;
 
 	if (ip_mroute_opt(optname))
-		return ip_mroute_getsockopt(sk, optname, optval, optlen);
+		return ip_mroute_getsockopt(sk, optname, sopt);
 
-	if (copy_from_sockptr(&len, optlen, sizeof(int)))
-		return -EFAULT;
+	len = sopt->optlen;
 	if (len < 0)
 		return -EINVAL;
 
@@ -1620,16 +1618,15 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		rcu_read_unlock();
 
 		if (opt->optlen == 0) {
-			len = 0;
-			return copy_to_sockptr(optlen, &len, sizeof(int));
+			sopt->optlen = 0;
+			return 0;
 		}
 
 		ip_options_undo(opt);
 
 		len = min_t(unsigned int, len, opt->optlen);
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, opt->__data, len))
+		sopt->optlen = len;
+		if (copy_to_iter(opt->__data, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -1653,12 +1650,12 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		if (sk->sk_type != SOCK_STREAM)
 			return -ENOPROTOOPT;
 
-		if (optval.is_kernel) {
+		if (iov_iter_is_kvec(&sopt->iter_out)) {
 			msg.msg_control_is_user = false;
-			msg.msg_control = optval.kernel;
+			msg.msg_control = sopt->iter_out.kvec->iov_base;
 		} else {
 			msg.msg_control_is_user = true;
-			msg.msg_control_user = optval.user;
+			msg.msg_control_user = sopt->iter_out.ubuf;
 		}
 		msg.msg_controllen = len;
 		msg.msg_flags = in_compat_syscall() ? MSG_CMSG_COMPAT : 0;
@@ -1680,8 +1677,8 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 			int tos = READ_ONCE(inet->rcv_tos);
 			put_cmsg(&msg, SOL_IP, IP_TOS, sizeof(tos), &tos);
 		}
-		len -= msg.msg_controllen;
-		return copy_to_sockptr(optlen, &len, sizeof(int));
+		sopt->optlen = len - msg.msg_controllen;
+		return 0;
 	}
 	case IP_UNICAST_IF:
 		val = (__force int)htonl((__u32) READ_ONCE(inet->uc_index));
@@ -1692,9 +1689,8 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		len = min_t(unsigned int, len, sizeof(struct in_addr));
 		addr.s_addr = READ_ONCE(inet->mc_addr);
 
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &addr, len))
+		sopt->optlen = len;
+		if (copy_to_iter(&addr, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -1711,43 +1707,25 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 	case IP_MSFILTER:
 	{
 		struct ip_msfilter msf;
-		struct kvec kvec;
-		sockopt_t opt;
 
 		if (len < IP_MSFILTER_SIZE(0)) {
 			err = -EINVAL;
 			goto out;
 		}
-		if (copy_from_sockptr(&msf, optval, IP_MSFILTER_SIZE(0))) {
+		if (copy_from_iter(&msf, IP_MSFILTER_SIZE(0), &sopt->iter_in) !=
+		    IP_MSFILTER_SIZE(0)) {
 			err = -EFAULT;
 			goto out;
 		}
-		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
-		if (err)
-			goto out;
-
-		err = ip_mc_msfget(sk, &msf, &opt);
-		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
-			err = -EFAULT;
+		err = ip_mc_msfget(sk, &msf, sopt);
 		goto out;
 	}
 	case MCAST_MSFILTER:
-	{
-		struct kvec kvec;
-		sockopt_t opt;
-
-		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
-		if (err)
-			goto out;
-
 		if (in_compat_syscall())
-			err = compat_ip_get_mcast_msfilter(sk, &opt);
+			err = compat_ip_get_mcast_msfilter(sk, sopt);
 		else
-			err = ip_get_mcast_msfilter(sk, &opt);
-		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
-			err = -EFAULT;
+			err = ip_get_mcast_msfilter(sk, sopt);
 		goto out;
-	}
 	case IP_PROTOCOL:
 		val = inet_sk(sk)->inet_num;
 		break;
@@ -1759,16 +1737,14 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 copyval:
 	if (len < sizeof(int) && len > 0 && val >= 0 && val <= 255) {
 		unsigned char ucval = (unsigned char)val;
-		len = 1;
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &ucval, 1))
+
+		sopt->optlen = 1;
+		if (copy_to_iter(&ucval, 1, &sopt->iter_out) != 1)
 			return -EFAULT;
 	} else {
 		len = min_t(unsigned int, sizeof(int), len);
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &val, len))
+		sopt->optlen = len;
+		if (copy_to_iter(&val, len, &sopt->iter_out) != len)
 			return -EFAULT;
 	}
 	return 0;
@@ -1783,19 +1759,22 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 int ip_getsockopt(struct sock *sk, int level,
 		  int optname, char __user *optval, int __user *optlen)
 {
+	sockopt_t opt;
 	int err;
 
-	err = do_ip_getsockopt(sk, level, optname,
-			       USER_SOCKPTR(optval), USER_SOCKPTR(optlen));
+	err = sockopt_init_user(&opt, optval, optlen);
+	if (err)
+		return err;
+
+	err = do_ip_getsockopt(sk, level, optname, &opt);
+	if (put_user(opt.optlen, optlen))
+		return -EFAULT;
 
 #ifdef CONFIG_NETFILTER
 	/* we need to exclude all possible ENOPROTOOPTs except default case */
 	if (err == -ENOPROTOOPT && optname != IP_PKTOPTIONS &&
 			!ip_mroute_opt(optname)) {
-		int len;
-
-		if (get_user(len, optlen))
-			return -EFAULT;
+		int len = opt.optlen;
 
 		err = nf_getsockopt(sk, PF_INET, optname, optval, &len);
 		if (err >= 0)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index e4c51ca473019c..f249569b4bf8b9 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1638,8 +1638,7 @@ int ipmr_sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
 }
 
 /* Getsock opt support for the multicast routing system. */
-int ip_mroute_getsockopt(struct sock *sk, int optname, sockptr_t optval,
-			 sockptr_t optlen)
+int ip_mroute_getsockopt(struct sock *sk, int optname, sockopt_t *opt)
 {
 	int olr;
 	int val;
@@ -1670,16 +1669,14 @@ int ip_mroute_getsockopt(struct sock *sk, int optname, sockptr_t optval,
 		return -ENOPROTOOPT;
 	}
 
-	if (copy_from_sockptr(&olr, optlen, sizeof(int)))
-		return -EFAULT;
+	olr = opt->optlen;
 	if (olr < 0)
 		return -EINVAL;
 
 	olr = min_t(unsigned int, olr, sizeof(int));
 
-	if (copy_to_sockptr(optlen, &olr, sizeof(int)))
-		return -EFAULT;
-	if (copy_to_sockptr(optval, &val, olr))
+	opt->optlen = olr;
+	if (copy_to_iter(&val, olr, &opt->iter_out) != olr)
 		return -EFAULT;
 	return 0;
 }

-- 
2.53.0-Meta

[PATCH net-next 5/6] ipv6: convert do_ipv6_getsockopt() to sockopt_t

[Breno Leitao]

Convert the SOL_IPV6 switch and its two remaining sockptr_t leaves,
ip6_mroute_getsockopt() and ipv6_getsockopt_sticky(). The
MCAST_MSFILTER bridge goes away.

ipv6_getsockopt() builds the sockopt_t with sockopt_init_user() after
its own level checks, and writes optlen back unconditionally.
IPV6_2292PKTOPTIONS takes its buffer off iter_out; IPV6_FLOWLABEL_MGR
reads through iter_in. The argument is sopt, since the sticky options
already have an ipv6_txoptions local named opt.

MRT6_* is still dispatched before optlen is validated, so a negative
optlen there wins over -EOPNOTSUPP.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/mroute6.h  |  5 ++--
 include/net/ipv6.h       |  2 +-
 net/core/filter.c        | 18 ++++++++---
 net/ipv6/ip6mr.c         | 13 +++-----
 net/ipv6/ipv6_sockglue.c | 78 ++++++++++++++++++++----------------------------
 5 files changed, 53 insertions(+), 63 deletions(-)

diff --git a/include/linux/mroute6.h b/include/linux/mroute6.h
index fddafdc168f733..ab4d206fb32065 100644
--- a/include/linux/mroute6.h
+++ b/include/linux/mroute6.h
@@ -27,7 +27,7 @@ struct sock;
 
 #ifdef CONFIG_IPV6_MROUTE
 extern int ip6_mroute_setsockopt(struct sock *, int, sockptr_t, unsigned int);
-extern int ip6_mroute_getsockopt(struct sock *, int, sockptr_t, sockptr_t);
+int ip6_mroute_getsockopt(struct sock *sk, int optname, sockopt_t *sopt);
 extern int ip6_mr_input(struct sk_buff *skb);
 extern int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
 extern int ip6_mr_init(void);
@@ -42,8 +42,7 @@ static inline int ip6_mroute_setsockopt(struct sock *sock, int optname,
 }
 
 static inline
-int ip6_mroute_getsockopt(struct sock *sock,
-			  int optname, sockptr_t optval, sockptr_t optlen)
+int ip6_mroute_getsockopt(struct sock *sock, int optname, sockopt_t *sopt)
 {
 	return -ENOPROTOOPT;
 }
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 9bb68d75890364..a1e1de7da8c70d 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1142,7 +1142,7 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval
 int ipv6_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
 		    unsigned int optlen);
 int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
-		       sockptr_t optval, sockptr_t optlen);
+		       sockopt_t *sopt);
 int ipv6_getsockopt(struct sock *sk, int level, int optname,
 		    char __user *optval, int __user *optlen);
 
diff --git a/net/core/filter.c b/net/core/filter.c
index 9a706ec1f7f122..e0b79c78c9a9c8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5774,10 +5774,20 @@ static int sol_ipv6_sockopt(struct sock *sk, int optname,
 		return -EINVAL;
 	}
 
-	if (getopt)
-		return do_ipv6_getsockopt(sk, SOL_IPV6, optname,
-					  KERNEL_SOCKPTR(optval),
-					  KERNEL_SOCKPTR(optlen));
+	if (getopt) {
+		struct kvec kvec;
+		sockopt_t opt;
+		int err;
+
+		err = sockptr_to_sockopt(&opt, KERNEL_SOCKPTR(optval),
+					 KERNEL_SOCKPTR(optlen), &kvec);
+		if (err)
+			return err;
+
+		err = do_ipv6_getsockopt(sk, SOL_IPV6, optname, &opt);
+		*optlen = opt.optlen;
+		return err;
+	}
 
 	return do_ipv6_setsockopt(sk, SOL_IPV6, optname,
 				  KERNEL_SOCKPTR(optval), *optlen);
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 36f117ad367081..9513258b95fb88 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1893,8 +1893,7 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
  *	Getsock opt support for the multicast routing system.
  */
 
-int ip6_mroute_getsockopt(struct sock *sk, int optname, sockptr_t optval,
-			  sockptr_t optlen)
+int ip6_mroute_getsockopt(struct sock *sk, int optname, sockopt_t *sopt)
 {
 	int olr;
 	int val;
@@ -1925,16 +1924,12 @@ int ip6_mroute_getsockopt(struct sock *sk, int optname, sockptr_t optval,
 		return -ENOPROTOOPT;
 	}
 
-	if (copy_from_sockptr(&olr, optlen, sizeof(int)))
-		return -EFAULT;
-
-	olr = min_t(int, olr, sizeof(int));
+	olr = min_t(int, sopt->optlen, sizeof(int));
 	if (olr < 0)
 		return -EINVAL;
 
-	if (copy_to_sockptr(optlen, &olr, sizeof(int)))
-		return -EFAULT;
-	if (copy_to_sockptr(optval, &val, olr))
+	sopt->optlen = olr;
+	if (copy_to_iter(&val, olr, &sopt->iter_out) != olr)
 		return -EFAULT;
 	return 0;
 }
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 1bdb3e001e4fe6..3a43e26234eea3 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -889,7 +889,7 @@ int ipv6_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
 EXPORT_SYMBOL(ipv6_setsockopt);
 
 static int ipv6_getsockopt_sticky(struct sock *sk, struct ipv6_txoptions *opt,
-				  int optname, sockptr_t optval, int len)
+				  int optname, sockopt_t *sopt, int len)
 {
 	struct ipv6_opt_hdr *hdr;
 
@@ -917,7 +917,7 @@ static int ipv6_getsockopt_sticky(struct sock *sk, struct ipv6_txoptions *opt,
 		return 0;
 
 	len = min_t(unsigned int, len, ipv6_optlen(hdr));
-	if (copy_to_sockptr(optval, hdr, len))
+	if (copy_to_iter(hdr, len, &sopt->iter_out) != len)
 		return -EFAULT;
 	return len;
 }
@@ -997,39 +997,23 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockopt_t *opt)
 	return 0;
 }
 
-int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
-		       sockptr_t optval, sockptr_t optlen)
+int do_ipv6_getsockopt(struct sock *sk, int level, int optname, sockopt_t *sopt)
 {
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	int len;
 	int val;
 
 	if (ip6_mroute_opt(optname))
-		return ip6_mroute_getsockopt(sk, optname, optval, optlen);
+		return ip6_mroute_getsockopt(sk, optname, sopt);
 
-	if (copy_from_sockptr(&len, optlen, sizeof(int)))
-		return -EFAULT;
+	len = sopt->optlen;
 	if (len < 0)
 		return -EINVAL;
 	switch (optname) {
 	case MCAST_MSFILTER:
-	{
-		struct kvec kvec;
-		sockopt_t opt;
-		int err;
-
-		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
-		if (err)
-			return err;
-
 		if (in_compat_syscall())
-			err = compat_ipv6_get_msfilter(sk, &opt);
-		else
-			err = ipv6_get_msfilter(sk, &opt);
-		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
-			err = -EFAULT;
-		return err;
-	}
+			return compat_ipv6_get_msfilter(sk, sopt);
+		return ipv6_get_msfilter(sk, sopt);
 	case IPV6_2292PKTOPTIONS:
 	{
 		struct msghdr msg;
@@ -1038,12 +1022,12 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		if (sk->sk_type != SOCK_STREAM)
 			return -ENOPROTOOPT;
 
-		if (optval.is_kernel) {
+		if (iov_iter_is_kvec(&sopt->iter_out)) {
 			msg.msg_control_is_user = false;
-			msg.msg_control = optval.kernel;
+			msg.msg_control = sopt->iter_out.kvec->iov_base;
 		} else {
 			msg.msg_control_is_user = true;
-			msg.msg_control_user = optval.user;
+			msg.msg_control_user = sopt->iter_out.ubuf;
 		}
 		msg.msg_controllen = len;
 		msg.msg_flags = 0;
@@ -1094,8 +1078,8 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 				put_cmsg(&msg, SOL_IPV6, IPV6_FLOWINFO, sizeof(flowinfo), &flowinfo);
 			}
 		}
-		len -= msg.msg_controllen;
-		return copy_to_sockptr(optlen, &len, sizeof(int));
+		sopt->optlen = len - msg.msg_controllen;
+		return 0;
 	}
 	case IPV6_MTU:
 	{
@@ -1150,12 +1134,13 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		sockopt_lock_sock(sk);
 		opt = rcu_dereference_protected(np->opt,
 						lockdep_sock_is_held(sk));
-		len = ipv6_getsockopt_sticky(sk, opt, optname, optval, len);
+		len = ipv6_getsockopt_sticky(sk, opt, optname, sopt, len);
 		sockopt_release_sock(sk);
 		/* check if ipv6_getsockopt_sticky() returns err code */
 		if (len < 0)
 			return len;
-		return copy_to_sockptr(optlen, &len, sizeof(int));
+		sopt->optlen = len;
+		return 0;
 	}
 
 	case IPV6_RECVHOPOPTS:
@@ -1209,9 +1194,8 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		if (!mtuinfo.ip6m_mtu)
 			return -ENOTCONN;
 
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &mtuinfo, len))
+		sopt->optlen = len;
+		if (copy_to_iter(&mtuinfo, len, &sopt->iter_out) != len)
 			return -EFAULT;
 
 		return 0;
@@ -1288,7 +1272,8 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		if (len < sizeof(freq))
 			return -EINVAL;
 
-		if (copy_from_sockptr(&freq, optval, sizeof(freq)))
+		if (copy_from_iter(&freq, sizeof(freq), &sopt->iter_in) !=
+		    sizeof(freq))
 			return -EFAULT;
 
 		if (freq.flr_action != IPV6_FL_A_GET)
@@ -1303,9 +1288,8 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		if (val < 0)
 			return val;
 
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &freq, len))
+		sopt->optlen = len;
+		if (copy_to_iter(&freq, len, &sopt->iter_out) != len)
 			return -EFAULT;
 
 		return 0;
@@ -1363,9 +1347,8 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		return -ENOPROTOOPT;
 	}
 	len = min_t(unsigned int, sizeof(int), len);
-	if (copy_to_sockptr(optlen, &len, sizeof(int)))
-		return -EFAULT;
-	if (copy_to_sockptr(optval, &val, len))
+	sopt->optlen = len;
+	if (copy_to_iter(&val, len, &sopt->iter_out) != len)
 		return -EFAULT;
 	return 0;
 }
@@ -1373,6 +1356,7 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 int ipv6_getsockopt(struct sock *sk, int level, int optname,
 		    char __user *optval, int __user *optlen)
 {
+	sockopt_t sopt;
 	int err;
 
 	if (level == SOL_IP && sk->sk_type != SOCK_RAW)
@@ -1381,15 +1365,17 @@ int ipv6_getsockopt(struct sock *sk, int level, int optname,
 	if (level != SOL_IPV6)
 		return -ENOPROTOOPT;
 
-	err = do_ipv6_getsockopt(sk, level, optname,
-				 USER_SOCKPTR(optval), USER_SOCKPTR(optlen));
+	err = sockopt_init_user(&sopt, optval, optlen);
+	if (err)
+		return err;
+
+	err = do_ipv6_getsockopt(sk, level, optname, &sopt);
+	if (put_user(sopt.optlen, optlen))
+		return -EFAULT;
 #ifdef CONFIG_NETFILTER
 	/* we need to exclude all possible ENOPROTOOPTs except default case */
 	if (err == -ENOPROTOOPT && optname != IPV6_2292PKTOPTIONS) {
-		int len;
-
-		if (get_user(len, optlen))
-			return -EFAULT;
+		int len = sopt.optlen;
 
 		err = nf_getsockopt(sk, PF_INET6, optname, optval, &len);
 		if (err >= 0)

-- 
2.53.0-Meta

[PATCH net-next 6/6] selftests: net: getsockopt_iter: cover ip and ipv6

[Breno Leitao]

Add an ip and an ipv6 fixture, pinning the returned length and errno
across buffer sizes, the branches that answer empty, an unknown
optname and the options dispatched before the switch.

SOL_IP answers a sub-int buffer with one byte where SOL_IPV6 clamps the
int. IP_PKTOPTIONS and IPV6_2292PKTOPTIONS want a stream socket;
MRT_*/MRT6_* want a raw one, and skip without CONFIG_IP_MROUTE or
CONFIG_IPV6_MROUTE, where the switch answers ENOPROTOOPT instead.

MRT_VERSION and MRT6_VERSION are spelled out; linux/mroute.h does not
coexist with netinet/in.h here.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/net/getsockopt_iter.c | 302 ++++++++++++++++++++++++++
 1 file changed, 302 insertions(+)

diff --git a/tools/testing/selftests/net/getsockopt_iter.c b/tools/testing/selftests/net/getsockopt_iter.c
index 6c2408df461232..32b6e0a50f1f9c 100644
--- a/tools/testing/selftests/net/getsockopt_iter.c
+++ b/tools/testing/selftests/net/getsockopt_iter.c
@@ -55,6 +55,13 @@
 #ifndef TCP_ULP
 #define TCP_ULP 31
 #endif
+/* linux/mroute.h does not coexist with netinet/in.h here. */
+#ifndef MRT_VERSION
+#define MRT_VERSION 206
+#endif
+#ifndef MRT6_VERSION
+#define MRT6_VERSION 206
+#endif
 
 /* ---------- netlink ---------- */
 
@@ -492,6 +499,301 @@ TEST_F(rawv6, bad_optname)
 	ASSERT_EQ(sizeof(val), optlen);
 }
 
+/* ---------- ip (SOL_IP) ---------- */
+
+FIXTURE(ip)
+{
+	int fd;
+};
+
+FIXTURE_SETUP(ip)
+{
+	/* a router alert option, so IP_OPTIONS has something to answer with */
+	static const unsigned char ipopts[4] = { 0x94, 0x04, 0x00, 0x00 };
+	int ttl = 42;
+
+	self->fd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (self->fd < 0)
+		SKIP(return, "AF_INET dgram socket: %s", strerror(errno));
+
+	if (setsockopt(self->fd, SOL_IP, IP_TTL, &ttl, sizeof(ttl)) < 0)
+		SKIP(return, "set IP_TTL: %s", strerror(errno));
+
+	if (setsockopt(self->fd, SOL_IP, IP_OPTIONS, ipopts,
+		       sizeof(ipopts)) < 0)
+		SKIP(return, "set IP_OPTIONS: %s", strerror(errno));
+}
+
+FIXTURE_TEARDOWN(ip)
+{
+	if (self->fd >= 0)
+		close(self->fd);
+}
+
+TEST_F(ip, ttl_exact)
+{
+	socklen_t optlen = sizeof(int);
+	int val = 0;
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_TTL, &val, &optlen));
+	ASSERT_EQ(sizeof(int), optlen);
+	ASSERT_EQ(42, val);
+}
+
+TEST_F(ip, ttl_oversize_clamped)
+{
+	socklen_t optlen = 64;
+	char buf[64] = {};
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_TTL, buf, &optlen));
+	ASSERT_EQ(sizeof(int), optlen);
+}
+
+/* SOL_IP answers a sub-int buffer with a single byte when the value fits
+ * in one, rather than clamping the int down.
+ */
+TEST_F(ip, ttl_single_byte)
+{
+	unsigned char buf[3] = {};
+	socklen_t optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_TTL, buf, &optlen));
+	ASSERT_EQ(1, optlen);
+	ASSERT_EQ(42, buf[0]);
+}
+
+TEST_F(ip, ttl_zero_len)
+{
+	socklen_t optlen = 0;
+	int val;
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_TTL, &val, &optlen));
+	ASSERT_EQ(0, optlen);
+}
+
+TEST_F(ip, negative_optlen)
+{
+	socklen_t optlen = (socklen_t)-1;
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IP, IP_TTL, &val, &optlen));
+	ASSERT_EQ(EINVAL, errno);
+}
+
+TEST_F(ip, options_roundtrip)
+{
+	unsigned char buf[40] = {};
+	socklen_t optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_OPTIONS, buf, &optlen));
+	ASSERT_EQ(4, optlen);
+	ASSERT_EQ(0x94, buf[0]);
+}
+
+TEST_F(ip, options_undersize_clamped)
+{
+	unsigned char buf[2] = {};
+	socklen_t optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_OPTIONS, buf, &optlen));
+	ASSERT_EQ(sizeof(buf), optlen);
+}
+
+/* With no option set the reply is empty and the call still succeeds. */
+TEST_F(ip, options_absent)
+{
+	unsigned char buf[40] = {};
+	socklen_t optlen = sizeof(buf);
+	int fd;
+
+	fd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (fd < 0)
+		SKIP(return, "AF_INET dgram socket: %s", strerror(errno));
+
+	ASSERT_EQ(0, getsockopt(fd, SOL_IP, IP_OPTIONS, buf, &optlen));
+	ASSERT_EQ(0, optlen);
+	close(fd);
+}
+
+TEST_F(ip, multicast_if_oversize_clamped)
+{
+	socklen_t optlen = 64;
+	char buf[64] = {};
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IP, IP_MULTICAST_IF, buf,
+				&optlen));
+	ASSERT_EQ(sizeof(struct in_addr), optlen);
+}
+
+/* IP_PKTOPTIONS only answers on a stream socket. */
+TEST_F(ip, pktoptions_wrong_type)
+{
+	socklen_t optlen = 64;
+	char buf[64];
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IP, IP_PKTOPTIONS, buf,
+				 &optlen));
+	ASSERT_EQ(ENOPROTOOPT, errno);
+}
+
+/* The MRT_* options are dispatched ahead of the rest of the switch and
+ * want a raw IGMP socket. Without CONFIG_IP_MROUTE they are not
+ * dispatched at all and the switch answers ENOPROTOOPT instead.
+ */
+TEST_F(ip, mroute_wrong_type)
+{
+	socklen_t optlen = sizeof(int);
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IP, MRT_VERSION, &val,
+				 &optlen));
+	if (errno == ENOPROTOOPT)
+		SKIP(return, "CONFIG_IP_MROUTE disabled");
+	ASSERT_EQ(EOPNOTSUPP, errno);
+}
+
+TEST_F(ip, bad_optname)
+{
+	socklen_t optlen = sizeof(int);
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IP, 0x7fff, &val, &optlen));
+	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(int), optlen);
+}
+
+/* ---------- ipv6 (SOL_IPV6) ---------- */
+
+FIXTURE(ipv6)
+{
+	int fd;
+};
+
+FIXTURE_SETUP(ipv6)
+{
+	/* an 8 byte hop-by-hop header, so the sticky options answer */
+	static const unsigned char hopopt[8] = { 0, 0, 1, 4, 0, 0, 0, 0 };
+	int hops = 42;
+
+	self->fd = socket(AF_INET6, SOCK_DGRAM, 0);
+	if (self->fd < 0)
+		SKIP(return, "AF_INET6 dgram socket: %s", strerror(errno));
+
+	if (setsockopt(self->fd, SOL_IPV6, IPV6_UNICAST_HOPS, &hops,
+		       sizeof(hops)) < 0)
+		SKIP(return, "set IPV6_UNICAST_HOPS: %s", strerror(errno));
+
+	if (setsockopt(self->fd, SOL_IPV6, IPV6_HOPOPTS, hopopt,
+		       sizeof(hopopt)) < 0)
+		SKIP(return, "set IPV6_HOPOPTS: %s", strerror(errno));
+}
+
+FIXTURE_TEARDOWN(ipv6)
+{
+	if (self->fd >= 0)
+		close(self->fd);
+}
+
+TEST_F(ipv6, hops_exact)
+{
+	socklen_t optlen = sizeof(int);
+	int val = 0;
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_UNICAST_HOPS, &val,
+				&optlen));
+	ASSERT_EQ(sizeof(int), optlen);
+	ASSERT_EQ(42, val);
+}
+
+TEST_F(ipv6, hops_oversize_clamped)
+{
+	socklen_t optlen = 64;
+	char buf[64] = {};
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_UNICAST_HOPS, buf,
+				&optlen));
+	ASSERT_EQ(sizeof(int), optlen);
+}
+
+TEST_F(ipv6, hopopts_roundtrip)
+{
+	unsigned char buf[64] = {};
+	socklen_t optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_HOPOPTS, buf,
+				&optlen));
+	ASSERT_EQ(8, optlen);
+}
+
+TEST_F(ipv6, hopopts_undersize_clamped)
+{
+	unsigned char buf[4] = {};
+	socklen_t optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_HOPOPTS, buf,
+				&optlen));
+	ASSERT_EQ(sizeof(buf), optlen);
+}
+
+/* With no header set the reply is empty and the call still succeeds. */
+TEST_F(ipv6, hopopts_absent)
+{
+	unsigned char buf[64] = {};
+	socklen_t optlen = sizeof(buf);
+	int fd;
+
+	fd = socket(AF_INET6, SOCK_DGRAM, 0);
+	if (fd < 0)
+		SKIP(return, "AF_INET6 dgram socket: %s", strerror(errno));
+
+	ASSERT_EQ(0, getsockopt(fd, SOL_IPV6, IPV6_HOPOPTS, buf, &optlen));
+	ASSERT_EQ(0, optlen);
+	close(fd);
+}
+
+/* IPV6_PATHMTU wants room for the whole struct ip6_mtuinfo. */
+TEST_F(ipv6, pathmtu_undersize)
+{
+	socklen_t optlen = 8;
+	char buf[8];
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IPV6, IPV6_PATHMTU, buf,
+				 &optlen));
+	ASSERT_EQ(EINVAL, errno);
+}
+
+TEST_F(ipv6, pktoptions_wrong_type)
+{
+	socklen_t optlen = 64;
+	char buf[64];
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IPV6, IPV6_2292PKTOPTIONS, buf,
+				 &optlen));
+	ASSERT_EQ(ENOPROTOOPT, errno);
+}
+
+TEST_F(ipv6, mroute_wrong_type)
+{
+	socklen_t optlen = sizeof(int);
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IPV6, MRT6_VERSION, &val,
+				 &optlen));
+	if (errno == ENOPROTOOPT)
+		SKIP(return, "CONFIG_IPV6_MROUTE disabled");
+	ASSERT_EQ(EOPNOTSUPP, errno);
+}
+
+TEST_F(ipv6, bad_optname)
+{
+	socklen_t optlen = sizeof(int);
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IPV6, 0x7fff, &val, &optlen));
+	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(int), optlen);
+}
+
 /* ---------- tls ---------- */
 
 FIXTURE(tls)

-- 
2.53.0-Meta

Re: [PATCH net-next 1/6] ipv6: reject a negative optlen in do_ipv6_getsockopt()

[Stanislav Fomichev]

On 09/25, Breno Leitao wrote:
> IPv4's do_ip_getsockopt() rejects a negative optlen right after reading
> it. do_ipv6_getsockopt() never has, and nothing downstream treats it as
> an error either: len is an int, but every consumer compares it unsigned,
> so -1 behaves as a huge value and each site clamps to its own reply
> size.
> 
> 	len = min_t(unsigned int, sizeof(int), len);
> 
> So getsockopt(fd, SOL_IPV6, IPV6_TCLASS, buf, &len) with len set to -1
> answers 4 bytes and reports 4, rather than failing.
> 
> This is a bug ready to bite us in the near future, let's get this fixed.
> 
> I've found this because testing the rest of the patch was returning
> inconsistency when optlen = -1.

If I can do getsockopt with len=-1 today and get 4 bytes back, isn't
that a uapi and we are gonna break someone?

Re: [PATCH net-next 1/6] ipv6: reject a negative optlen in do_ipv6_getsockopt()

[David Laight]

On Fri, 25 Sep 2026 12:02:22 -0700
Stanislav Fomichev <sdf.kernel@gmail.com> wrote:

> On 09/25, Breno Leitao wrote:
> > IPv4's do_ip_getsockopt() rejects a negative optlen right after reading
> > it. do_ipv6_getsockopt() never has, and nothing downstream treats it as
> > an error either: len is an int, but every consumer compares it unsigned,
> > so -1 behaves as a huge value and each site clamps to its own reply
> > size.
> > 
> > 	len = min_t(unsigned int, sizeof(int), len);
> > 
> > So getsockopt(fd, SOL_IPV6, IPV6_TCLASS, buf, &len) with len set to -1
> > answers 4 bytes and reports 4, rather than failing.
> > 
> > This is a bug ready to bite us in the near future, let's get this fixed.
> > 
> > I've found this because testing the rest of the patch was returning
> > inconsistency when optlen = -1.  
> 
> If I can do getsockopt with len=-1 today and get 4 bytes back, isn't
> that a uapi and we are gonna break someone?
> 

Treating negative values as 4 goes way back into the pre-historic annals,
And I agree that there could be code out there that fails to set a value
so passes 'dirty stack' and it always works because it never passed 0..3.

I suspect all the per-protocol code ought to be passed an unsigned 'len'
(and return back a possibly modified value for the wrapper code to give
to the user).
Then you have somewhere:
	/* Historic bug compatibility */
	ulen = len >= 0 ? len : 4;

David

Re: [PATCH net-next 1/6] ipv6: reject a negative optlen in do_ipv6_getsockopt()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message misdescribes how a negative optlen behaved
  before the patch.
- [Low] The commit message frames the change as a standalone bug fix
  ("the fix stands on its own here") for user-visible behaviour, but it…

--

ipv6: reject a negative optlen in do_ipv6_getsockopt()

This commit makes do_ipv6_getsockopt() return -EINVAL when the optlen
read from userspace is negative. IPv4's do_ip_getsockopt() already does
this.

> IPv4's do_ip_getsockopt() rejects a negative optlen right after reading
> it. do_ipv6_getsockopt() never has, and nothing downstream treats it as
> an error either: len is an int, but every consumer compares it unsigned,
> so -1 behaves as a huge value and each site clamps to its own reply
> size.

[Severity: Low]

Is this an accurate description of the old behaviour? Two cases in
do_ipv6_getsockopt() in net/ipv6/ipv6_sockglue.c don't seem to match it.

For MCAST_MSFILTER, ipv6_get_msfilter() and compat_ipv6_get_msfilter()
already use a signed compare, because size0 is a const int:

	const int size0 = offsetof(struct group_filter, gf_slist_flex);
	...
	if (len < size0)
		return -EINVAL;

So len = -1 already returned -EINVAL before this patch.

For IPV6_2292PKTOPTIONS on a SOCK_STREAM socket, len is never compared
or clamped:

		msg.msg_controllen = len;

msg_controllen is __kernel_size_t, so -1 becomes SIZE_MAX. That value
passes both size checks in put_cmsg():

  msg_controllen < sizeof(struct cmsghdr)
  msg_controllen < cmlen

As a result, every pending cmsg is written to optval with no limit from
the real buffer size. This covers the cmsgs from ip6_datagram_recv_ctl()
and from the sticky rxopt put_cmsg() calls. After that:

		len -= msg.msg_controllen;

reports the number of bytes written, not a clamped reply size.

The writes only land in the caller's own address space. The BPF kernel
sockptr path through sol_ipv6_sockopt() only allows IPV6_TCLASS,
IPV6_AUTOFLOWLABEL and IPV6_TRANSPARENT with *optlen == sizeof(int).
So this is a changelog problem only, and the code change covers both
paths.

Could the commit message mention the IPV6_2292PKTOPTIONS case, and drop
the claim that every consumer compares len unsigned?

> This is a bug ready to bite us in the near future, let's get this fixed.
>
> I've found this because testing the rest of the patch was returning
> inconsistency when optlen = -1.
>
> Later patches make the check unreachable, since sockopt_init_user() and
> sockptr_to_sockopt() both reject a negative length before the switch
> runs. Keep it as a guard; the fix stands on its own here.

[Severity: Low]

Is this meant as a standalone fix, or as preparatory hardening for the
series?

Userspace can see the change on the baseline. The IPv6 proto_ops have no
getsockopt_iter, so do_sock_getsockopt() takes the legacy
ops->getsockopt path and never reaches sockptr_to_sockopt():

do_sock_getsockopt()
  sock_common_getsockopt()
    udpv6_getsockopt()
      ipv6_getsockopt()
        do_ipv6_getsockopt()

Nothing on that path checks the sign of len. With len = -1,
getsockopt(udp6_fd, SOL_IPV6, IPV6_TCLASS, buf, &len) reaches:

	len = min_t(unsigned int, sizeof(int), len);

It then writes 4 bytes and returns 0. With this patch it returns -EINVAL.

If this is a fix, should it have a Fixes: tag and target net? The
unchecked len predates git history, so something like this would fit:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

If it is hardening for the net-next series, could "a bug ready to bite
us" and "the fix stands on its own here" be reworded?

Also, "the rest of the patch" and "Later patches make the check
unreachable" refer to the series. Once this is in git history, readers
won't have that context. Could those sentences be reworded too?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-sockopt_expand_out_v2-v1-0-c3ef2e3bb5c0%40debian.org

Re: [PATCH net-next 2/6] ipv6: mcast: convert ip6_mc_msfget() to sockopt_t

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the -EFAULT returns "still" leave the
  caller's optlen untouched.

--

ipv6: mcast: convert ip6_mc_msfget() to sockopt_t

This converts ip6_mc_msfget() and the MCAST_MSFILTER getsockopt helpers
from sockptr_t to sockopt_t. do_ipv6_getsockopt() builds the sockopt_t.
The source list is written through iter_out after sockopt_expand_out(),
and the callers then rewind to write the fixed header. The *optlen store
moves to do_ipv6_getsockopt() and happens only on success.

> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 5c6a0819a2aaf..1bdb3e001e4fe 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -922,48 +922,51 @@ static int ipv6_getsockopt_sticky(struct sock *sk, struct ipv6_txoptions *opt,

[ ... ]

>  	sockopt_lock_sock(sk);
> -	err = ip6_mc_msfget(sk, &gsf, optval, size0);
> +	err = ip6_mc_msfget(sk, &gsf, opt, size0);
>  	if (!err) {
>  		if (num > gsf.gf_numsrc)
>  			num = gsf.gf_numsrc;
>  		len = GROUP_FILTER_SIZE(num);
> -		if (copy_to_sockptr(optlen, &len, sizeof(int)) ||
> -		    copy_to_sockptr(optval, &gsf, size0))
> +		opt->optlen = len;
> +
> +		/* ip6_mc_msfget() consumed the whole reply; rewind to the
> +		 * fixed part.
> +		 */
> +		iov_iter_revert(&opt->iter_out, len);
> +		if (copy_to_iter(&gsf, size0, &opt->iter_out) != size0)
>  			err = -EFAULT;
>  	}
>  	sockopt_release_sock(sk);
>  	return err;
>  }

[ ... ]

> @@ -974,18 +977,22 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval,

[ ... ]

>  	len = GROUP_FILTER_SIZE(num) - (sizeof(gf)-sizeof(gf32));
> -	if (copy_to_sockptr(optlen, &len, sizeof(int)) ||
> -	    copy_to_sockptr_offset(optval, offsetof(struct compat_group_filter, gf_fmode),
> -				   &gf.gf_fmode, sizeof(gf32.gf_fmode)) ||
> -	    copy_to_sockptr_offset(optval, offsetof(struct compat_group_filter, gf_numsrc),
> -				   &gf.gf_numsrc, sizeof(gf32.gf_numsrc)))
> +	opt->optlen = len;
> +
> +	/* Rewind to gf_fmode, which gf_numsrc follows. */
> +	iov_iter_revert(&opt->iter_out,
> +			len - offsetof(struct compat_group_filter, gf_fmode));
> +	if (copy_to_iter(&gf.gf_fmode, sizeof(gf32.gf_fmode),
> +			 &opt->iter_out) != sizeof(gf32.gf_fmode) ||
> +	    copy_to_iter(&gf.gf_numsrc, sizeof(gf32.gf_numsrc),
> +			 &opt->iter_out) != sizeof(gf32.gf_numsrc))
>  		return -EFAULT;
>  	return 0;
>  }
> @@ -1006,9 +1013,23 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
>  		return -EINVAL;
>  	switch (optname) {
>  	case MCAST_MSFILTER:
> +	{
> +		struct kvec kvec;
> +		sockopt_t opt;
> +		int err;
> +
> +		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
> +		if (err)
> +			return err;
> +
>  		if (in_compat_syscall())
> -			return compat_ipv6_get_msfilter(sk, optval, optlen, len);
> -		return ipv6_get_msfilter(sk, optval, optlen, len);
> +			err = compat_ipv6_get_msfilter(sk, &opt);
> +		else
> +			err = ipv6_get_msfilter(sk, &opt);
> +		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
> +			err = -EFAULT;

[Severity: Low]

The commit message says:

    The *optlen store moves out to the call site, guarded by !err so the
    -EINVAL, -EADDRNOTAVAIL and -EFAULT returns still leave the caller's
    optlen word untouched.

Is "still" accurate for the -EFAULT that comes from writing the fixed
header?

Before this patch, ipv6_get_msfilter() stored optlen first and then wrote
the header:

    if (copy_to_sockptr(optlen, &len, sizeof(int)) ||
        copy_to_sockptr(optval, &gsf, size0))

compat_ipv6_get_msfilter() used the same order for gf_fmode and
gf_numsrc. So when the header write faulted, the caller got -EFAULT and
optlen had already been set to the reply length.

With this patch, the helpers write the header through iter_out, and
do_ipv6_getsockopt() only stores opt.optlen when !err. After a fault on
the header write, optlen is now left untouched.

The write order is also reversed. If optval is writable and optlen is
read-only, the header now gets written before the -EFAULT. The old code
never wrote the header in that case.

This seems to last only until a later patch in the series. In "ipv6:
convert do_ipv6_getsockopt() to sockopt_t", ipv6_getsockopt() writes
sopt.optlen back unconditionally. Both helpers set opt->optlen = len
before the header copy, so a header fault reports optlen = len again, as
the original code did.

Could the commit message describe this intermediate change instead of
saying the behavior is unchanged?

> +		return err;
> +	}
>  	case IPV6_2292PKTOPTIONS:
>  	{
>  		struct msghdr msg;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-sockopt_expand_out_v2-v1-0-c3ef2e3bb5c0%40debian.org

Re: [PATCH net-next 3/6] ipv4: igmp: convert ip_mc_gsfget() to sockopt_t

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] ip_get_mcast_msfilter() and compat_ip_get_mcast_msfilter()
  (net/ipv4/ip_sockglue.c) rewind the output iterator by passing the…
- [Low] The commit message says the optlen store is guarded by !err "so
  the -EINVAL, -EADDRNOTAVAIL and -EFAULT returns still leave the…

--

ipv4: igmp: convert ip_mc_gsfget() to sockopt_t

This converts ip_mc_gsfget() and its two MCAST_MSFILTER callers from
sockptr_t to sockopt_t. The callee now expands iter_out, advances over the
fixed part and writes the sources. The callers then rewind with
iov_iter_revert() and write the fixed header, and the *optlen store moves
to do_ip_getsockopt(), where it only happens on success.

> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index e06c1f48ecad6..1f452b6ea86e9 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -1442,45 +1442,46 @@ static bool getsockopt_needs_rtnl(int optname)

[ ... ]

>  	num = gsf.gf_numsrc;
> -	err = ip_mc_gsfget(sk, &gsf, optval,
> +	err = ip_mc_gsfget(sk, &gsf, opt,
>  			   offsetof(struct group_filter, gf_slist_flex));
>  	if (err)
>  		return err;
>  	if (gsf.gf_numsrc < num)
>  		num = gsf.gf_numsrc;
>  	gsf_size = GROUP_FILTER_SIZE(num);
> -	if (copy_to_sockptr(optlen, &gsf_size, sizeof(int)) ||
> -	    copy_to_sockptr(optval, &gsf, size0))
> +	opt->optlen = gsf_size;
> +
> +	/* ip_mc_gsfget() consumed the whole reply; rewind to the fixed part. */
> +	iov_iter_revert(&opt->iter_out, gsf_size);
> +	if (copy_to_iter(&gsf, size0, &opt->iter_out) != size0)
>  		return -EFAULT;

[Severity: Medium]
What happens here if gsf_size is larger than MAX_RW_COUNT?

iov_iter_revert() refuses large rewinds:

lib/iov_iter.c:iov_iter_revert() {
    ...
	if (WARN_ON(unroll > MAX_RW_COUNT))
		return;
    ...
}

MAX_RW_COUNT is (INT_MAX & PAGE_MASK), but sockopt_expand_out() only
rejects sizes above INT_MAX:

	if (size > INT_MAX)
		return -EINVAL;

That means ip_mc_gsfget() can expand iter_out to a size in the range
(MAX_RW_COUNT, INT_MAX] and then use all of it. For example, with
n = 0xffffe0 sources the native reply is 144 + n * 128 = 0x7ffff090. That
is above MAX_RW_COUNT for both 4K pages (0x7ffff000) and 64K pages
(0x7fff0000).

When that happens, the WARN_ON fires and the iterator is not rewound.

If optlen only covered the fixed part, iter_out is empty, copy_to_iter()
returns 0, and getsockopt fails with -EFAULT. The old code handled the
same request correctly.

If optlen was at least 144 bytes larger than the reply, no expansion
happens. The fixed header is then written right after the source list,
offset 0 is never written, and the call returns success with the caller's
own gf_fmode and gf_numsrc still in the buffer.

The compat path in compat_ip_get_mcast_msfilter() looks like it has the
same problem:

	iov_iter_revert(&opt->iter_out,
			len - offsetof(struct compat_group_filter, gf_fmode));

Its unroll is 8 + n * 128 (0x7ffff008 for the same n), so gf_fmode and
gf_numsrc are not rewritten at their offsets either.

Reaching this needs net.core.optmem_max and net.ipv4.igmp_max_msf raised.
Both are writable per-netns, so root inside a user+net namespace can set
them. It also needs about 16.7M sources installed with IP_MSFILTER and a
~2 GiB buffer.

The ~64 MiB kmallocs in memdup_sockptr() and sock_kmalloc() are more than
KMALLOC_MAX_SIZE allows on x86-64, but they fit on configs such as arm64
with 64K pages. The compat variant also needs AArch32 compat support.

The old copy_to_sockptr_offset() writes used size_t offsets and had no
such limit. This code is also unchanged at the end of the series. Both
revert calls are still the same after "ipv4: convert do_ip_getsockopt()
to sockopt_t".

Should the expansion be capped at MAX_RW_COUNT instead of INT_MAX?

[ ... ]

> @@ -1727,12 +1732,22 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
>  		goto out;
>  	}
>  	case MCAST_MSFILTER:
> +	{
> +		struct kvec kvec;
> +		sockopt_t opt;
> +
> +		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
> +		if (err)
> +			goto out;
> +
>  		if (in_compat_syscall())
> -			err = compat_ip_get_mcast_msfilter(sk, optval, optlen,
> -							   len);
> +			err = compat_ip_get_mcast_msfilter(sk, &opt);
>  		else
> -			err = ip_get_mcast_msfilter(sk, optval, optlen, len);
> +			err = ip_get_mcast_msfilter(sk, &opt);
> +		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
> +			err = -EFAULT;

[Severity: Low]
The commit message says:

    The *optlen store moves out to the call site, guarded by !err so the
    -EINVAL, -EADDRNOTAVAIL and -EFAULT returns still leave the caller's
    optlen word untouched.

Is that accurate for a fault on the fixed-header write? The old
ip_get_mcast_msfilter() wrote optlen first:

	if (copy_to_sockptr(optlen, &gsf_size, sizeof(int)) ||
	    copy_to_sockptr(optval, &gsf, size0))
		return -EFAULT;

So a fault on the header write returned -EFAULT with optlen already set to
the reply size. The old compat_ip_get_mcast_msfilter() used the same order
for gf_fmode and gf_numsrc.

Now the header is written by copy_to_iter() after iov_iter_revert(), and
optlen is only stored when err is zero. That -EFAULT now leaves optlen
unchanged. It can be hit with a read-only optval header, a writable optlen
in a separate page, and zero sources copied.

The order is also reversed when the optlen write itself faults. The header
is now written before -EFAULT is returned, and the old code never wrote it
in that case. If optlen aliases gf_numsrc, the value left in that word
changes from the source count to the reply length.

Later in the series, "ipv4: convert do_ip_getsockopt() to sockopt_t" makes
ip_getsockopt() write opt.optlen back unconditionally. That restores the
old optlen value on header-write faults, but optval is still written
before optlen.

Could the commit message be updated to describe this?

>  		goto out;
> +	}
>  	case IP_PROTOCOL:
>  		val = inet_sk(sk)->inet_num;
>  		break;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-sockopt_expand_out_v2-v1-0-c3ef2e3bb5c0%40debian.org

Re: [PATCH net-next 4/6] ipv4: convert do_ip_getsockopt() to sockopt_t

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The conversion quietly fixes a bug where getsockopt() could
  return a positive value, but the commit message does not mention it…

Pre-existing issues:
- [Low] ip_get_mcast_msfilter() and compat_ip_get_mcast_msfilter() in
  net/ipv4/ip_sockglue.c set opt->optlen (to gsf_size or len) before…

--

ipv4: convert do_ip_getsockopt() to sockopt_t

This patch moves do_ip_getsockopt() and ip_mroute_getsockopt() from a
sockptr_t optval/optlen pair to sockopt_t, and removes the IP_MSFILTER
and MCAST_MSFILTER bridges. ip_getsockopt() now builds the sockopt_t with
sockopt_init_user() and always writes optlen back to userspace.
sol_ip_sockopt() in net/core/filter.c builds a kernel sockopt_t with
sockptr_to_sockopt().

> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index 1f452b6ea86e9..2e78b11d294a9 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c

[ ... ]

> @@ -1620,16 +1618,15 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
>  		rcu_read_unlock();
>  
>  		if (opt->optlen == 0) {
> -			len = 0;
> -			return copy_to_sockptr(optlen, &len, sizeof(int));
> +			sopt->optlen = 0;
> +			return 0;
>  		}

[ ... ]

> @@ -1680,8 +1677,8 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
>  			int tos = READ_ONCE(inet->rcv_tos);
>  			put_cmsg(&msg, SOL_IP, IP_TOS, sizeof(tos), &tos);
>  		}
> -		len -= msg.msg_controllen;
> -		return copy_to_sockptr(optlen, &len, sizeof(int));
> +		sopt->optlen = len - msg.msg_controllen;
> +		return 0;
>  	}

[Severity: Low]
This isn't a bug, but the commit message doesn't mention that these two
branches also change the getsockopt() return value.

Before this patch, the empty IP_OPTIONS branch and the IP_PKTOPTIONS tail
both ended with:

    return copy_to_sockptr(optlen, &len, sizeof(int));

For a user optlen, copy_to_sockptr() returns the copy_to_user() count of
bytes not copied, not -EFAULT.

Take an optlen on a page that is readable but not writable (for example
PROT_READ). The earlier copy_from_sockptr(&len, optlen, ...) succeeds, the
write fails, and do_ip_getsockopt() returns +4. That value passes back
through this chain:

    do_sock_getsockopt()
      -> sock_common_getsockopt()
        -> udp_getsockopt() / tcp_getsockopt()
          -> ip_getsockopt()
            -> do_ip_getsockopt()

As a result, getsockopt(2) returns 4.

With this patch, both branches only set sopt->optlen, and the put_user()
in ip_getsockopt() returns -EFAULT. That looks correct.

Could this be mentioned in the commit message? Another option is a separate
patch with a Fixes: tag, so stable can pick it up.

[ ... ]

> @@ -1711,43 +1707,25 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,

[ ... ]

>  	case MCAST_MSFILTER:
> -	{
> -		struct kvec kvec;
> -		sockopt_t opt;
> -
> -		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
> -		if (err)
> -			goto out;
> -
>  		if (in_compat_syscall())
> -			err = compat_ip_get_mcast_msfilter(sk, &opt);
> +			err = compat_ip_get_mcast_msfilter(sk, sopt);
>  		else
> -			err = ip_get_mcast_msfilter(sk, &opt);
> -		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
> -			err = -EFAULT;
> +			err = ip_get_mcast_msfilter(sk, sopt);
>  		goto out;
> -	}

[Severity: Low]
This is a pre-existing issue, but it now shows up again. With the !err
guard gone and the unconditional write-back in ip_getsockopt():

    err = do_ip_getsockopt(sk, level, optname, &opt);
    if (put_user(opt.optlen, optlen))
        return -EFAULT;

MCAST_MSFILTER can return -EFAULT after it has already changed the user's
optlen. ip_get_mcast_msfilter() sets the length before its last copy:

net/ipv4/ip_sockglue.c:ip_get_mcast_msfilter() {
    ...
	opt->optlen = gsf_size;

	/* ip_mc_gsfget() consumed the whole reply; rewind to the fixed part. */
	iov_iter_revert(&opt->iter_out, gsf_size);
	if (copy_to_iter(&gsf, size0, &opt->iter_out) != size0)
		return -EFAULT;
    ...
}

compat_ip_get_mcast_msfilter() does the same thing: it sets
opt->optlen = len before the gf_fmode and gf_numsrc copy_to_iter() calls.

One trigger is an optval that is readable but not writable. The
copy_from_iter() of the header succeeds, then the header copy_to_iter()
fails.

The pre-series code already wrote optlen before the optval copy in these
helpers. So the end result matches the original behaviour, and only
reverses the !err guard from the previous patch, "ipv4: igmp: convert
ip_mc_gsfget() to sockopt_t". The IPv6 conversion later in the series
(ipv6_get_msfilter() and compat_ipv6_get_msfilter()) uses the same
set-then-copy order.

This patch does move ip_mc_msfget() to store its length last, though. So
IP_MSFILTER and MCAST_MSFILTER now act differently when the final copy
faults.

Should ip_get_mcast_msfilter() and compat_ip_get_mcast_msfilter() also
set opt->optlen after their copies, to match ip_mc_msfget()?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-sockopt_expand_out_v2-v1-0-c3ef2e3bb5c0%40debian.org

Re: [PATCH net-next 5/6] ipv6: convert do_ipv6_getsockopt() to sockopt_t

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the wrong thing about the order of
  checks.

--

ipv6: convert do_ipv6_getsockopt() to sockopt_t

This commit moves do_ipv6_getsockopt(), ip6_mroute_getsockopt() and
ipv6_getsockopt_sticky() from sockptr_t to sockopt_t, and drops the
MCAST_MSFILTER bridge. ipv6_getsockopt() now builds the sockopt_t with
sockopt_init_user() and always writes optlen back. The BPF caller
sol_ipv6_sockopt() builds it with sockptr_to_sockopt().

> MRT6_* is still dispatched before optlen is validated, so a negative
> optlen there wins over -EOPNOTSUPP.

[Severity: Low]
Is this description accurate? After this patch, ipv6_getsockopt() checks
optlen in sockopt_init_user(). That happens before do_ipv6_getsockopt()
reaches the ip6_mroute_opt() dispatch:

net/ipv6/ipv6_sockglue.c:ipv6_getsockopt() {
    ...
	err = sockopt_init_user(&sopt, optval, optlen);
	if (err)
		return err;

	err = do_ipv6_getsockopt(sk, level, optname, &sopt);
    ...
}

sockopt_init_user() rejects a bad optlen before anything else runs:

include/linux/net.h:sockopt_init_user() {
	if (get_user(len, optlen))
		return -EFAULT;
	if (len < 0)
		return -EINVAL;
    ...
}

sol_ipv6_sockopt() does the same thing through sockptr_to_sockopt().

So optlen is now validated before MRT6_* is dispatched, which is the
opposite of what the message says. The stated result is still right: a
negative optlen now wins over -EOPNOTSUPP. But that is because of the new
order, not the old one.

The IPv4 sibling commit "ipv4: convert do_ip_getsockopt() to sockopt_t"
describes the same change this way: "Validating optlen now precedes the
level check and the MRT_* dispatch". Could this message use similar
wording?

Could the message also list every errno whose precedence changed? Before
this patch, ip6_mroute_getsockopt() returned these without reading optlen:

  -EOPNOTSUPP (not a raw ICMPv6 socket)
  -ENOENT (no mr_table)
  -ENOPROTOOPT (unknown MRT6 name)

All three are now overridden by -EINVAL for a negative optlen and by
-EFAULT for an unreadable one. Under CONFIG_NETFILTER, the -ENOPROTOOPT
case used to fall through to nf_getsockopt().

Both callers now reject a negative optlen before calling in. Can the
len < 0 check in do_ipv6_getsockopt() and the olr < 0 check in
ip6_mroute_getsockopt() still be reached?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-sockopt_expand_out_v2-v1-0-c3ef2e3bb5c0%40debian.org

Re: [PATCH net-next 6/6] selftests: net: getsockopt_iter: cover ip and ipv6

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The ipv6 fixture does not test the SOL_IPV6 length handling that
  the commit message describes.
- [Low] FIXTURE_SETUP(ipv6) sets IPV6_HOPOPTS and SKIPs the whole fixture
  if that fails.
- [Low] ip.mroute_wrong_type and ipv6.mroute_wrong_type read ENOPROTOOPT
  as "CONFIG_IP_MROUTE / CONFIG_IPV6_MROUTE disabled" and SKIP.
- [Low] The "roundtrip" and clamp tests check only return codes and
  lengths, not the data that comes back.

--

selftests: net: getsockopt_iter: cover ip and ipv6

This adds ip and ipv6 fixtures to the getsockopt_iter selftest. They
check the returned length and errno across buffer sizes, the branches
that answer empty, an unknown optname, and the MRT options dispatched
before the main switch.

> diff --git a/tools/testing/selftests/net/getsockopt_iter.c b/tools/testing/selftests/net/getsockopt_iter.c
> index 6c2408df46123..32b6e0a50f1f9 100644
> --- a/tools/testing/selftests/net/getsockopt_iter.c
> +++ b/tools/testing/selftests/net/getsockopt_iter.c

[ ... ]

> @@ -492,6 +499,301 @@ TEST_F(rawv6, bad_optname)

[ ... ]

> +/* The MRT_* options are dispatched ahead of the rest of the switch and
> + * want a raw IGMP socket. Without CONFIG_IP_MROUTE they are not
> + * dispatched at all and the switch answers ENOPROTOOPT instead.
> + */
> +TEST_F(ip, mroute_wrong_type)
> +{
> +	socklen_t optlen = sizeof(int);
> +	int val;
> +
> +	ASSERT_EQ(-1, getsockopt(self->fd, SOL_IP, MRT_VERSION, &val,
> +				 &optlen));
> +	if (errno == ENOPROTOOPT)
> +		SKIP(return, "CONFIG_IP_MROUTE disabled");
> +	ASSERT_EQ(EOPNOTSUPP, errno);
> +}

[Severity: Low]
Can this SKIP hide a regression in the early MRT dispatch? A kernel with
CONFIG_IP_MROUTE=n returns ENOPROTOOPT here. A kernel where a conversion
dropped or reordered this dispatch returns the same errno:

net/ipv4/ip_sockglue.c:do_ip_getsockopt() {
    ...
	if (ip_mroute_opt(optname))
		return ip_mroute_getsockopt(sk, optname, sopt);
    ...
}

Without that dispatch, MRT_VERSION falls through to the default case and
gets -ENOPROTOOPT. ip_getsockopt() then skips the nf_getsockopt() fallback,
because !ip_mroute_opt(optname) is false. The test sees ENOPROTOOPT and
skips.

ipv6.mroute_wrong_type does the same for the ip6_mroute_opt() dispatch in
do_ipv6_getsockopt(). There, nf_getsockopt() also answers ENOPROTOOPT for an
unknown optname.

tools/testing/selftests/net/config sets CONFIG_IPV6_MROUTE=y. So on a CI
kernel, the IPv6 skip would hide a real regression.

Could the tests tell these two cases apart, rather than treating every
ENOPROTOOPT as a config skip?

[ ... ]

> +FIXTURE_SETUP(ipv6)
> +{
> +	/* an 8 byte hop-by-hop header, so the sticky options answer */
> +	static const unsigned char hopopt[8] = { 0, 0, 1, 4, 0, 0, 0, 0 };
> +	int hops = 42;
> +
> +	self->fd = socket(AF_INET6, SOCK_DGRAM, 0);
> +	if (self->fd < 0)
> +		SKIP(return, "AF_INET6 dgram socket: %s", strerror(errno));
> +
> +	if (setsockopt(self->fd, SOL_IPV6, IPV6_UNICAST_HOPS, &hops,
> +		       sizeof(hops)) < 0)
> +		SKIP(return, "set IPV6_UNICAST_HOPS: %s", strerror(errno));
> +
> +	if (setsockopt(self->fd, SOL_IPV6, IPV6_HOPOPTS, hopopt,
> +		       sizeof(hopopt)) < 0)
> +		SKIP(return, "set IPV6_HOPOPTS: %s", strerror(errno));
> +}

[Severity: Low]
Does this make the whole ipv6 fixture skip without CAP_NET_RAW? The
IPV6_HOPOPTS setsockopt is privileged:

net/ipv6/ipv6_sockglue.c:ipv6_set_opt_hdr() {
    ...
	/* hop-by-hop / destination options are privileged option */
	if (optname != IPV6_RTHDR && !sockopt_ns_capable(net->user_ns, CAP_NET_RAW))
		return -EPERM;
    ...
}

When that returns EPERM, all nine ipv6 tests skip. Six of them never use
the sticky header: hops_exact, hops_oversize_clamped, pathmtu_undersize,
pktoptions_wrong_type, mroute_wrong_type and bad_optname. hopopts_absent
opens its own socket, so it doesn't use the header either.

The ip fixture sets a router alert IP_OPTIONS, which needs no capability.
So the two fixtures behave differently under the same privileges.

Could the IPV6_HOPOPTS setup move into the hopopts tests that need it?

[ ... ]

> +TEST_F(ipv6, hops_exact)
> +{
> +	socklen_t optlen = sizeof(int);
> +	int val = 0;
> +
> +	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_UNICAST_HOPS, &val,
> +				&optlen));
> +	ASSERT_EQ(sizeof(int), optlen);
> +	ASSERT_EQ(42, val);
> +}
> +
> +TEST_F(ipv6, hops_oversize_clamped)
> +{
> +	socklen_t optlen = 64;
> +	char buf[64] = {};
> +
> +	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_UNICAST_HOPS, buf,
> +				&optlen));
> +	ASSERT_EQ(sizeof(int), optlen);
> +}

[Severity: Low]
The commit message says:

    SOL_IP answers a sub-int buffer with one byte where SOL_IPV6 clamps the
    int.

ip.ttl_single_byte covers the SOL_IP half. Is there an ipv6 test for the
other half? The ipv6 fixture only has hops_exact and hops_oversize_clamped.
Nothing matches ip.ttl_single_byte, ip.ttl_zero_len or ip.negative_optlen.

So no test runs the clamping tail of do_ipv6_getsockopt() with a sub-int
buffer:

net/ipv6/ipv6_sockglue.c:do_ipv6_getsockopt() {
    ...
	len = min_t(unsigned int, sizeof(int), len);
	sopt->optlen = len;
    ...
}

An earlier patch in this series, "ipv6: reject a negative optlen in
do_ipv6_getsockopt()", added a negative optlen rejection. It now lives in
sockopt_init_user(), called from ipv6_getsockopt(), and nothing here tests
it.

rawv6.hdrincl_undersize_clamped doesn't reach this path either.
rawv6_getsockopt() sends IPV6_HDRINCL to do_rawv6_getsockopt() instead.

Would it make sense to add sub-int, zero-length and negative optlen cases
for IPV6_UNICAST_HOPS?

> +
> +TEST_F(ipv6, hopopts_roundtrip)
> +{
> +	unsigned char buf[64] = {};
> +	socklen_t optlen = sizeof(buf);
> +
> +	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_HOPOPTS, buf,
> +				&optlen));
> +	ASSERT_EQ(8, optlen);
> +}
> +
> +TEST_F(ipv6, hopopts_undersize_clamped)
> +{
> +	unsigned char buf[4] = {};
> +	socklen_t optlen = sizeof(buf);
> +
> +	ASSERT_EQ(0, getsockopt(self->fd, SOL_IPV6, IPV6_HOPOPTS, buf,
> +				&optlen));
> +	ASSERT_EQ(sizeof(buf), optlen);
> +}

[Severity: Low]
Should these tests compare buf against hopopt[]? Both only check optlen.
ipv6_getsockopt_sticky() copies the data separately from returning the
length:

	len = min_t(unsigned int, len, ipv6_optlen(hdr));
	if (copy_to_iter(hdr, len, &sopt->iter_out) != len)
		return -EFAULT;
	return len;

A conversion that copied the wrong bytes, or skipped the copy entirely,
would still pass as long as the length was right.

The IPv4 tests have a similar gap. ip.options_roundtrip only checks
buf[0] == 0x94, and ignores the length byte and the value bytes of
ipopts[]. ip.options_undersize_clamped only checks the length. In both
cases the data comes from the copy_to_iter() of opt->__data in
do_ip_getsockopt().

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-sockopt_expand_out_v2-v1-0-c3ef2e3bb5c0%40debian.org