[PATCH net-next 0/3] tcp: convert the SOL_TCP getsockopt path to sockopt_t

[PATCH net-next 0/3] tcp: convert the SOL_TCP getsockopt path to sockopt_t

[Breno Leitao]

This series continues the migration to sockopt_t, as described in [1].
do_ip_getsockopt() and do_ipv6_getsockopt() moved to sockopt_t in [2];
this does the same for do_tcp_getsockopt(), which still takes a
sockptr_t pair.

Once this lands, the MPTCP and SCTP getsockopt paths come next. MPTCP
builds on this series: with do_tcp_getsockopt() on sockopt_t,
mptcp_getsockopt_first_sf_only() can pass its own sockopt_t to it on the
first subflow, instead of calling tcp_getsockopt() with the user
pointers.

Patch 1 converts the TCP-AO getters.
Patch 2 converts do_tcp_getsockopt() itself. tcp_getsockopt() keeps its
__user prototype and builds the sockopt_t once the level is SOL_TCP, as
ip_getsockopt() does. 
Patch 3 adds a tcp fixture to the getsockopt_iter selftest.

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

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (3):
      net/tcp-ao: convert the getsockopt helpers to sockopt_t
      tcp: convert do_tcp_getsockopt() to sockopt_t
      selftests: net: getsockopt_iter: cover tcp

 include/net/tcp.h                             |   3 +-
 include/net/tcp_ao.h                          |  13 +-
 net/core/filter.c                             |  30 +++-
 net/ipv4/tcp.c                                | 116 ++++++------
 net/ipv4/tcp_ao.c                             |  77 ++++----
 tools/testing/selftests/net/getsockopt_iter.c | 245 ++++++++++++++++++++++++++
 6 files changed, 372 insertions(+), 112 deletions(-)
---
base-commit: c88fd6989aea9237a60a4859139a265bfb7ddc0b
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git main
change-id: 20261109-getsockopt_phase7-9f14457eba34

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

[PATCH net-next 1/3] net/tcp-ao: convert the getsockopt helpers to sockopt_t

[Breno Leitao]

TCP_AO_GET_KEYS, TCP_AO_INFO and TCP_AO_REPAIR are read through
tcp_ao_get_mkts(), tcp_ao_get_sock_info() and tcp_ao_get_repair(),
reached from do_tcp_getsockopt() and from nowhere else. Convert them to
sockopt_t. While do_tcp_getsockopt() still carries a sockptr_t pair, it
calls them through tcp_ao_get_sockptr(), which builds the sockopt_t and
writes optlen back.

TCP_AO_GET_KEYS and TCP_AO_INFO read a versioned struct from optval.
Only a user buffer gets there, as BPF's sol_tcp_sockopt() takes no
TCP-AO option, so they keep reading it with copy_struct_from_user(),
now on iter_in.ubuf. A kernel buffer gets -EINVAL, as in
sockopt_expand_out().

TCP_AO_GET_KEYS is sized by a field in optval rather than by optlen:
optlen is the size of one entry, and nkeys says how many entries the
buffer holds. The match count then goes into the first entry's nkeys,
past optlen when the caller's struct is shorter than that field.

Userspace relies on this, so sockopt_expand_out() grows optval for the
reply, as for MCAST_MSFILTER. The growth is bounded by the keys on the
socket, not by nkeys alone: sockopt_expand_out() refuses a reply above
MAX_RW_COUNT, and a large nkeys would otherwise get -EINVAL.

iter_out only moves forward, so the entries are written in turn and the
bytes past each one are skipped, as before. The match count is written
last, through a copy of iter_out saved at the head of optval.

A reply that still needs more than MAX_RW_COUNT bytes now fails with
-EINVAL. With two keys on the socket, that takes entries of about 1 GiB.

TCP_AO_INFO and TCP_AO_REPAIR now write optlen back on success, with
the value they were given. TCP_AO_GET_KEYS stores optlen after nkeys
instead of before it, which only shows when one of the two writes
faults.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/net/tcp_ao.h | 13 ++++-----
 net/ipv4/tcp.c       | 28 +++++++++++++++++--
 net/ipv4/tcp_ao.c    | 77 ++++++++++++++++++++++++++++++----------------------
 3 files changed, 76 insertions(+), 42 deletions(-)

diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
index 9a2333e62e9940..0e1edd777a600a 100644
--- a/include/net/tcp_ao.h
+++ b/include/net/tcp_ao.h
@@ -210,9 +210,9 @@ void tcp_ao_calc_traffic_key(const struct tcp_ao_key *mkt, u8 *traffic_key,
 void tcp_ao_destroy_sock(struct sock *sk, bool twsk);
 void tcp_ao_time_wait(struct tcp_timewait_sock *tcptw, struct tcp_sock *tp);
 bool tcp_ao_ignore_icmp(const struct sock *sk, int family, int type, int code);
-int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen);
-int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen);
-int tcp_ao_get_repair(struct sock *sk, sockptr_t optval, sockptr_t optlen);
+int tcp_ao_get_mkts(struct sock *sk, sockopt_t *sopt);
+int tcp_ao_get_sock_info(struct sock *sk, sockopt_t *sopt);
+int tcp_ao_get_repair(struct sock *sk, sockopt_t *sopt);
 int tcp_ao_set_repair(struct sock *sk, sockptr_t optval, unsigned int optlen);
 enum skb_drop_reason tcp_inbound_ao_hash(struct sock *sk,
 			const struct sk_buff *skb, unsigned short int family,
@@ -334,18 +334,17 @@ static inline void tcp_ao_connect_init(struct sock *sk)
 {
 }
 
-static inline int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen)
+static inline int tcp_ao_get_mkts(struct sock *sk, sockopt_t *sopt)
 {
 	return -ENOPROTOOPT;
 }
 
-static inline int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen)
+static inline int tcp_ao_get_sock_info(struct sock *sk, sockopt_t *sopt)
 {
 	return -ENOPROTOOPT;
 }
 
-static inline int tcp_ao_get_repair(struct sock *sk,
-				    sockptr_t optval, sockptr_t optlen)
+static inline int tcp_ao_get_repair(struct sock *sk, sockopt_t *sopt)
 {
 	return -ENOPROTOOPT;
 }
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a1f89c1fa2ae51..552e8fe22e7e20 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4492,6 +4492,25 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk,
 	return stats;
 }
 
+/* Bridge the sockptr_t pair of do_tcp_getsockopt() to a TCP-AO getter. */
+static int tcp_ao_get_sockptr(struct sock *sk,
+			      int (*get)(struct sock *sk, sockopt_t *sopt),
+			      sockptr_t optval, sockptr_t optlen)
+{
+	struct kvec kvec;
+	sockopt_t sopt;
+	int err;
+
+	err = sockptr_to_sockopt(&sopt, optval, optlen, &kvec);
+	if (err)
+		return err;
+
+	err = get(sk, &sopt);
+	if (!err && copy_to_sockptr(optlen, &sopt.optlen, sizeof(int)))
+		err = -EFAULT;
+	return err;
+}
+
 int do_tcp_getsockopt(struct sock *sk, int level,
 		      int optname, sockptr_t optval, sockptr_t optlen)
 {
@@ -4830,16 +4849,19 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_AO_REPAIR:
 		if (!tcp_can_repair_sock(sk))
 			return -EPERM;
-		return tcp_ao_get_repair(sk, optval, optlen);
+		return tcp_ao_get_sockptr(sk, tcp_ao_get_repair,
+					  optval, optlen);
 	case TCP_AO_GET_KEYS:
 	case TCP_AO_INFO: {
 		int err;
 
 		sockopt_lock_sock(sk);
 		if (optname == TCP_AO_GET_KEYS)
-			err = tcp_ao_get_mkts(sk, optval, optlen);
+			err = tcp_ao_get_sockptr(sk, tcp_ao_get_mkts,
+						 optval, optlen);
 		else
-			err = tcp_ao_get_sock_info(sk, optval, optlen);
+			err = tcp_ao_get_sockptr(sk, tcp_ao_get_sock_info,
+						 optval, optlen);
 		sockopt_release_sock(sk);
 
 		return err;
diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index bb7bbc20ba3fad..af33042478f9a8 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -2054,14 +2054,11 @@ int tcp_v4_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen)
 	return tcp_parse_ao(sk, cmd, AF_INET, optval, optlen);
 }
 
