Messages in this thread Patch in this message |  | | Date | Thu, 2 Apr 2026 08:39:26 -0700 | | From | Breno Leitao <> | | Subject | Re: [PATCH net-next v2 2/4] net: call getsockopt_iter if available |
| |
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
|  |