All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: David Laight <david.laight.linux@gmail.com>
Cc: sdf@fomichev.me, "David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Simon Horman" <horms@kernel.org>,
	"Alexander Aring" <alex.aring@gmail.com>,
	"Stefan Schmidt" <stefan@datenfreihafen.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Remi Denis-Courmont" <courmisch@gmail.com>,
	"Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Sabrina Dubroca" <sd@queasysnail.net>,
	"Shuah Khan" <shuah@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-wpan@vger.kernel.org, linux-kselftest@vger.kernel.org,
	kernel-team@meta.com
Subject: Re: [PATCH net-next 5/7] phonet: pep: convert getsockopt to sockopt_t
Date: Fri, 17 Jul 2026 06:02:29 -0700	[thread overview]
Message-ID: <alonKANZx3ll0Ryo@gmail.com> (raw)
In-Reply-To: <20260717092258.776e63db@pumpkin>

On Fri, Jul 17, 2026 at 09:22:58AM +0100, David Laight wrote:

> > -	len = min_t(unsigned int, sizeof(int), len);
> > -	if (put_user(len, optlen))
> > +	len = min_t(unsigned int, sizeof(int), opt->optlen);
> 
> I'm pretty sure that can be min().

Right. opt->optlen is a plain int and sizeof(int) is a non-negative
constant, so min() takes it fine.

I'll switch it to min() in v2, thanks for the review.

> More generally I'm not at all sure about negative lengths.
> Historically the user type would have been 'int', but it got replaced
> by socklen_t which is probably unsigned.
> (IIRC one of the 64bit Unix had started using a 64bit type but I think
> it was sun objected to making that change so socklen_t was born.)

That's now handled one level up, in sockopt_init_user():

        if (get_user(len, optlen))
                return -EFAULT;
        if (len < 0)
                return -EINVAL;

> Also truncating below 4 bytes makes no sense here on BE systems.

Sure, but this is pre-existing, isn't it?

> Some code will try to write the significant bytes out, but that
> would be better done in the wrapper.

This is the transitional shim. The ->getsockopt prototype is shared by
every proto:

        int (*getsockopt)(struct sock *sk, int level, int optname,
                          char __user *optval, int __user *optlen);

So it can't take a sockopt_t until every leaf is converted.

Each converted leaf keeps the __user signature, builds the sockopt_t
itself and calls a do_*_getsockopt(sk, optname, sockopt_t *) helper.
Once all the getsockopt leaves are done, sockopt_init_user() moves into
the common caller, the prototype flips to sockopt_t *, and these
per-leaf wrappers are deleted. The full plan is in the phase-1 cover
letter:

https://lore.kernel.org/all/20260401-getsockopt-v2-0-611df6771aff@debian.org/

Thanks for the review,
--breno

  reply	other threads:[~2026-07-17 13:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 12:59 [PATCH net-next 0/7] net: convert rawv6, ieee802154, phonet and tls getsockopt to sockopt_t Breno Leitao
2026-07-16 12:59 ` [PATCH net-next 1/7] ipv6: raw: drop unused level argument from do_rawv6_getsockopt Breno Leitao
2026-07-16 13:00 ` [PATCH net-next 2/7] ipv6: raw: convert do_rawv6_getsockopt to sockopt_t Breno Leitao
2026-07-16 13:00 ` [PATCH net-next 3/7] ieee802154: convert dgram getsockopt " Breno Leitao
2026-07-16 13:00 ` [PATCH net-next 4/7] phonet: pep: do not write beyond optlen in getsockopt Breno Leitao
2026-07-17  6:50   ` Rémi Denis-Courmont
2026-07-16 13:00 ` [PATCH net-next 5/7] phonet: pep: convert getsockopt to sockopt_t Breno Leitao
2026-07-17  6:52   ` Rémi Denis-Courmont
2026-07-17  8:22   ` David Laight
2026-07-17 13:02     ` Breno Leitao [this message]
2026-07-16 13:00 ` [PATCH net-next 6/7] tls: " Breno Leitao
2026-07-16 13:00 ` [PATCH net-next 7/7] selftests: net: getsockopt_iter: cover rawv6, ieee802154, phonet and tls Breno Leitao
2026-07-17  6:53   ` Rémi Denis-Courmont

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alonKANZx3ll0Ryo@gmail.com \
    --to=leitao@debian.org \
    --cc=alex.aring@gmail.com \
    --cc=courmisch@gmail.com \
    --cc=davem@davemloft.net \
    --cc=david.laight.linux@gmail.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@meta.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-wpan@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=remi.denis-courmont@nokia.com \
    --cc=sd@queasysnail.net \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --cc=stefan@datenfreihafen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.