mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen
@ 2026-09-14 12:20 Breno Leitao
  2026-09-14 12:20 ` [PATCH net-next v2 1/2] net: add sockopt_expand_out() Breno Leitao
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Breno Leitao @ 2026-09-14 12:20 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
	Ido Schimmel
  Cc: netdev, linux-kernel, david.laight.linux, Breno Leitao, kernel-team

This series continues the migration of our protocols to sockopt_t, as
described in [1].

There are some protocols that use optlen as the header size, and the real
buffer size comes from a field inside the header. This means a bug, given
that optlen should be the full buffer size, but there are indications [2]
that we have programs that use the bad behaviour above, and we want to
avoid breaking them (or, honestly, avoid being cursed by Linus).

That said, create a quirk helper that preserves the same behaviour, even
using sockopt_t. The way to do it is simple:

1) Only do it for userspace callers (ubuf), otherwise a bug here will
   corrupt the kernel instead of a simple SIGSEGV.
2) Expand optval mid-air based on the header field.

IP_MSFILTER is the first user, and the smallest one: a single caller, no
compat variant, and a reply written front to back. MCAST_MSFILTER on ipv4
and ipv6 comes next, and TCP_AO_GET_KEYS has the same shape. Do this
quirk on IP_MSFILTER to make sure the dynamic is ok, so, we can expand
it later.

None of this is meant to change what userspace sees.

Link: https://lore.kernel.org/all/20260401-getsockopt-v2-0-611df6771aff@debian.org/ [1]
Link: https://lore.kernel.org/all/20260806-mcast_fix-v1-0-bed0a5518e57@debian.org/ [2]

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- sockopt_expand_out() measures the request against opt->optlen instead
  of the iterator's remaining count, and asserts that nothing has been
  written through iter_out yet. Comparing against the remaining count
  made the verdict depend on how far a callback had already got, and a
  late call would rewind the write cursor to the head of optval.
- Cap the requested size at INT_MAX, since optlen and the getsockopt ABI
  are int.
- do_ip_getsockopt() writes optlen back only when ip_mc_msfget()
  succeeded. Without the guard, a read-only optlen turned -EINVAL,
  -ENODEV and -EADDRNOTAVAIL into -EFAULT.
- Name IP_MSFILTER in patch 1, document the WARN_ON_ONCE() and that no
  in-tree path reaches it, and mention sockptr_to_sockopt() losing its
  static.
- Link to v1: https://patch.msgid.link/20260910-getsockopt_phase6-v1-0-e681e102d5b8@debian.org

---
Breno Leitao (2):
      net: add sockopt_expand_out()
      ipv4: igmp: convert ip_mc_msfget() to sockopt_t

 include/linux/igmp.h   |  3 ++-
 include/linux/net.h    | 32 ++++++++++++++++++++++++++++++++
 net/ipv4/igmp.c        | 23 ++++++++++++++---------
 net/ipv4/ip_sockglue.c | 10 +++++++++-
 net/socket.c           |  4 ++--
 5 files changed, 59 insertions(+), 13 deletions(-)
---
base-commit: 548b86839f7fb819a4d6c83b71c73ec378d24275
change-id: 20260909-getsockopt_phase6-c7c96a21b062

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next v2 1/2] net: add sockopt_expand_out()
  2026-09-14 12:20 [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen Breno Leitao
@ 2026-09-14 12:20 ` Breno Leitao
  2026-09-14 12:20 ` [PATCH net-next v2 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t Breno Leitao
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-09-14 12:20 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
	Ido Schimmel
  Cc: netdev, linux-kernel, david.laight.linux, Breno Leitao, kernel-team

Some getsockopt options size their reply from a count the caller left in
optval rather than from optlen, and so write past the optlen the caller
declared. Userspace relies on that, so the sockopt_t conversion has to
keep doing it.

IP_MSFILTER is the first one to convert: its reply covers the
imsf_numsrc sources the caller asked for, while optlen only has to cover
the fixed header.

Add sockopt_expand_out() to grow opt->iter_out mid-air, so the quirk
sits in one place instead of each protocol assuming it implicitly. It is
a no-op unless the reply outruns optlen. Growing re-anchors the iterator
at the head of optval, so it has to be called before anything is written
through iter_out.

Only a user buffer can be longer than optlen says. A kernel-backed
optval keeps the bounded iterator, and a callback that asks to grow one
gets a WARN_ON_ONCE() and -EINVAL.

Nothing in tree can trip that WARN: the only kernel-backed path into
do_ip_getsockopt() is sol_ip_sockopt(), which takes IP_TOS and
IP_TRANSPARENT only. It is an assert for the in-kernel callers BPF and
io_uring gain once the conversion is done, so they declare an optlen
covering the whole buffer instead of repeating the userspace mistake.

The conversion in the next patch calls sockptr_to_sockopt() from
net/ipv4/, so drop its static and declare it in net.h.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/net.h | 32 ++++++++++++++++++++++++++++++++
 net/socket.c        |  4 ++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index 470100ae710773..7db3aff33f2ba5 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -70,6 +70,38 @@ static inline int sockopt_init_user(sockopt_t *opt, char __user *optval,
 	return 0;
 }
 
+/*
+ * Grow optval to @size, for the options whose reply is sized by a count the
+ * caller left in optval rather than by optlen. Those write past optlen today
+ * and userspace relies on it.
+ *
+ * Call it before writing through opt->iter_out: it re-anchors the iterator at
+ * the head of optval. Only a user buffer can be longer than the optlen the
+ * caller declared, so a kernel-backed optval is refused with -EINVAL.
+ */
+static inline int sockopt_expand_out(sockopt_t *opt, size_t size)
+{
+	if (size <= (size_t)opt->optlen)
+		return 0;
+
+	if (size > INT_MAX)
+		return -EINVAL;
+
+	/* Re-anchoring reads iter_out.ubuf, so the iterator has to be a user
+	 * buffer that nothing has written through yet.
+	 */
+	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out) ||
+			 iov_iter_count(&opt->iter_out) != (size_t)opt->optlen))
+		return -EINVAL;
+
+	iov_iter_ubuf(&opt->iter_out, ITER_DEST, opt->iter_out.ubuf, size);
+
+	return 0;
+}
+
+int sockptr_to_sockopt(sockopt_t *opt, sockptr_t optval, sockptr_t optlen,
+		       struct kvec *kvec);
+
 struct poll_table_struct;
 struct pipe_inode_info;
 struct inode;
diff --git a/net/socket.c b/net/socket.c
index c05d86e63abf7d..29a0f7f8e2cabe 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2437,8 +2437,8 @@ INDIRECT_CALLABLE_DECLARE(bool tcp_bpf_bypass_getsockopt(int level,
  * It is important to remember that both iov points to the same data, but,
  * .iter_in is read-only and .iter_out is write-only by the protocol callbacks
  */
-static int sockptr_to_sockopt(sockopt_t *opt, sockptr_t optval,
-			      sockptr_t optlen, struct kvec *kvec)
+int sockptr_to_sockopt(sockopt_t *opt, sockptr_t optval,
+		       sockptr_t optlen, struct kvec *kvec)
 {
 	int koptlen;
 

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next v2 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t
  2026-09-14 12:20 [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen Breno Leitao
  2026-09-14 12:20 ` [PATCH net-next v2 1/2] net: add sockopt_expand_out() Breno Leitao