-/* tcp_ao_copy_mkts_to_user(ao_info, optval, optlen)
+/* tcp_ao_copy_mkts_to_user(ao_info, sopt)
  *
  * @ao_info:	struct tcp_ao_info on the socket that
  *		socket getsockopt(TCP_AO_GET_KEYS) is executed on
- * @optval:	pointer to array of tcp_ao_getsockopt structures in user space.
- *		Must be != NULL.
- * @optlen:	pointer to size of tcp_ao_getsockopt structure.
- *		Must be != NULL.
+ * @sopt:	optval and optlen of that getsockopt(), described below.
  *
  * Return value: 0 on success, a negative error number otherwise.
  *
@@ -2091,29 +2088,31 @@ int tcp_v4_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen)
  */
 static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
 				    struct tcp_ao_info *ao_info,
-				    sockptr_t optval, sockptr_t optlen)
+				    sockopt_t *sopt)
 {
 	struct tcp_ao_getsockopt opt_in, opt_out;
 	struct tcp_ao_key *key, *current_key;
 	bool do_address_matching = true;
 	union tcp_ao_addr *addr = NULL;
 	int err, l3index, user_len;
+	unsigned int nr_keys = 0;
 	unsigned int max_keys;	/* maximum number of keys to copy to user */
-	size_t out_offset = 0;
 	size_t bytes_to_write;	/* number of bytes to write to user level */
+	struct iov_iter head;
 	u32 matched_keys;	/* keys from ao_info matched so far */
-	int optlen_out;
+	size_t reply_len;
 	__be16 port = 0;
 
-	if (copy_from_sockptr(&user_len, optlen, sizeof(int)))
-		return -EFAULT;
+	user_len = sopt->optlen;
 
 	if (user_len <= 0)
 		return -EINVAL;
 
 	memset(&opt_in, 0, sizeof(struct tcp_ao_getsockopt));
-	err = copy_struct_from_sockptr(&opt_in, sizeof(opt_in),
-				       optval, user_len);
+	if (WARN_ON_ONCE(!iter_is_ubuf(&sopt->iter_in)))
+		return -EINVAL;
+	err = copy_struct_from_user(&opt_in, sizeof(opt_in), sopt->iter_in.ubuf,
+				    user_len);
 	if (err < 0)
 		return err;
 
@@ -2214,6 +2213,21 @@ static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
 	}
 
 	bytes_to_write = min_t(int, user_len, sizeof(struct tcp_ao_getsockopt));
+
+	/* The reply is up to nkeys entries, user_len apart, with the match
+	 * count in the first entry's nkeys, so it can outgrow optlen. Size it
+	 * by the keys on the socket rather than by the caller's nkeys.
+	 */
+	hlist_for_each_entry_rcu(key, &ao_info->head, node,
+				 lockdep_sock_is_held(sk))
+		nr_keys++;
+	reply_len = max(array_size(min(max_keys, nr_keys), user_len),
+			offsetofend(struct tcp_ao_getsockopt, nkeys));
+	err = sockopt_expand_out(sopt, reply_len);
+	if (err)
+		return err;
+	head = sopt->iter_out;
+
 	matched_keys = 0;
 	/* May change in RX, while we're dumping, pre-fetch it */
 	current_key = READ_ONCE(ao_info->current_key);
@@ -2275,25 +2289,22 @@ static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
 			strscpy(opt_out.alg_name, tcp_ao_algos[key->algo].name);
 
 		/* Copy key to user */
-		if (copy_to_sockptr_offset(optval, out_offset,
-					   &opt_out, bytes_to_write))
+		if (copy_to_iter(&opt_out, bytes_to_write, &sopt->iter_out) !=
+		    bytes_to_write)
 			return -EFAULT;
-		out_offset += user_len;
+		iov_iter_advance(&sopt->iter_out, user_len - bytes_to_write);
 	}
 
-	optlen_out = (int)sizeof(struct tcp_ao_getsockopt);
-	if (copy_to_sockptr(optlen, &optlen_out, sizeof(int)))
-		return -EFAULT;
+	sopt->optlen = sizeof(struct tcp_ao_getsockopt);
 
-	out_offset = offsetof(struct tcp_ao_getsockopt, nkeys);
-	if (copy_to_sockptr_offset(optval, out_offset,
-				   &matched_keys, sizeof(u32)))
+	iov_iter_advance(&head, offsetof(struct tcp_ao_getsockopt, nkeys));
+	if (copy_to_iter(&matched_keys, sizeof(u32), &head) != sizeof(u32))
 		return -EFAULT;
 
 	return 0;
 }
 
-int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen)
+int tcp_ao_get_mkts(struct sock *sk, sockopt_t *sopt)
 {
 	struct tcp_ao_info *ao_info;
 
@@ -2303,18 +2314,17 @@ int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen)
 	if (!ao_info)
 		return -ENOENT;
 
-	return tcp_ao_copy_mkts_to_user(sk, ao_info, optval, optlen);
+	return tcp_ao_copy_mkts_to_user(sk, ao_info, sopt);
 }
 
-int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen)
+int tcp_ao_get_sock_info(struct sock *sk, sockopt_t *sopt)
 {
 	struct tcp_ao_info_opt out, in = {};
 	struct tcp_ao_key *current_key;
 	struct tcp_ao_info *ao;
 	int err, len;
 
-	if (copy_from_sockptr(&len, optlen, sizeof(int)))
-		return -EFAULT;
+	len = sopt->optlen;
 
 	if (len <= 0)
 		return -EINVAL;
@@ -2323,7 +2333,9 @@ int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen)
 	 * that may be needed to extend (struct tcp_ao_info_opt) and
 	 * what getsockopt() provides in future.
 	 */
-	err = copy_struct_from_sockptr(&in, sizeof(in), optval, len);
+	if (WARN_ON_ONCE(!iter_is_ubuf(&sopt->iter_in)))
+		return -EINVAL;
+	err = copy_struct_from_user(&in, sizeof(in), sopt->iter_in.ubuf, len);
 	if (err)
 		return err;
 
@@ -2355,7 +2367,8 @@ int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen)
 		out.rnext = ao->rnext_key->rcvid;
 	}
 
-	if (copy_to_sockptr(optval, &out, min_t(int, len, sizeof(out))))
+	len = min_t(int, len, sizeof(out));
+	if (copy_to_iter(&out, len, &sopt->iter_out) != len)
 		return -EFAULT;
 
 	return 0;
@@ -2396,15 +2409,14 @@ int tcp_ao_set_repair(struct sock *sk, sockptr_t optval, unsigned int optlen)
 	return 0;
 }
 
-int tcp_ao_get_repair(struct sock *sk, sockptr_t optval, sockptr_t optlen)
+int tcp_ao_get_repair(struct sock *sk, sockopt_t *sopt)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct tcp_ao_repair opt;
 	struct tcp_ao_info *ao;
 	int len;
 
-	if (copy_from_sockptr(&len, optlen, sizeof(int)))
-		return -EFAULT;
+	len = sopt->optlen;
 
 	if (len <= 0)
 		return -EINVAL;
@@ -2425,7 +2437,8 @@ int tcp_ao_get_repair(struct sock *sk, sockptr_t optval, sockptr_t optlen)
 	opt.rcv_sne	= READ_ONCE(ao->rcv_sne);
 	rcu_read_unlock();
 
-	if (copy_to_sockptr(optval, &opt, min_t(int, len, sizeof(opt))))
+	len = min_t(int, len, sizeof(opt));
+	if (copy_to_iter(&opt, len, &sopt->iter_out) != len)
 		return -EFAULT;
 	return 0;
 }

-- 
2.53.0-Meta

Re: [PATCH net-next 1/3] net/tcp-ao: convert the getsockopt helpers to sockopt_t

[Stanislav Fomichev]

