[PATCH net-next 0/2] net: a sockopt_t quirk for the options that write past optlen

[PATCH net-next 0/2] net: a sockopt_t quirk for the options that write past optlen

[Breno Leitao]

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>
---
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    | 25 +++++++++++++++++++++++++
 net/ipv4/igmp.c        | 23 ++++++++++++++---------
 net/ipv4/ip_sockglue.c | 10 +++++++++-
 net/socket.c           |  4 ++--
 5 files changed, 52 insertions(+), 13 deletions(-)
---
base-commit: 548b86839f7fb819a4d6c83b71c73ec378d24275
change-id: 20260909-getsockopt_phase6-c7c96a21b062

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

[PATCH net-next 1/2] net: add sockopt_expand_out()

[Breno Leitao]

Add sockopt_expand_out() to grow opt->iter_out mid-air.

It is a no-op unless the proper size outruns optlen (i.e, some
not-well-behaved userspace program calling it).

In this case, only a user buffer can be longer than optlen says, so
a kernel-backed optval keeps the bounded iterator and the callback gets
-EINVAL if it asks to grow.

This whole quirk is added to:

  1) Avoid breaking userspace
  2) Making the quirk explict
     * Instead of protocol doing implict assumping like this.

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

diff --git a/include/linux/net.h b/include/linux/net.h
index 470100ae710773..de0ed362b37794 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -70,6 +70,31 @@ 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 <= iov_iter_count(&opt->iter_out))
+		return 0;
+
+	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
+		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

[PATCH net-next 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t

[Breno Leitao]

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.

This is a special case, where optlen might only point
to the header, and the real structure size is inside the header.

This is nasty, but, in order to avoid breaking userspace, we need to
preserve the same mechanism, by:

1) Only applying it for userspace address, otherwise it is too risky
2) Assume there is room to support the new size (in userspace)

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..c58e565f2a5aa7 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 (copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
+			err = -EFAULT;
 		goto out;
 	}
 	case MCAST_MSFILTER:

-- 
2.53.0-Meta

Re: [PATCH net-next 1/2] net: add sockopt_expand_out()

[Stanislav Fomichev]

On 09/10, Breno Leitao wrote:
> Add sockopt_expand_out() to grow opt->iter_out mid-air.
> 
> It is a no-op unless the proper size outruns optlen (i.e, some
> not-well-behaved userspace program calling it).
> 
> In this case, only a user buffer can be longer than optlen says, so
> a kernel-backed optval keeps the bounded iterator and the callback gets
> -EINVAL if it asks to grow.
> 
> This whole quirk is added to:
> 
>   1) Avoid breaking userspace
>   2) Making the quirk explict
>      * Instead of protocol doing implict assumping like this.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  include/linux/net.h | 25 +++++++++++++++++++++++++
>  net/socket.c        |  4 ++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 470100ae710773..de0ed362b37794 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -70,6 +70,31 @@ 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 <= iov_iter_count(&opt->iter_out))
> +		return 0;
> +
> +	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
> +		return -EINVAL;

nit: if you end up re-spinning for some reason, maybe swap these two?
I always get confused by the count vs len of iov (iov_iter_count vs
iter_iov_len). Because I think count for ubuf is len because of the
aliasing? (and then, if !iter_is_ubuf check is first, at least iov_iter_count
will 100% be ubuf specific)

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

Re: [PATCH net-next 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t

[Stanislav Fomichev]

On 09/10, Breno Leitao wrote:
> 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.
> 
> This is a special case, where optlen might only point
> to the header, and the real structure size is inside the header.
> 
> This is nasty, but, in order to avoid breaking userspace, we need to
> preserve the same mechanism, by:
> 
> 1) Only applying it for userspace address, otherwise it is too risky
> 2) Assume there is room to support the new size (in userspace)
> 
> 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>

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

Re: [PATCH net-next 1/2] net: add sockopt_expand_out()

[Breno Leitao]