@ 2026-09-14 12:20 ` Breno Leitao
  2026-09-14 13:46 ` [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen David Laight
  2026-09-14 16:19 ` Stanislav Fomichev
  3 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2026-09-14 12:20 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
	Ido Schimmel
  Cc: netdev, linux-kernel, david.laight.linux, Breno Leitao, kernel-team

IP_MSFILTER reads its reply through ip_mc_msfget(), reached from
do_ip_getsockopt() and from nowhere else. Convert it, and build the
sockopt_t at the call site for as long as the caller still carries a
sockptr_t pair.

optlen here only has to cover the header, and the real reply size comes
from the imsf_numsrc field inside it. This is nasty, but userspace
relies on it, so sockopt_expand_out() preserves the same mechanism: it
grows optval only for a user address, and assumes the caller left room
for the size its own header asked for.

The *optlen store moves out of ip_mc_msfget() and into the call site,
guarded by !err so the -EINVAL, -ENODEV and -EADDRNOTAVAIL returns still
leave the caller's optlen word untouched.

The source list also moves from copy_to_sockptr_offset() to a sequential
copy_to_iter(). IP_MSFILTER_SIZE(0) and offsetof(struct ip_msfilter,
imsf_slist_flex) are both 16, so the bytes land where they did.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/igmp.h   |  3 ++-
 net/ipv4/igmp.c        | 23 ++++++++++++++---------
 net/ipv4/ip_sockglue.c | 10 +++++++++-
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index a0cf0398519fd7..e075611344ef3b 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -14,6 +14,7 @@
 #include <linux/timer.h>
 #include <linux/in.h>
 #include <linux/ip.h>
+#include <linux/net.h>
 #include <linux/refcount.h>
 #include <linux/sockptr.h>
 #include <uapi/linux/igmp.h>
@@ -273,7 +274,7 @@ extern int ip_mc_source(int add, int omode, struct sock *sk,
 		struct ip_mreq_source *mreqs, int ifindex);
 extern int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf,int ifindex);
 extern int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
-			sockptr_t optval, sockptr_t optlen);
+			sockopt_t *opt);
 extern int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 			sockptr_t optval, size_t offset);
 extern int ip_mc_sf_allow(const struct sock *sk, __be32 local, __be32 rmt,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d56355aca79776..144fca158adcb0 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2709,8 +2709,8 @@ int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf, int ifindex)
 		err = ip_mc_leave_group(sk, &imr);
 	return err;
 }
-int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
-		 sockptr_t optval, sockptr_t optlen)
+
+int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf, sockopt_t *opt)
 {
 	int err, len, count, copycount, msf_size;
 	struct ip_mreqn	imr;
@@ -2755,14 +2755,19 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 	len = flex_array_size(psl, sl_addr, copycount);
 	msf->imsf_numsrc = count;
 	msf_size = IP_MSFILTER_SIZE(copycount);
-	if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) ||
-	    copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) {
+
+	/* The source list is sized by the imsf_numsrc the caller left in
+	 * optval, not by optlen, which only has to cover the fixed part.
+	 */
+	err = sockopt_expand_out(opt, msf_size);
+	if (err)
+		return err;
+
+	opt->optlen = msf_size;
+	if (copy_to_iter(msf, IP_MSFILTER_SIZE(0), &opt->iter_out) !=
+	    IP_MSFILTER_SIZE(0))
 		return -EFAULT;
-	}
-	if (len &&
-	    copy_to_sockptr_offset(optval,
-				   offsetof(struct ip_msfilter, imsf_slist_flex),
-				   psl->sl_addr, len))
+	if (len && copy_to_iter(psl->sl_addr, len, &opt->iter_out) != len)
 		return -EFAULT;
 	return 0;
 done:
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index a55ef327ec932c..e06c1f48ecad6e 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1706,6 +1706,8 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 	case IP_MSFILTER:
 	{
 		struct ip_msfilter msf;
+		struct kvec kvec;
+		sockopt_t opt;
 
 		if (len < IP_MSFILTER_SIZE(0)) {
 			err = -EINVAL;
@@ -1715,7 +1717,13 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
 			err = -EFAULT;
 			goto out;
 		}
-		err = ip_mc_msfget(sk, &msf, optval, optlen);
+		err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
+		if (err)
+			goto out;
+
+		err = ip_mc_msfget(sk, &msf, &opt);
+		if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
+			err = -EFAULT;
 		goto out;
 	}
 	case MCAST_MSFILTER:

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen
  2026-09-14 12:20 [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen Breno Leitao
  2026-09-14 12:20 ` [PATCH net-next v2 1/2] net: add sockopt_expand_out() Breno Leitao
  2026-09-14 12:20 ` [PATCH net-next v2 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t Breno Leitao
@ 2026-09-14 13:46 ` David Laight
  2026-09-14 16:19 ` Stanislav Fomichev
  3 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2026-09-14 13:46 UTC (permalink / raw)
  To: Breno Leitao
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
	Ido Schimmel, netdev, linux-kernel, kernel-team

On Mon, 14 Sep 2026 05:20:06 -0700
Breno Leitao <leitao@debian.org> wrote:

...
> and ipv6 comes next, and TCP_AO_GET_KEYS has the same shape.

That is the sockopt I couldn't remember last time.

It is really horrid and shouldn't have been allowed.

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen
  2026-09-14 12:20 [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen Breno Leitao
                   ` (2 preceding siblings ...)
  2026-09-14 13:46 ` [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen David Laight
@ 2026-09-14 16:19 ` Stanislav Fomichev
  3 siblings, 0 replies; 5+ messages in thread
From: Stanislav Fomichev @ 2026-09-14 16:19 UTC (permalink / raw)
  To: Breno Leitao
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
	Ido Schimmel, netdev, linux-kernel, david.laight.linux,
	kernel-team

On 09/14, Breno Leitao wrote:
> This series continues the migration of our protocols to sockopt_t, as
> described in [1].
> 
> There are some protocols that use optlen as the header size, and the real
> buffer size comes from a field inside the header. This means a bug, given
> that optlen should be the full buffer size, but there are indications [2]
> that we have programs that use the bad behaviour above, and we want to
> avoid breaking them (or, honestly, avoid being cursed by Linus).
> 
> That said, create a quirk helper that preserves the same behaviour, even
> using sockopt_t. The way to do it is simple:
> 
> 1) Only do it for userspace callers (ubuf), otherwise a bug here will
>    corrupt the kernel instead of a simple SIGSEGV.
> 2) Expand optval mid-air based on the header field.
> 
> IP_MSFILTER is the first user, and the smallest one: a single caller, no
> compat variant, and a reply written front to back. MCAST_MSFILTER on ipv4
> and ipv6 comes next, and TCP_AO_GET_KEYS has the same shape. Do this
> quirk on IP_MSFILTER to make sure the dynamic is ok, so, we can expand
> it later.
> 
> None of this is meant to change what userspace sees.
> 
> Link: https://lore.kernel.org/all/20260401-getsockopt-v2-0-611df6771aff@debian.org/ [1]
> Link: https://lore.kernel.org/all/20260806-mcast_fix-v1-0-bed0a5518e57@debian.org/ [2]
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - sockopt_expand_out() measures the request against opt->optlen instead
>   of the iterator's remaining count, and asserts that nothing has been
>   written through iter_out yet. Comparing against the remaining count
>   made the verdict depend on how far a callback had already got, and a
>   late call would rewind the write cursor to the head of optval.
> - Cap the requested size at INT_MAX, since optlen and the getsockopt ABI
>   are int.
> - do_ip_getsockopt() writes optlen back only when ip_mc_msfget()
>   succeeded. Without the guard, a read-only optlen turned -EINVAL,
>   -ENODEV and -EADDRNOTAVAIL into -EFAULT.
> - Name IP_MSFILTER in patch 1, document the WARN_ON_ONCE() and that no
>   in-tree path reaches it, and mention sockptr_to_sockopt() losing its
>   static.
> - Link to v1: https://patch.msgid.link/20260910-getsockopt_phase6-v1-0-e681e102d5b8@debian.org

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-14 16:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-14 12:20 [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen Breno Leitao
2026-09-14 12:20 ` [PATCH net-next v2 1/2] net: add sockopt_expand_out() Breno Leitao
2026-09-14 12:20 ` [PATCH net-next v2 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t Breno Leitao
2026-09-14 13:46 ` [PATCH net-next v2 0/2] net: a sockopt_t quirk for the options that write past optlen David Laight
2026-09-14 16:19 ` Stanislav Fomichev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®