On 11/12, Breno Leitao wrote:
> TCP_AO_GET_KEYS, TCP_AO_INFO and TCP_AO_REPAIR are read through
> tcp_ao_get_mkts(), tcp_ao_get_sock_info() and tcp_ao_get_repair(),
> reached from do_tcp_getsockopt() and from nowhere else. Convert them to
> sockopt_t. While do_tcp_getsockopt() still carries a sockptr_t pair, it
> calls them through tcp_ao_get_sockptr(), which builds the sockopt_t and
> writes optlen back.
> 
> TCP_AO_GET_KEYS and TCP_AO_INFO read a versioned struct from optval.
> Only a user buffer gets there, as BPF's sol_tcp_sockopt() takes no
> TCP-AO option, so they keep reading it with copy_struct_from_user(),
> now on iter_in.ubuf. A kernel buffer gets -EINVAL, as in
> sockopt_expand_out().
> 
> TCP_AO_GET_KEYS is sized by a field in optval rather than by optlen:
> optlen is the size of one entry, and nkeys says how many entries the
> buffer holds. The match count then goes into the first entry's nkeys,
> past optlen when the caller's struct is shorter than that field.
> 
> Userspace relies on this, so sockopt_expand_out() grows optval for the
> reply, as for MCAST_MSFILTER. The growth is bounded by the keys on the
> socket, not by nkeys alone: sockopt_expand_out() refuses a reply above
> MAX_RW_COUNT, and a large nkeys would otherwise get -EINVAL.
> 
> iter_out only moves forward, so the entries are written in turn and the
> bytes past each one are skipped, as before. The match count is written
> last, through a copy of iter_out saved at the head of optval.
> 
> A reply that still needs more than MAX_RW_COUNT bytes now fails with
> -EINVAL. With two keys on the socket, that takes entries of about 1 GiB.
> 
> TCP_AO_INFO and TCP_AO_REPAIR now write optlen back on success, with
> the value they were given. TCP_AO_GET_KEYS stores optlen after nkeys
> instead of before it, which only shows when one of the two writes
> faults.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  include/net/tcp_ao.h | 13 ++++-----
>  net/ipv4/tcp.c       | 28 +++++++++++++++++--
>  net/ipv4/tcp_ao.c    | 77 ++++++++++++++++++++++++++++++----------------------
>  3 files changed, 76 insertions(+), 42 deletions(-)
> 
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index 9a2333e62e9940..0e1edd777a600a 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -210,9 +210,9 @@ void tcp_ao_calc_traffic_key(const struct tcp_ao_key *mkt, u8 *traffic_key,
>  void tcp_ao_destroy_sock(struct sock *sk, bool twsk);
>  void tcp_ao_time_wait(struct tcp_timewait_sock *tcptw, struct tcp_sock *tp);
>  bool tcp_ao_ignore_icmp(const struct sock *sk, int family, int type, int code);
> -int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen);
> -int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen);
> -int tcp_ao_get_repair(struct sock *sk, sockptr_t optval, sockptr_t optlen);
> +int tcp_ao_get_mkts(struct sock *sk, sockopt_t *sopt);
> +int tcp_ao_get_sock_info(struct sock *sk, sockopt_t *sopt);
> +int tcp_ao_get_repair(struct sock *sk, sockopt_t *sopt);
>  int tcp_ao_set_repair(struct sock *sk, sockptr_t optval, unsigned int optlen);
>  enum skb_drop_reason tcp_inbound_ao_hash(struct sock *sk,
>  			const struct sk_buff *skb, unsigned short int family,
> @@ -334,18 +334,17 @@ static inline void tcp_ao_connect_init(struct sock *sk)
>  {
>  }
>  
> -static inline int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen)
> +static inline int tcp_ao_get_mkts(struct sock *sk, sockopt_t *sopt)
>  {
>  	return -ENOPROTOOPT;
>  }
>  
> -static inline int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen)
> +static inline int tcp_ao_get_sock_info(struct sock *sk, sockopt_t *sopt)
>  {
>  	return -ENOPROTOOPT;
>  }
>  
> -static inline int tcp_ao_get_repair(struct sock *sk,
> -				    sockptr_t optval, sockptr_t optlen)
> +static inline int tcp_ao_get_repair(struct sock *sk, sockopt_t *sopt)
>  {
>  	return -ENOPROTOOPT;
>  }
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index a1f89c1fa2ae51..552e8fe22e7e20 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4492,6 +4492,25 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk,
>  	return stats;
>  }
>  
> +/* Bridge the sockptr_t pair of do_tcp_getsockopt() to a TCP-AO getter. */
> +static int tcp_ao_get_sockptr(struct sock *sk,
> +			      int (*get)(struct sock *sk, sockopt_t *sopt),
> +			      sockptr_t optval, sockptr_t optlen)
> +{
> +	struct kvec kvec;
> +	sockopt_t sopt;
> +	int err;
> +
> +	err = sockptr_to_sockopt(&sopt, optval, optlen, &kvec);
> +	if (err)
> +		return err;
> +
> +	err = get(sk, &sopt);
> +	if (!err && copy_to_sockptr(optlen, &sopt.optlen, sizeof(int)))
> +		err = -EFAULT;
> +	return err;
> +}
> +
>  int do_tcp_getsockopt(struct sock *sk, int level,
>  		      int optname, sockptr_t optval, sockptr_t optlen)
>  {
> @@ -4830,16 +4849,19 @@ int do_tcp_getsockopt(struct sock *sk, int level,
>  	case TCP_AO_REPAIR:
>  		if (!tcp_can_repair_sock(sk))
>  			return -EPERM;
> -		return tcp_ao_get_repair(sk, optval, optlen);
> +		return tcp_ao_get_sockptr(sk, tcp_ao_get_repair,
> +					  optval, optlen);
>  	case TCP_AO_GET_KEYS:
>  	case TCP_AO_INFO: {
>  		int err;
>  
>  		sockopt_lock_sock(sk);
>  		if (optname == TCP_AO_GET_KEYS)
> -			err = tcp_ao_get_mkts(sk, optval, optlen);
> +			err = tcp_ao_get_sockptr(sk, tcp_ao_get_mkts,
> +						 optval, optlen);
>  		else
> -			err = tcp_ao_get_sock_info(sk, optval, optlen);
> +			err = tcp_ao_get_sockptr(sk, tcp_ao_get_sock_info,
> +						 optval, optlen);
>  		sockopt_release_sock(sk);
>  
>  		return err;
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index bb7bbc20ba3fad..af33042478f9a8 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -2054,14 +2054,11 @@ int tcp_v4_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen)
>  	return tcp_parse_ao(sk, cmd, AF_INET, optval, optlen);
>  }
>  
> -/* tcp_ao_copy_mkts_to_user(ao_info, optval, optlen)
> +/* tcp_ao_copy_mkts_to_user(ao_info, sopt)
>   *
>   * @ao_info:	struct tcp_ao_info on the socket that
>   *		socket getsockopt(TCP_AO_GET_KEYS) is executed on
> - * @optval:	pointer to array of tcp_ao_getsockopt structures in user space.
> - *		Must be != NULL.
> - * @optlen:	pointer to size of tcp_ao_getsockopt structure.
> - *		Must be != NULL.
> + * @sopt:	optval and optlen of that getsockopt(), described below.
>   *
>   * Return value: 0 on success, a negative error number otherwise.
>   *
> @@ -2091,29 +2088,31 @@ int tcp_v4_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen)
>   */
>  static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
>  				    struct tcp_ao_info *ao_info,
> -				    sockptr_t optval, sockptr_t optlen)
> +				    sockopt_t *sopt)
>  {
>  	struct tcp_ao_getsockopt opt_in, opt_out;
>  	struct tcp_ao_key *key, *current_key;
>  	bool do_address_matching = true;
>  	union tcp_ao_addr *addr = NULL;
>  	int err, l3index, user_len;
> +	unsigned int nr_keys = 0;
>  	unsigned int max_keys;	/* maximum number of keys to copy to user */
> -	size_t out_offset = 0;
>  	size_t bytes_to_write;	/* number of bytes to write to user level */
> +	struct iov_iter head;
>  	u32 matched_keys;	/* keys from ao_info matched so far */
> -	int optlen_out;
> +	size_t reply_len;
>  	__be16 port = 0;
>  
> -	if (copy_from_sockptr(&user_len, optlen, sizeof(int)))
> -		return -EFAULT;
> +	user_len = sopt->optlen;
>  
>  	if (user_len <= 0)
>  		return -EINVAL;
>  
>  	memset(&opt_in, 0, sizeof(struct tcp_ao_getsockopt));
> -	err = copy_struct_from_sockptr(&opt_in, sizeof(opt_in),
> -				       optval, user_len);
> +	if (WARN_ON_ONCE(!iter_is_ubuf(&sopt->iter_in)))
> +		return -EINVAL;
> +	err = copy_struct_from_user(&opt_in, sizeof(opt_in), sopt->iter_in.ubuf,
> +				    user_len);
>  	if (err < 0)
>  		return err;
>  
> @@ -2214,6 +2213,21 @@ static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
>  	}
>  
>  	bytes_to_write = min_t(int, user_len, sizeof(struct tcp_ao_getsockopt));
> +