On Fri, Sep 11, 2026 at 08:56:23AM -0700, Stanislav Fomichev wrote:
> On 09/10, Breno Leitao wrote:
> >
> > +	if (size <= iov_iter_count(&opt->iter_out))
> > +		return 0;
> > +
> > +	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
> > +		return -EINVAL;
> 
> nit: if you end up re-spinning for some reason, maybe swap these two?

I am not sure we want to swap these two. This is the reason:

1) The first check (size <= iov_iter_count(&opt->iter_out)), check if we
   need the "hack" or not.

   a) If we don't need the tack, then we are fine and this could be called
      from user or kernel.
   
   b) for the hack (which happens in the next line -- iov_iter_ubuf()), it
      needs to be ubuf.

So, this force kernel callers to use the right optlen, avoiding hitting
the same issue as userspace.

Re: [PATCH net-next 1/2] net: add sockopt_expand_out()

[David Laight]

On Fri, 11 Sep 2026 09:09:22 -0700
Breno Leitao <leitao@debian.org> wrote:

> On Fri, Sep 11, 2026 at 08:56:23AM -0700, Stanislav Fomichev wrote:
> > On 09/10, Breno Leitao wrote:  
> > >
> > > +	if (size <= iov_iter_count(&opt->iter_out))
> > > +		return 0;
> > > +
> > > +	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
> > > +		return -EINVAL;  
> > 
> > nit: if you end up re-spinning for some reason, maybe swap these two?  
> 
> I am not sure we want to swap these two. This is the reason:
> 
> 1) The first check (size <= iov_iter_count(&opt->iter_out)), check if we
>    need the "hack" or not.
> 
>    a) If we don't need the tack, then we are fine and this could be called
>       from user or kernel.
>    
>    b) for the hack (which happens in the next line -- iov_iter_ubuf()), it
>       needs to be ubuf.
> 
> So, this force kernel callers to use the right optlen, avoiding hitting
> the same issue as userspace.
> 

kernel users might need to specify both lengths.
That is the value of optlen and the length of the buffer.
For 99.9% of the calls they will be the same.

David

Re: [PATCH net-next 1/2] net: add sockopt_expand_out()

[Stanislav Fomichev]

On 09/11, Breno Leitao wrote:
> On Fri, Sep 11, 2026 at 08:56:23AM -0700, Stanislav Fomichev wrote:
> > On 09/10, Breno Leitao wrote:
> > >
> > > +	if (size <= iov_iter_count(&opt->iter_out))
> > > +		return 0;
> > > +
> > > +	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
> > > +		return -EINVAL;
> > 
> > nit: if you end up re-spinning for some reason, maybe swap these two?
> 
> I am not sure we want to swap these two. This is the reason:
> 
> 1) The first check (size <= iov_iter_count(&opt->iter_out)), check if we
>    need the "hack" or not.
> 
>    a) If we don't need the tack, then we are fine and this could be called
>       from user or kernel.
>    
>    b) for the hack (which happens in the next line -- iov_iter_ubuf()), it
>       needs to be ubuf.
> 
> So, this force kernel callers to use the right optlen, avoiding hitting
> the same issue as userspace.

Hmm, that feels a bit too magical :-/ Who are the kernel callers? Do we
have any? Is it reachable via bpf or something else?

Re: [PATCH net-next 1/2] net: add sockopt_expand_out()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 4

- [Medium] sockopt_expand_out() (include/linux/net.h:82-93) mixes two
  length semantics: the growth predicate compares @size against…
- [Low] Undocumented linkage change: net/socket.c drops `static` from
  sockptr_to_sockopt() and include/linux/net.h publishes its prototype,…
- [Low] sockopt_expand_out() (include/linux/net.h:82-93) rejects a
  kernel-backed optval with WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out))…
- [Low] sockopt_expand_out() is introduced with no callers and the commit
  message never names the options that 'write past optlen today', so its…
- [Low] sockopt_expand_out() installs a caller-supplied size_t as the new
  count of a user-buffer iterator with no ceiling at all — no INT_MAX…

