Re: [PATCH net-next v3 0/4] net: move .getsockopt away from __user buffers

[Breno Leitao]

On Wed, Apr 08, 2026 at 07:56:40PM +0100, David Laight wrote:
> On Wed, 8 Apr 2026 06:52:54 -0700
> Breno Leitao <leitao@xxxxxxxxxx> wrote:
>
> > Hello David,
> >
> > On Wed, Apr 08, 2026 at 12:26:53PM +0100, David Laight wrote:
> > > On Wed, 08 Apr 2026 03:30:28 -0700
> > > Breno Leitao <leitao@xxxxxxxxxx> wrote:
> > >
> > > > Currently, the .getsockopt callback requires __user pointers:
> > > >
> > > >   int (*getsockopt)(struct socket *sock, int level,
> > > >                     int optname, char __user *optval, int __user *optlen);
> > > >
> > > > This prevents kernel callers (io_uring, BPF) from using getsockopt on
> > > > levels other than SOL_SOCKET, since they pass kernel pointers.
> > > >
> > > > Following Linus' suggestion [0], this series introduces sockopt_t, a
> > > > type-safe wrapper around iov_iter, and a getsockopt_iter callback that
> > > > works with both user and kernel buffers. AF_PACKET and CAN raw are
> > > > converted as initial users, with selftests covering the trickiest
> > > > conversion patterns.
> > >
> > > What are you doing about the cases where 'optlen' is a complete lie?
> >
> > Is this incorrect optlen originating from userspace, and getting into
> > the .getsockopt callbacks?
>
> Look at tcp_ao_copy_mptks_to_user() in net/ipv4/tcp_ao.c
> This isn't 'old code' it was added in 2023.
>
> Basically what is being transferred is an array and 'optlen' is the
> size of one element.
> The number of elements is in the first one.

Thank you for pointing this out. I now understand the issue you're raising.

The problem is that optlen doesn't represent the full buffer length. Instead:
	optlen = per-element struct size (stride)
	actual buffer length = optlen * nkeys (where nkeys comes from optval[0])

For handling these cases, I can think of several approaches:

1) Dynamically resize the iter once the actual buffer size is discovered:

  int sockopt_set_buflen(sockopt_t *opt, size_t new_len)
  {
      if (!opt->legacy)   /* Following Stefan's suggestion */
	return -EINVAL;

      /* Re-initialize iter with the actual buffer size.
       * For ubuf: same base pointer, updated count.
       * For kvec: same iov_base, updated iov_len + re-init.
       */
  }

This allows legacy protocols to adjust the iov_iter later, mirroring
current behavior. Note that this doesn't worsen the existing
situation—currently, the current code is like having iov_iter length is
set to INT_MAX, given the callback can read/write to any location based
on that __user pointer.

2) Use a special legacy callback path, as proposed by Stefan.

3) Store base pointers in sockopt_t and defer iter initialization for
   legacy callbacks:

  static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
                                    struct tcp_ao_info *ao_info,
                                    sockopt_t *opt)
  {
        struct tcp_ao_getsockopt opt_in;
        int user_len = opt->optlen;
        struct kvec kvec;

        /* First, initialize a small iter to read the first element */
        sockopt_init_iter(opt, user_len, &kvec);

        if (copy_from_iter(&opt_in, user_len, &opt->iter_in) != user_len)
                return -EFAULT;

        /* Now we know the actual buffer size */
        sockopt_init_iter(opt, user_len * opt_in.nkeys, &kvec);

        /* ... write the full array via copy_to_iter() ... */
  }

4) Maintain two separate callbacks: ->getsockopt_iter (to be renamed to ->getsockopt
   after the transition) and ->getsockopt_unsafe for legacy cases.


Regardless of which approach we take for these legacy implementations, I don't
believe any of them invalidate the current patchset from a design standpoint.

Since these legacy protocols represent less than 1% of the cases, I'd prefer to
optimize for the common path and handle the exceptional cases as exceptions.