[..]

> +	/* The reply is up to nkeys entries, user_len apart, with the match
> +	 * count in the first entry's nkeys, so it can outgrow optlen. Size it
> +	 * by the keys on the socket rather than by the caller's nkeys.
> +	 */
> +	hlist_for_each_entry_rcu(key, &ao_info->head, node,
> +				 lockdep_sock_is_held(sk))
> +		nr_keys++;
> +	reply_len = max(array_size(min(max_keys, nr_keys), user_len),
> +			offsetofend(struct tcp_ao_getsockopt, nkeys));
> +	err = sockopt_expand_out(sopt, reply_len);
> +	if (err)
> +		return err;
> +	head = sopt->iter_out;

Is that faster / cleaner / more preferable than calling sockopt_expand_out
on every iteration hlist_for_each_entry_rcu below?

[PATCH net-next 2/3] tcp: convert do_tcp_getsockopt() to sockopt_t

[Breno Leitao]

Convert the SOL_TCP switch to sockopt_t. The TCP-AO helpers already
take one, so they get it directly and the tcp_ao_get_sockptr() bridge
goes away.

The options that re-read optlen to get past the sizeof(int) clamp at
the top take it from sopt->optlen instead, so optlen is read once.

Only a user buffer reaches TCP_ZEROCOPY_RECEIVE: BPF's sol_tcp_sockopt()
does not take it, and it maps pages into the caller's mm anyway. It
keeps checking the bytes past its struct with check_zeroed_user(), now
on iter_in.ubuf, and a kernel buffer gets -EINVAL.

tcp_getsockopt() builds the sockopt_t with sockopt_init_user() once the
level is known to be SOL_TCP, and writes optlen back unconditionally,
because TCP_SAVED_SYN reports the length it needs and then fails with
-EINVAL. This mirrors ip_getsockopt().

Every SOL_TCP getsockopt() now writes optlen, including the ones that
did not before, such as TCP_REPAIR_WINDOW, TCP_ZEROCOPY_RECEIVE and the
error returns. An unwritable optlen word therefore turns them into
-EFAULT.

The others stored optlen before optval, so an unwritable optlen word
stopped optval from being written. Now optval is written first.

sol_tcp_sockopt() and sol_tcp_sockopt_congestion() reach
do_tcp_getsockopt() through sol_tcp_getsockopt(), which builds the
sockopt_t with sockptr_to_sockopt() and stores optlen back, as
sol_ip_sockopt() does.

copy_to_iter() runs the hardened usercopy check on its source, which
copy_to_sockptr() skips for a kernel buffer. BPF programs call
bpf_getsockopt() from the receive softirq too. On the vmapped IRQ
stack, the check then takes the lock in find_vmap_area(), which is not
softirq safe, and lockdep reports an inconsistent lock state.

sol_tcp_getsockopt() therefore calls do_tcp_getsockopt() with page
faults disabled, which makes check_heap_object() skip that lookup.
Commit d319f344561d ("mm: Fix copy_from_user_nofault().") added that
guard for BPF and kprobes.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/net/tcp.h |   3 +-
 net/core/filter.c |  30 +++++++++---
 net/ipv4/tcp.c    | 138 ++++++++++++++++++++----------------------------------
 3 files changed, 76 insertions(+), 95 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index f265ae06bb558a..8e936770fc97c6 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -498,8 +498,7 @@ void tcp_init_sock(struct sock *sk);
 void tcp_init_transfer(struct sock *sk, int bpf_op, struct sk_buff *skb);
 __poll_t tcp_poll(struct file *file, struct socket *sock,
 		      struct poll_table_struct *wait);
-int do_tcp_getsockopt(struct sock *sk, int level,
-		      int optname, sockptr_t optval, sockptr_t optlen);
+int do_tcp_getsockopt(struct sock *sk, int level, int optname, sockopt_t *sopt);
 int tcp_getsockopt(struct sock *sk, int level, int optname,
 		   char __user *optval, int __user *optlen);
 bool tcp_bpf_bypass_getsockopt(int level, int optname);
diff --git a/net/core/filter.c b/net/core/filter.c
index 9b8d5191c4ba27..0d5afe1c5606b1 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5686,6 +5686,28 @@ static int bpf_sol_tcp_setsockopt(struct sock *sk, int optname,
 	return 0;
 }
 