--

net: add sockopt_expand_out()

This adds a sockopt_expand_out() helper that regrows opt->iter_out past the
optlen userspace declared, for the getsockopt options whose reply length is
derived from a count stored in optval instead of from optlen.  It also makes
sockptr_to_sockopt() non-static and declares it in include/linux/net.h.

A few questions below.

> diff --git a/include/linux/net.h b/include/linux/net.h
> index 470100ae71077..de0ed362b3779 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -70,6 +70,31 @@ 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)
> +{

[Severity: Low]
This isn't a bug, but the helper lands with no callers and the changelog
never names the options whose "reply is sized by a count the caller left in
optval".

The first user shows up in the following patch of the series, where
ip_mc_msfget() in net/ipv4/igmp.c calls it for the IP_MSFILTER reply.

Could the changelog name IP_MSFILTER, so the semantics of the new helper can
be read against a concrete conversion?

> +	if (size <= iov_iter_count(&opt->iter_out))
> +		return 0;
> +

[Severity: Medium]
This test compares @size against iov_iter_count(&opt->iter_out), which is the
number of bytes still remaining in the iterator, while the growth action below
re-anchors at the head of optval:

	iov_iter_ubuf(&opt->iter_out, ITER_DEST, opt->iter_out.ubuf, size);

For ITER_UBUF, iov_iter_advance() only bumps iov_offset and shrinks count;
ubuf keeps pointing at the head of the buffer.

So if a callback ever calls sockopt_expand_out() after copying part of its
reply, does the write cursor get rewound and the already-copied bytes
overwritten?

The same mismatch has two more effects: a @size that fits within the declared
optlen can still fall into the kernel-backed -EINVAL branch once the iterator
has been advanced, and the no-op branch can return 0 with the cursor sitting
mid-buffer.

opt->iter_in and opt->optlen also keep describing the old, smaller buffer
after the expansion.  Only ip_mc_msfget() assigning opt->optlen = msf_size
itself covers that up:

	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) !=

Nothing is broken today, since that single caller invokes the helper as the
first operation on a freshly built iterator, and the "call it before writing
through opt->iter_out" rule is stated in the comment.

Would comparing against opt->optlen (or iov_offset + count) and refusing a
partially consumed iterator make the helper's verdict independent of how much
has already been written?

> +	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
> +		return -EINVAL;
> +

[Severity: Low]
The changelog says only that "a kernel-backed optval keeps the bounded
iterator and the callback gets -EINVAL if it asks to grow", but this path also
emits a splat.

sockptr_to_sockopt() builds an ITER_KVEC iter_out whenever optval.is_kernel:

	if (optval.is_kernel) {
		kvec->iov_base = optval.kernel;
		kvec->iov_len = koptlen;
		iov_iter_kvec(&opt->iter_out, ITER_DEST, kvec, 1, koptlen);

so the state this WARN treats as impossible is a supported construction.

Following the reachability at the end of the series, the only caller of
sockopt_expand_out() is ip_mc_msfget(), reached only through the IP_MSFILTER
case of do_ip_getsockopt(), and the only kernel-backed entry into
do_ip_getsockopt() is sol_ip_sockopt() in net/core/filter.c:

	switch (optname) {
	case IP_TOS:
	case IP_TRANSPARENT:
		if (*optlen != sizeof(int))
			return -EINVAL;
		break;
	default:
		return -EINVAL;
	}

so IP_MSFILTER is rejected before any sockopt_t is built and no in-tree path
can trip the WARN.

Could the changelog mention the WARN as well, given it advertises only the
-EINVAL?

> +	iov_iter_ubuf(&opt->iter_out, ITER_DEST, opt->iter_out.ubuf, size);
> +
> +	return 0;
> +}
> +

