Re: [PATCH net-next v2 2/4] net: call getsockopt_iter if available

[Breno Leitao <leitao@debian.org>]

Hello Stanislav,

On Wed, Apr 01, 2026 at 11:10:22AM -0700, Stanislav Fomichev wrote:
> So maybe something like this is better to communicate your long term intent?
> 
> 	} else if (ops->getsockopt_iter) {
> 		optval = sockptr_to_iter(optval)
> 		optlen = sockptr_to_iter(optlen)
> 		do_sock_getsockopt_iter(...) /* does not know what sockpt_t is */
> 	}
> 
> ?
> 
> Then your new do_sock_getsockopt_iter is sockptr-free from the beginning
> and at some point we'll just drop/move those sockptr_to_iter calls?

Sure, that would work as well. It would look like the following, from my
current implemention:

+static int sockptr_to_sockopt(sockopt_t *opt, sockptr_t optval,
+                             sockptr_t optlen, struct kvec *kvec)
+{
+       int koptlen;
+
+       if (copy_from_sockptr(&koptlen, optlen, sizeof(int)))
+               return -EFAULT;
+
+       if (optval.is_kernel) {
+               kvec->iov_base = optval.kernel;
+               kvec->iov_len = koptlen;
+               iov_iter_kvec(&opt->iter_out, ITER_DEST, kvec, 1, koptlen);
+               iov_iter_kvec(&opt->iter_in, ITER_SOURCE, kvec, 1, koptlen);
+       } else {
+               iov_iter_ubuf(&opt->iter_out, ITER_DEST, optval.user, koptlen);
+               iov_iter_ubuf(&opt->iter_in, ITER_SOURCE, optval.user,
+                             koptlen);
+       }
+       opt->optlen = koptlen;
+
+       return 0;
+}
+
 int do_sock_getsockopt(struct socket *sock, bool compat, int level,
                       int optname, sockptr_t optval, sockptr_t optlen)
 {
@@ -2366,15 +2390,31 @@ int do_sock_getsockopt(struct socket *sock, bool compat, int level,

+       } else if (ops->getsockopt_iter) {
+               struct kvec kvec;
+               sockopt_t opt;
+
+               err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
+               if (err)
+                       return err;
+
+               err = ops->getsockopt_iter(sock, level, optname, &opt);
+
+               /* Always write back optlen, even on failure. Some protocols
+                * (e.g. CAN raw) return -ERANGE and set optlen to the
+                * required buffer size so userspace can discover it.
+                */
+               if (copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
+                       return -EFAULT;
+       } else if (ops->getsockopt) {
....

> I hope this way it will be easier to review protocol handler changes.
> 
> For example, looking at your AF_PACKET patch, you won't have to care
> about flipping the source and doing the revert. Most/all of the changes will
> be simple:
> - s/get_user(len, optlen)/len = opt->optlen/
> - s/put_user(len, optlen)/opt->optlen = len/
> - s/copy_from_user(xxx, optval, len)/copy_from_iter(xxx, len, &opt->iter_in)/
> - s/copy_to_user(optval, xxx, len)/copy_to_iter(xxx, len, &opt->iter_out)/

That is, in fact, a great proposal. It will make the protocol changes review
way easier.

This is what I have right now.

	typedef struct sockopt {
		struct iov_iter iter_out;
		struct iov_iter iter_in;
		int optlen;
	} sockopt_t;


And then, the drivers change would be as simple as:

 static int packet_getsockopt(struct socket *sock, int level, int optname,
-                            char __user *optval, int __user *optlen)
+                            sockopt_t *opt)
 {
        int len;
        int val, lv = sizeof(val);
@@ -4065,8 +4066,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
        if (level != SOL_PACKET)
                return -ENOPROTOOPT;

-       if (get_user(len, optlen))
-               return -EFAULT;
+       len = opt->optlen;

        if (len < 0)
                return -EINVAL;
@@ -4115,7 +4115,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
                        len = sizeof(int);
                if (len < sizeof(int))
                        return -EINVAL;
-               if (copy_from_user(&val, optval, len))
+               if (copy_from_iter(&val, len, &opt->iter_in) != len)
                        return -EFAULT;
                switch (val) {
                case TPACKET_V1:
@@ -4171,9 +4171,8 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,

        if (len > lv)
                len = lv;
-       if (put_user(len, optlen))
-               return -EFAULT;
-       if (copy_to_user(optval, data, len))
+       opt->optlen = len;
+       if (copy_to_iter(data, len, &opt->iter_out) != len)
                return -EFAULT;
        return 0;

This is not fully tested yet, but, in case you want to see how this looks like
so far, I have it in https://github.com/leitao/linux/tree/b4/getsockopt_v3.

I will submit a newer version after I am done with the testing.

Thanks for the insights,
--breno