+static int sol_tcp_getsockopt(struct sock *sk, int optname, char *optval,
+			      int *optlen)
+{
+	struct kvec kvec;
+	sockopt_t opt;
+	int err;
+
+	err = sockptr_to_sockopt(&opt, KERNEL_SOCKPTR(optval),
+				 KERNEL_SOCKPTR(optlen), &kvec);
+	if (err)
+		return err;
+
+	/* BPF may call this in softirq: with page faults disabled, the
+	 * usercopy check skips find_vmap_area() and its lock.
+	 */
+	pagefault_disable();
+	err = do_tcp_getsockopt(sk, SOL_TCP, optname, &opt);
+	pagefault_enable();
+	*optlen = opt.optlen;
+	return err;
+}
+
 static int sol_tcp_sockopt_congestion(struct sock *sk, char *optval,
 				      int *optlen, bool getopt)
 {
@@ -5700,9 +5722,7 @@ static int sol_tcp_sockopt_congestion(struct sock *sk, char *optval,
 			return -EINVAL;
 		/* BPF expects NULL-terminated tcp-cc string */
 		optval[--(*optlen)] = '\0';
-		return do_tcp_getsockopt(sk, SOL_TCP, TCP_CONGESTION,
-					 KERNEL_SOCKPTR(optval),
-					 KERNEL_SOCKPTR(optlen));
+		return sol_tcp_getsockopt(sk, TCP_CONGESTION, optval, optlen);
 	}
 
 	/* "cdg" is the only cc that alloc a ptr
@@ -5788,9 +5808,7 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
 			return 0;
 		}
 
-		return do_tcp_getsockopt(sk, SOL_TCP, optname,
-					 KERNEL_SOCKPTR(optval),
-					 KERNEL_SOCKPTR(optlen));
+		return sol_tcp_getsockopt(sk, optname, optval, optlen);
 	}
 
 	return do_tcp_setsockopt(sk, SOL_TCP, optname,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 552e8fe22e7e20..de287845e6c0dc 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4492,27 +4492,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk,
 	return stats;
 }
 
-/* Bridge the sockptr_t pair of do_tcp_getsockopt() to a TCP-AO getter. */
-static int tcp_ao_get_sockptr(struct sock *sk,
-			      int (*get)(struct sock *sk, sockopt_t *sopt),
-			      sockptr_t optval, sockptr_t optlen)
-{
-	struct kvec kvec;
-	sockopt_t sopt;
-	int err;
-
-	err = sockptr_to_sockopt(&sopt, optval, optlen, &kvec);
-	if (err)
-		return err;
-
-	err = get(sk, &sopt);
-	if (!err && copy_to_sockptr(optlen, &sopt.optlen, sizeof(int)))
-		err = -EFAULT;
-	return err;
-}
-
-int do_tcp_getsockopt(struct sock *sk, int level,
-		      int optname, sockptr_t optval, sockptr_t optlen)
+int do_tcp_getsockopt(struct sock *sk, int level, int optname, sockopt_t *sopt)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -4520,8 +4500,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	int user_mss;
 	int val, len;
 
-	if (copy_from_sockptr(&len, optlen, sizeof(int)))
-		return -EFAULT;
+	len = sopt->optlen;
 
 	if (len < 0)
 		return -EINVAL;
@@ -4573,15 +4552,13 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_INFO: {
 		struct tcp_info info;
 
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 
 		tcp_get_info(sk, &info);
 
 		len = min_t(unsigned int, len, sizeof(info));
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &info, len))
+		sopt->optlen = len;
+		if (copy_to_iter(&info, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -4591,8 +4568,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		size_t sz = 0;
 		int attr;
 
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 
 		rcu_read_lock();
 		ca_ops = READ_ONCE(icsk->icsk_ca_ops);
@@ -4601,9 +4577,8 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		rcu_read_unlock();
 
 		len = min_t(unsigned int, len, sz);
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, &info, len))
+		sopt->optlen = len;
+		if (copy_to_iter(&info, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -4614,34 +4589,29 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_CONGESTION: {
 		char ca_name[TCP_CA_NAME_MAX] = {};
 
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 		len = min_t(unsigned int, len, TCP_CA_NAME_MAX);
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
+		sopt->optlen = len;
 
 		rcu_read_lock();
 		memcpy(ca_name, READ_ONCE(icsk->icsk_ca_ops)->name,
 		       sizeof(ca_name));
 		rcu_read_unlock();
 
-		if (copy_to_sockptr(optval, ca_name, len))
+		if (copy_to_iter(ca_name, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
 	case TCP_ULP:
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 		len = min_t(unsigned int, len, TCP_ULP_NAME_MAX);
 		if (!icsk->icsk_ulp_ops) {
-			len = 0;
-			if (copy_to_sockptr(optlen, &len, sizeof(int)))
-				return -EFAULT;
+			sopt->optlen = 0;
 			return 0;
 		}
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, icsk->icsk_ulp_ops->name, len))
+		sopt->optlen = len;
+		if (copy_to_iter(icsk->icsk_ulp_ops->name, len,
+				 &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 
@@ -4649,15 +4619,13 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		u64 key[TCP_FASTOPEN_KEY_BUF_LENGTH / sizeof(u64)];
 		unsigned int key_len;
 
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 
 		key_len = tcp_fastopen_get_cipher(net, icsk, key) *
 				TCP_FASTOPEN_KEY_LENGTH;
 		len = min_t(unsigned int, len, key_len);
-		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-			return -EFAULT;
-		if (copy_to_sockptr(optval, key, len))
+		sopt->optlen = len;
+		if (copy_to_iter(key, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -4683,8 +4651,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_REPAIR_WINDOW: {
 		struct tcp_repair_window opt;
 
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 
 		if (len != sizeof(opt))
 			return -EINVAL;
@@ -4698,7 +4665,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		opt.rcv_wnd	= tp->rcv_wnd;
 		opt.rcv_wup	= tp->rcv_wup;
 
-		if (copy_to_sockptr(optval, &opt, len))
+		if (copy_to_iter(&opt, len, &sopt->iter_out) != len)
 			return -EFAULT;
 		return 0;
 	}
@@ -4748,26 +4715,20 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		val = tp->save_syn;
 		break;
 	case TCP_SAVED_SYN: {
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 
 		sockopt_lock_sock(sk);
 		if (tp->saved_syn) {
 			if (len < tcp_saved_syn_len(tp->saved_syn)) {
 				len = tcp_saved_syn_len(tp->saved_syn);
-				if (copy_to_sockptr(optlen, &len, sizeof(int))) {
-					sockopt_release_sock(sk);
-					return -EFAULT;
-				}
+				sopt->optlen = len;
 				sockopt_release_sock(sk);
 				return -EINVAL;
 			}
 			len = tcp_saved_syn_len(tp->saved_syn);
-			if (copy_to_sockptr(optlen, &len, sizeof(int))) {
-				sockopt_release_sock(sk);
-				return -EFAULT;
-			}
-			if (copy_to_sockptr(optval, tp->saved_syn->data, len)) {
+			sopt->optlen = len;
+			if (copy_to_iter(tp->saved_syn->data, len,
+					 &sopt->iter_out) != len) {
 				sockopt_release_sock(sk);
 				return -EFAULT;
 			}
@@ -4775,9 +4736,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 			sockopt_release_sock(sk);
 		} else {
 			sockopt_release_sock(sk);
-			len = 0;
-			if (copy_to_sockptr(optlen, &len, sizeof(int)))
-				return -EFAULT;
+			sopt->optlen = 0;
 		}
 		return 0;
 	}
@@ -4787,21 +4746,21 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		struct tcp_zerocopy_receive zc = {};
 		int err;
 
-		if (copy_from_sockptr(&len, optlen, sizeof(int)))
-			return -EFAULT;
+		len = sopt->optlen;
 		if (len < 0 ||
 		    len < offsetofend(struct tcp_zerocopy_receive, length))
 			return -EINVAL;
 		if (unlikely(len > sizeof(zc))) {
-			err = check_zeroed_sockptr(optval, sizeof(zc),
-						   len - sizeof(zc));
+			if (WARN_ON_ONCE(!iter_is_ubuf(&sopt->iter_in)))
+				return -EINVAL;
+			err = check_zeroed_user(sopt->iter_in.ubuf + sizeof(zc),
+						len - sizeof(zc));
 			if (err < 1)
 				return err == 0 ? -EINVAL : err;
 			len = sizeof(zc);
-			if (copy_to_sockptr(optlen, &len, sizeof(int)))
-				return -EFAULT;
+			sopt->optlen = len;
 		}
-		if (copy_from_sockptr(&zc, optval, len))
+		if (copy_from_iter(&zc, len, &sopt->iter_in) != len)
 			return -EFAULT;
 		if (zc.reserved)
 			return -EINVAL;
@@ -4841,7 +4800,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 zerocopy_rcv_inq:
 		zc.inq = tcp_inq_hint(sk);
 zerocopy_rcv_out:
-		if (!err && copy_to_sockptr(optval, &zc, len))
+		if (!err && copy_to_iter(&zc, len, &sopt->iter_out) != len)
 			err = -EFAULT;
 		return err;
 	}
@@ -4849,19 +4808,16 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_AO_REPAIR:
 		if (!tcp_can_repair_sock(sk))
 			return -EPERM;
-		return tcp_ao_get_sockptr(sk, tcp_ao_get_repair,
-					  optval, optlen);
+		return tcp_ao_get_repair(sk, sopt);
 	case TCP_AO_GET_KEYS:
 	case TCP_AO_INFO: {
 		int err;
 
 		sockopt_lock_sock(sk);
 		if (optname == TCP_AO_GET_KEYS)
-			err = tcp_ao_get_sockptr(sk, tcp_ao_get_mkts,
-						 optval, optlen);
+			err = tcp_ao_get_mkts(sk, sopt);
 		else
-			err = tcp_ao_get_sockptr(sk, tcp_ao_get_sock_info,
-						 optval, optlen);
+			err = tcp_ao_get_sock_info(sk, sopt);
 		sockopt_release_sock(sk);
 
 		return err;
@@ -4882,9 +4838,8 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		return -ENOPROTOOPT;
 	}
 
-	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;
 }
@@ -4904,13 +4859,22 @@ int tcp_getsockopt(struct sock *sk, int level, int optname, char __user *optval,
 		   int __user *optlen)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
+	sockopt_t sopt;
+	int err;
 
 	if (level != SOL_TCP)
 		/* Paired with WRITE_ONCE() in do_ipv6_setsockopt() and tcp_v6_connect() */
 		return READ_ONCE(icsk->icsk_af_ops)->getsockopt(sk, level, optname,
 								optval, optlen);
-	return do_tcp_getsockopt(sk, level, optname, USER_SOCKPTR(optval),
-				 USER_SOCKPTR(optlen));
+
+	err = sockopt_init_user(&sopt, optval, optlen);
+	if (err)
+		return err;
+
+	err = do_tcp_getsockopt(sk, level, optname, &sopt);
+	if (put_user(sopt.optlen, optlen))
+		return -EFAULT;
+	return err;
 }
 
 #ifdef CONFIG_TCP_MD5SIG

-- 
2.53.0-Meta

Re: [PATCH net-next 2/3] tcp: convert do_tcp_getsockopt() to sockopt_t

[Stanislav Fomichev]

On 11/12, Breno Leitao wrote:
> Convert the SOL_TCP switch to sockopt_t. The TCP-AO helpers already
> take one, so they get it directly and the tcp_ao_get_sockptr() bridge
> goes away.
> 
> The options that re-read optlen to get past the sizeof(int) clamp at
> the top take it from sopt->optlen instead, so optlen is read once.
> 
> Only a user buffer reaches TCP_ZEROCOPY_RECEIVE: BPF's sol_tcp_sockopt()
> does not take it, and it maps pages into the caller's mm anyway. It
> keeps checking the bytes past its struct with check_zeroed_user(), now
> on iter_in.ubuf, and a kernel buffer gets -EINVAL.
> 
> tcp_getsockopt() builds the sockopt_t with sockopt_init_user() once the
> level is known to be SOL_TCP, and writes optlen back unconditionally,
> because TCP_SAVED_SYN reports the length it needs and then fails with
> -EINVAL. This mirrors ip_getsockopt().
> 
> Every SOL_TCP getsockopt() now writes optlen, including the ones that
> did not before, such as TCP_REPAIR_WINDOW, TCP_ZEROCOPY_RECEIVE and the
> error returns. An unwritable optlen word therefore turns them into
> -EFAULT.
> 
> The others stored optlen before optval, so an unwritable optlen word
> stopped optval from being written. Now optval is written first.
> 
> sol_tcp_sockopt() and sol_tcp_sockopt_congestion() reach
> do_tcp_getsockopt() through sol_tcp_getsockopt(), which builds the
> sockopt_t with sockptr_to_sockopt() and stores optlen back, as
> sol_ip_sockopt() does.
> 
> copy_to_iter() runs the hardened usercopy check on its source, which
> copy_to_sockptr() skips for a kernel buffer. BPF programs call
> bpf_getsockopt() from the receive softirq too. On the vmapped IRQ
> stack, the check then takes the lock in find_vmap_area(), which is not
> softirq safe, and lockdep reports an inconsistent lock state.
> 
> sol_tcp_getsockopt() therefore calls do_tcp_getsockopt() with page
> faults disabled, which makes check_heap_object() skip that lookup.
> Commit d319f344561d ("mm: Fix copy_from_user_nofault().") added that
> guard for BPF and kprobes.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next 2/3] tcp: convert do_tcp_getsockopt() to sockopt_t

[netdev-bot+sashiko]

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

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

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).

- [High] The commit message identifies a bug class: copy_to_iter() runs
  the hardened-usercopy check on a kernel stack source.
- [Low] TCP_SAVED_SYN now consumes and frees tp->saved_syn before optlen
  is written back.
- [Low] bpf: non-standard multi-line comment style in
  sol_tcp_getsockopt()

--

tcp: convert do_tcp_getsockopt() to sockopt_t

This converts do_tcp_getsockopt() from a sockptr_t optval/optlen pair to
sockopt_t and removes the tcp_ao_get_sockptr() bridge. tcp_getsockopt()
now always writes optlen back. BPF callers go through a new
sol_tcp_getsockopt() helper, which calls do_tcp_getsockopt() with page
faults disabled so the hardened usercopy check skips find_vmap_area().

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9b8d5191c4ba2..0d5afe1c5606b 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5686,6 +5686,28 @@ static int bpf_sol_tcp_setsockopt(struct sock *sk, int optname,
>  	return 0;
>  }
>  
> +static int sol_tcp_getsockopt(struct sock *sk, int optname, char *optval,
> +			      int *optlen)
> +{
> +	struct kvec kvec;
> +	sockopt_t opt;
> +	int err;
> +
> +	err = sockptr_to_sockopt(&opt, KERNEL_SOCKPTR(optval),
> +				 KERNEL_SOCKPTR(optlen), &kvec);
> +	if (err)
> +		return err;
> +
> +	/* BPF may call this in softirq: with page faults disabled, the
> +	 * usercopy check skips find_vmap_area() and its lock.
> +	 */
> +	pagefault_disable();
> +	err = do_tcp_getsockopt(sk, SOL_TCP, optname, &opt);
> +	pagefault_enable();

[Severity: Low]
This is a minor style nit. net/core/filter.c follows the BPF comment
style, which wants the opening /* of a multi-line comment on its own
line, with the text starting on the next line. That applies to new code
even where older comments in the file still use the net style.

The new comment in sol_tcp_getsockopt() puts the text on the opening
line. Could it be reformatted as:

	/*
	 * BPF may call this in softirq: with page faults disabled, the
	 * usercopy check skips find_vmap_area() and its lock.
	 */

If the pagefault_disable() moves to a shared BPF entry point, as asked
above, the same applies to the comment wherever it ends up.

[Severity: High]
Is the same softirq problem still present in the SOL_IP and SOL_IPV6 BPF
getsockopt paths in this file?

sol_ip_sockopt() and sol_ipv6_sockopt() build the same kvec-backed
sockopt_t, and they call the converted getters with no guard:

net/core/filter.c:sol_ip_sockopt() {
    ...
		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;
    ...
}

sol_ipv6_sockopt() does the same with do_ipv6_getsockopt(). Both
do_ip_getsockopt() and do_ipv6_getsockopt() end with:

	if (copy_to_iter(&val, len, &sopt->iter_out) != len)

Here val is a stack local and len is not a compile-time constant.

Take a sock_ops program running from a softirq callback such as
BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB, or a TCP CC struct_ops program, that
calls bpf_getsockopt(SOL_IP, IP_TOS). Wouldn't it go down this path?

__bpf_getsockopt()
  sol_ip_sockopt()
    do_ip_getsockopt()
      copy_to_iter()
        check_copy_size()
          __check_object_size()
            check_heap_object()
              find_vmap_area()

The IRQ stack is not current's task stack, and check_heap_object() only
skips the lookup when page faults are disabled:

mm/usercopy.c:check_heap_object() {
    ...
	if (is_vmalloc_addr(ptr) && !pagefault_disabled()) {
		struct vmap_area *area = find_vmap_area(addr);
    ...
}

On CONFIG_HARDENED_USERCOPY plus CONFIG_VMAP_STACK kernels, this looks
like the same inconsistent lock state on the find_vmap_area() lock. That
lock is taken with plain spin_lock(), so a softirq that interrupts a
process-context holder on the same CPU could also deadlock. The same
applies to bpf_getsockopt(SOL_IP, IP_TRANSPARENT) and to
bpf_getsockopt(SOL_IPV6, IPV6_TCLASS / IPV6_AUTOFLOWLABEL /
IPV6_TRANSPARENT).

The commit message names sol_ip_sockopt() as the model for the optlen
handling, but that function has the same exposure.

Would it be better to put the pagefault_disable() at the shared BPF entry
(__bpf_getsockopt() / _bpf_getsockopt()), or in the kernel-buffer
sockopt_t setup? Otherwise every per-level wrapper has to remember it.

[ ... ]

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 552e8fe22e7e2..de287845e6c0d 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c

[ ... ]

> @@ -4904,13 +4859,22 @@ int tcp_getsockopt(struct sock *sk, int level, int optname, char __user *optval,
>  		   int __user *optlen)
>  {
>  	struct inet_connection_sock *icsk = inet_csk(sk);
> +	sockopt_t sopt;
> +	int err;
>  
>  	if (level != SOL_TCP)
>  		/* Paired with WRITE_ONCE() in do_ipv6_setsockopt() and tcp_v6_connect() */
>  		return READ_ONCE(icsk->icsk_af_ops)->getsockopt(sk, level, optname,
>  								optval, optlen);
> -	return do_tcp_getsockopt(sk, level, optname, USER_SOCKPTR(optval),
> -				 USER_SOCKPTR(optlen));
> +
> +	err = sockopt_init_user(&sopt, optval, optlen);
> +	if (err)
> +		return err;
> +
> +	err = do_tcp_getsockopt(sk, level, optname, &sopt);
> +	if (put_user(sopt.optlen, optlen))
> +		return -EFAULT;
> +	return err;

[Severity: Low]
Can TCP_SAVED_SYN now return -EFAULT after the saved SYN has already been
freed?

In do_tcp_getsockopt(), the TCP_SAVED_SYN success branch copies
tp->saved_syn->data to iter_out, calls tcp_saved_syn_free(tp) and returns
0. Only after that does this put_user() write optlen, and if it fails the
0 becomes -EFAULT.

Before this patch, optlen was written before optval and before
tcp_saved_syn_free(). Every -EFAULT return therefore left tp->saved_syn
in place, and the caller could retry.

Now suppose optlen is readable but not writable. get_user() in
sockopt_init_user() succeeds and the put_user() here fails. The caller
gets -EFAULT, but the one-shot saved SYN has already been freed.

TCP_ZEROCOPY_RECEIVE also writes optlen here after tcp_zerocopy_receive()
has consumed data. That option could already return -EFAULT after
consuming data when optval was unwritable, though.

The commit message mentions that an unwritable optlen now gives -EFAULT.
It does not mention that TCP_SAVED_SYN frees its saved SYN before
returning that error.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261112-getsockopt_phase7-v1-0-1eb759dd0929%40debian.org

[PATCH net-next 3/3] selftests: net: getsockopt_iter: cover tcp

[Breno Leitao]

Add a fixture for do_tcp_getsockopt(). The setup keeps both ends of a
loopback connection, with TCP_SAVE_SYN set on the listener, so the
accepted socket has a saved SYN to report.

TCP_INFO and TCP_CONGESTION cover clamping the reply down to what fits,
TCP_ULP covers reporting zero and writing nothing, and TCP_SAVED_SYN
covers the length-then-EINVAL path, which the second half of that test
uses to read the SYN back.

TCP_ZEROCOPY_RECEIVE reads its struct from optval and writes it back.
With nothing queued it maps nothing and succeeds, so the tests cover
the 16-byte struct glibc still declares, a larger buffer cut down to
the kernel's struct, and one whose extra tail is not zero.

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

diff --git a/tools/testing/selftests/net/getsockopt_iter.c b/tools/testing/selftests/net/getsockopt_iter.c
index 2367fce226d4cc..4e51c8aa6a8555 100644
--- a/tools/testing/selftests/net/getsockopt_iter.c
+++ b/tools/testing/selftests/net/getsockopt_iter.c
@@ -55,6 +55,9 @@
 #ifndef TCP_ULP
 #define TCP_ULP 31
 #endif
+#ifndef TCP_CA_NAME_MAX
+#define TCP_CA_NAME_MAX 16
+#endif
 /* linux/mroute.h does not coexist with netinet/in.h here. */
 #ifndef MRT_VERSION
 #define MRT_VERSION 206
@@ -1005,4 +1008,246 @@ TEST_F(tls, bad_optname)
 	ASSERT_EQ(ENOPROTOOPT, errno);
 }
 
+/* ---------- tcp ---------- */
+
+FIXTURE(tcp)
+{
+	int fd;		/* connected client */
+	int sfd;	/* accepted server side, its SYN was saved */
+};
+
+FIXTURE_SETUP(tcp)
+{
+	struct sockaddr_in a = {
+		.sin_family = AF_INET,
+		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
+	};
+	socklen_t alen = sizeof(a);
+	int one = 1;
+	int lfd;
+
+	self->fd = -1;
+	self->sfd = -1;
+
+	lfd = socket(AF_INET, SOCK_STREAM, 0);
+	if (lfd < 0)
+		SKIP(return, "TCP socket: %s", strerror(errno));
+	setsockopt(lfd, IPPROTO_TCP, TCP_SAVE_SYN, &one, sizeof(one));
+	if (bind(lfd, (struct sockaddr *)&a, sizeof(a)) || listen(lfd, 1) ||
+	    getsockname(lfd, (struct sockaddr *)&a, &alen)) {
+		close(lfd);
+		SKIP(return, "listener setup: %s", strerror(errno));
+	}
+	self->fd = socket(AF_INET, SOCK_STREAM, 0);
+	if (self->fd < 0) {
+		close(lfd);
+		SKIP(return, "TCP socket: %s", strerror(errno));
+	}
+	if (connect(self->fd, (struct sockaddr *)&a, sizeof(a))) {
+		close(lfd);
+		SKIP(return, "connect: %s", strerror(errno));
+	}
+	self->sfd = accept(lfd, NULL, NULL);
+	close(lfd);
+	if (self->sfd < 0)
+		SKIP(return, "accept: %s", strerror(errno));
+}
+
+FIXTURE_TEARDOWN(tcp)
+{
+	if (self->fd >= 0)
+		close(self->fd);
+	if (self->sfd >= 0)
+		close(self->sfd);
+}
+
+TEST_F(tcp, maxseg_exact)
+{
+	socklen_t optlen = sizeof(int);
+	int val = -1;
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_MAXSEG, &val,
+				&optlen));
+	ASSERT_EQ(sizeof(int), optlen);
+	ASSERT_GT(val, 0);
+}
+
+/* Unlike IPv4, TCP clamps the reply rather than shrinking it to a byte. */
+TEST_F(tcp, nodelay_undersize_clamped)
+{
+	socklen_t optlen = 1;
+	int val = 0;
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_NODELAY, &val,
+				&optlen));
+	ASSERT_EQ(1, optlen);
+}
+
+TEST_F(tcp, info_exact)
+{
+	struct tcp_info info;
+	socklen_t optlen = sizeof(info);
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_INFO, &info,
+				&optlen));
+	ASSERT_EQ(sizeof(info), optlen);
+	ASSERT_EQ(TCP_ESTABLISHED, info.tcpi_state);
+}
+
+TEST_F(tcp, info_short_clamped)
+{
+	struct tcp_info info;
+	socklen_t optlen = 8;
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_INFO, &info,
+				&optlen));
+	ASSERT_EQ(8, optlen);
+	ASSERT_EQ(TCP_ESTABLISHED, info.tcpi_state);
+}
+
+TEST_F(tcp, congestion_exact)
+{
+	socklen_t optlen = TCP_CA_NAME_MAX;
+	char name[TCP_CA_NAME_MAX] = {};
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_CONGESTION, name,
+				&optlen));
+	ASSERT_EQ(TCP_CA_NAME_MAX, optlen);
+	ASSERT_NE(0, name[0]);
+}
+
+TEST_F(tcp, congestion_short_clamped)
+{
+	socklen_t optlen = 4;
+	char name[4] = {};
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_CONGESTION, name,
+				&optlen));
+	ASSERT_EQ(4, optlen);
+	ASSERT_NE(0, name[0]);
+}
+
+/* No ULP attached: length reported as zero, nothing written. */
+TEST_F(tcp, ulp_none)
+{
+	socklen_t optlen = 16;
+	char buf[16];
+
+	memset(buf, 0x5a, sizeof(buf));
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_ULP, buf, &optlen));
+	ASSERT_EQ(0, optlen);
+	ASSERT_EQ(0x5a, (unsigned char)buf[0]);
+}
+
+TEST_F(tcp, repair_window_wrong_len)
+{
+	socklen_t optlen = 4;
+	char buf[4];
+
+	ASSERT_EQ(-1, getsockopt(self->fd, IPPROTO_TCP, TCP_REPAIR_WINDOW, buf,
+				 &optlen));
+	ASSERT_EQ(EINVAL, errno);
+}
+
+TEST_F(tcp, zerocopy_receive_short)
+{
+	socklen_t optlen = 4;
+	char buf[4] = {};
+
+	ASSERT_EQ(-1, getsockopt(self->fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
+				 buf, &optlen));
+	ASSERT_EQ(EINVAL, errno);
+}
+
+/* Nothing queued: the call maps nothing and succeeds. */
+TEST_F(tcp, zerocopy_receive_old_struct)
+{
+	struct tcp_zerocopy_receive zc = {};
+	socklen_t optlen = sizeof(zc);
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
+				&zc, &optlen));
+	ASSERT_EQ(sizeof(zc), optlen);
+	ASSERT_EQ(0, zc.length);
+}
+
+/* A larger struct is cut down to the kernel's, if its tail is zero. */
+TEST_F(tcp, zerocopy_receive_oversize_clamped)
+{
+	char buf[256] = {};
+	socklen_t optlen = sizeof(buf);
+
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
+				buf, &optlen));
+	ASSERT_GE(optlen, sizeof(struct tcp_zerocopy_receive));
+	ASSERT_LT(optlen, sizeof(buf));
+}
+
+TEST_F(tcp, zerocopy_receive_oversize_dirty)
+{
+	char buf[256] = {};
+	socklen_t optlen = sizeof(buf);
+
+	buf[sizeof(buf) - 1] = 1;
+	ASSERT_EQ(-1, getsockopt(self->fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
+				 buf, &optlen));
+	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(sizeof(buf), optlen);
+}
+
+/* The client never saved a SYN, so there is nothing to report. */
+TEST_F(tcp, saved_syn_none)
+{
+	socklen_t optlen = 128;
+	char buf[128];
+
+	memset(buf, 0x5a, sizeof(buf));
+	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_SAVED_SYN, buf,
+				&optlen));
+	ASSERT_EQ(0, optlen);
+	ASSERT_EQ(0x5a, (unsigned char)buf[0]);
+}
+
+/* A short buffer gets the required length back along with EINVAL, and the
+ * SYN is still there for the retry.
+ */
+TEST_F(tcp, saved_syn_short_reports_len)
+{
+	socklen_t optlen = 1;
+	char buf[256];
+	socklen_t need;
+
+	ASSERT_EQ(-1, getsockopt(self->sfd, IPPROTO_TCP, TCP_SAVED_SYN, buf,
+				 &optlen));
+	ASSERT_EQ(EINVAL, errno);
+	ASSERT_GT(optlen, 1);
+	ASSERT_LE(optlen, sizeof(buf));
+	need = optlen;
+
+	ASSERT_EQ(0, getsockopt(self->sfd, IPPROTO_TCP, TCP_SAVED_SYN, buf,
+				&optlen));
+	ASSERT_EQ(need, optlen);
+}
+
+TEST_F(tcp, negative_optlen)
+{
+	socklen_t optlen = (socklen_t)-1;
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, IPPROTO_TCP, TCP_MAXSEG, &val,
+				 &optlen));
+	ASSERT_EQ(EINVAL, errno);
+}
+
+TEST_F(tcp, bad_optname)
+{
+	socklen_t optlen = sizeof(int);
+	int val;
+
+	ASSERT_EQ(-1, getsockopt(self->fd, IPPROTO_TCP, 0x7fff, &val,
+				 &optlen));
+	ASSERT_EQ(ENOPROTOOPT, errno);
+	ASSERT_EQ(sizeof(int), optlen);
+}
+
 TEST_HARNESS_MAIN