[Severity: Low]
The caller-supplied @size becomes the new count of a user-buffer iterator with
no ceiling here: no INT_MAX clamp, even though opt->optlen and the getsockopt
optlen ABI are int, and no MAX_RW_COUNT limit of the kind import_ubuf()
applies to the same ITER_UBUF construction.  The comment above says @size
comes from "a count the caller left in optval", i.e. from user data.

At the only call site the value is bounded, so nothing overflows today:

	copycount = count < msf->imsf_numsrc ? count : msf->imsf_numsrc;
	len = flex_array_size(psl, sl_addr, copycount);
	msf->imsf_numsrc = count;
	msf_size = IP_MSFILTER_SIZE(copycount);

with psl->sl_count capped by net.ipv4.sysctl_igmp_max_msf, and the write past
the declared optlen matches what IP_MSFILTER already did before the
conversion.

Would a clamp on @size here, or a note that the helper is meant only for the
legacy options, be worth adding for future consumers?

> +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 c05d86e63abf7..29a0f7f8e2cab 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)

[Severity: Low]
The subject and changelog cover only sockopt_expand_out(), but this hunk also
drops static from sockptr_to_sockopt() and the header hunk publishes its
prototype.

The cross-file user only appears in the next patch, where do_ip_getsockopt()
in net/ipv4/ip_sockglue.c calls it:

	err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);

Since ip_sockglue.o is obj-y and CONFIG_INET is a bool, no EXPORT_SYMBOL is
needed and nothing breaks.  Could the changelog say why sockptr_to_sockopt()
is being made global here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260910-getsockopt_phase6-v1-0-e681e102d5b8%40debian.org

Re: [PATCH net-next 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] In do_ip_getsockopt() (net/ipv4/ip_sockglue.c, case
  IP_MSFILTER), the output length is now written back to userspace…

--

ipv4: igmp: convert ip_mc_msfget() to sockopt_t

This converts ip_mc_msfget() from the sockptr_t optval/optlen pair to a
sockopt_t built at the do_ip_getsockopt() call site, and replaces the
optval copies with sequential copy_to_iter() calls after growing the
output iterator with sockopt_expand_out().

> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index d56355aca7977..144fca158adcb 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c

[ ... ]

> @@ -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 a55ef327ec932..c58e565f2a5aa 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c

[ ... ]

> @@ -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 (copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
> +			err = -EFAULT;
>  		goto out;

[Severity: Medium]
Should this write-back be guarded by if (!err)?

Before this patch the *optlen store lived inside ip_mc_msfget() and only
ran once the lookup had succeeded:

	msf_size = IP_MSFILTER_SIZE(copycount);
	if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) ||
	    copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) {
		return -EFAULT;
	}

All of the ip_mc_msfget() failure returns left the user's optlen word
untouched: !ipv4_is_multicast(addr) returning -EINVAL, !in_dev returning
-ENODEV, and the !pmc "must have a prior join" case falling through to
done: with err = -EADDRNOTAVAIL.

Now do_ip_getsockopt() writes opt.optlen back on every return from
ip_mc_msfget(), including those three error paths and the new -EINVAL
from sockopt_expand_out(), since opt->optlen is only assigned on the
success path in ip_mc_msfget(). Is that change of behaviour on failure
intended?

There is a second effect from the missing guard: if the optlen pointer is
readable but not writable, copy_to_sockptr() fails and err is replaced
with -EFAULT, so -EINVAL, -ENODEV and -EADDRNOTAVAIL are no longer
distinguishable by userspace. The earlier copy_from_sockptr(&len, optlen,
...) and the copy_from_sockptr() inside sockptr_to_sockopt() both succeed
on a read-only mapping, so this state is reachable.

Would something like this preserve the old semantics?

	err = ip_mc_msfget(sk, &msf, &opt);
	if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
		err = -EFAULT;

The commit message describes the change as a conversion, with "the bytes
land where they did", and does not mention that the optlen store moved to
the caller and now also runs when ip_mc_msfget() fails. Could that be
called out explicitly?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260910-getsockopt_phase6-v1-0-e681e102d5b8%40debian.org