-- 
2.53.0-Meta

Re: [PATCH net-next 3/3] selftests: net: getsockopt_iter: cover tcp

[Stanislav Fomichev]

On 11/12, Breno Leitao wrote:
> Add a fixture for do_tcp_getsockopt(). The setup keeps both ends of a
> loopback connection, with TCP_SAVE_SYN set on the listener, so the
> accepted socket has a saved SYN to report.
> 
> TCP_INFO and TCP_CONGESTION cover clamping the reply down to what fits,
> TCP_ULP covers reporting zero and writing nothing, and TCP_SAVED_SYN
> covers the length-then-EINVAL path, which the second half of that test
> uses to read the SYN back.
> 
> TCP_ZEROCOPY_RECEIVE reads its struct from optval and writes it back.
> With nothing queued it maps nothing and succeeds, so the tests cover
> the 16-byte struct glibc still declares, a larger buffer cut down to
> the kernel's struct, and one whose extra tail is not zero.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next 3/3] selftests: net: getsockopt_iter: cover tcp

[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] In tools/testing/selftests/net/getsockopt_iter.c, the test
  tcp/nodelay_undersize_clamped is commented "Unlike IPv4, TCP clamps…
- [Low] In tools/testing/selftests/net/getsockopt_iter.c, the test
  tcp/zerocopy_receive_oversize_clamped is commented "A larger struct is…

--

selftests: net: getsockopt_iter: cover tcp

This commit adds a tcp fixture to the getsockopt_iter selftest. The fixture
sets up a loopback connection with TCP_SAVE_SYN on the listener. The new
tests cover do_tcp_getsockopt() clamping, zero-length replies, the
TCP_SAVED_SYN length-then-EINVAL path, and the TCP_ZEROCOPY_RECEIVE struct
size handling.

> diff --git a/tools/testing/selftests/net/getsockopt_iter.c b/tools/testing/selftests/net/getsockopt_iter.c
> index 2367fce226d4c..4e51c8aa6a855 100644
> --- a/tools/testing/selftests/net/getsockopt_iter.c
> +++ b/tools/testing/selftests/net/getsockopt_iter.c

[ ... ]

> +/* Unlike IPv4, TCP clamps the reply rather than shrinking it to a byte. */
> +TEST_F(tcp, nodelay_undersize_clamped)
> +{
> +	socklen_t optlen = 1;
> +	int val = 0;
> +
> +	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_NODELAY, &val,
> +				&optlen));
> +	ASSERT_EQ(1, optlen);
> +}

[Severity: Low]
Does this test actually show the difference the comment describes?

With optlen = 1, both paths report the same length. do_tcp_getsockopt()
clamps:

net/ipv4/tcp.c:do_tcp_getsockopt() {
    ...
	len = min_t(unsigned int, len, sizeof(int));
    ...
}

and do_ip_getsockopt() also ends up with optlen = 1:

net/ipv4/ip_sockglue.c:do_ip_getsockopt() {
    ...
	if (len < sizeof(int) && len > 0 && val >= 0 && val <= 255) {
		unsigned char ucval = (unsigned char)val;

		sopt->optlen = 1;
    ...
}

So if TCP started shrinking sub-int replies to a single byte the way SOL_IP
does, this test would still pass.

The ip/ttl_single_byte and ipv6/hops_sub_int tests in the same file use a
3-byte buffer, which does tell the two apart. Would optlen = 2 or 3, with a
check that optlen is unchanged, match the intent of the comment better?

[ ... ]

> +/* A larger struct is cut down to the kernel's, if its tail is zero. */
> +TEST_F(tcp, zerocopy_receive_oversize_clamped)
> +{
> +	char buf[256] = {};
> +	socklen_t optlen = sizeof(buf);
> +
> +	ASSERT_EQ(0, getsockopt(self->fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
> +				buf, &optlen));
> +	ASSERT_GE(optlen, sizeof(struct tcp_zerocopy_receive));
> +	ASSERT_LT(optlen, sizeof(buf));
> +}

[Severity: Low]
Should this assert the exact clamped length? When the tail is zero, the
kernel sets optlen to the size of its own struct:

net/ipv4/tcp.c:do_tcp_getsockopt() {
    ...
	if (unlikely(len > sizeof(zc))) {
		...
		len = sizeof(zc);
		sopt->optlen = len;
    ...
}

That is the uapi struct tcp_zerocopy_receive from include/uapi/linux/tcp.h,
which is 64 bytes.

The test includes netinet/tcp.h, so sizeof(struct tcp_zerocopy_receive) here
is the 16-byte glibc struct the commit message mentions. That means any
optlen from 16 to 255 passes. A clamp to the wrong size, such as one of the
older 16 or 40 byte layouts, would not be caught.

raw/icmpfilter_oversize_clamped in the same file uses ASSERT_EQ against the
exact struct size. Could this test do the same with the kernel struct size
(64)?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261112-getsockopt_phase7-v1-0-1eb759dd0929%40debian.org