[PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc

[PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc

[Greg Kroah-Hartman]

ip_get_mcast_msfilter() and its compat sibling read gf_numsrc from
the user's buffer header and pass it to ip_mc_gsfget(), which writes:

    min(actual_sources, gf_numsrc) * sizeof(struct sockaddr_storage)

bytes back into the user's optval starting at the gf_slist_flex offset.
The only optlen check is len >= size0 (the header), so a user can pass
optlen = 144 (header only) with gf_numsrc = 4.  If the socket has at
least 4 sources joined, the kernel writes 4*128 = 512 bytes via
copy_to_sockptr_offset() past the end of the user buffer.

This is a kernel-driven userspace heap overflow: the user told the
kernel their buffer size via optlen, the kernel ignored it and used a
field inside the buffer instead.  On a real system the writes go into
adjacent userspace heap and copy_to_user does not fault on mapped heap
pages.

Clamp gf_numsrc to (len - size0) / sizeof(sockaddr_storage) before the
call so the kernel never writes past what the user provided.  The
setsockopt path already has the equivalent check
(GROUP_FILTER_SIZE(gf_numsrc) > optlen at line 790).

Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Reported-by: Anthropic
Assisted-by: gkh_clanker_t1000
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/ipv4/ip_sockglue.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index a55ef327ec93..c9bf5d223f21 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1456,6 +1456,11 @@ static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
 		return -EFAULT;
 
 	num = gsf.gf_numsrc;
+
+	if (num > (len - size0) / sizeof(struct sockaddr_storage))
+		num = (len - size0) / sizeof(struct sockaddr_storage);
+	gsf.gf_numsrc = num;
+
 	err = ip_mc_gsfget(sk, &gsf, optval,
 			   offsetof(struct group_filter, gf_slist_flex));
 	if (err)
@@ -1486,8 +1491,12 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
 	gf.gf_interface = gf32.gf_interface;
 	gf.gf_fmode = gf32.gf_fmode;
 	num = gf.gf_numsrc = gf32.gf_numsrc;
-	gf.gf_group = gf32.gf_group;
 
+	if (num > (len - size0) / sizeof(struct sockaddr_storage))
+		num = (len - size0) / sizeof(struct sockaddr_storage);
+	gf.gf_numsrc = num;
+
+	gf.gf_group = gf32.gf_group;
 	err = ip_mc_gsfget(sk, &gf, optval,
 			   offsetof(struct compat_group_filter, gf_slist_flex));
 	if (err)
-- 
2.53.0

Re: [PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc

[Paolo Abeni]

On 4/20/26 9:26 PM, Greg Kroah-Hartman wrote:
> @@ -1486,8 +1491,12 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
>  	gf.gf_interface = gf32.gf_interface;
>  	gf.gf_fmode = gf32.gf_fmode;
>  	num = gf.gf_numsrc = gf32.gf_numsrc;
> -	gf.gf_group = gf32.gf_group;
>  
> +	if (num > (len - size0) / sizeof(struct sockaddr_storage))
> +		num = (len - size0) / sizeof(struct sockaddr_storage);
> +	gf.gf_numsrc = num;

Since this is exactly the same code added above, likely a common helper
would be useful.

I guess we don't care if this would break bad application passing optval
area properly sized for gf_numsrc sockets and a small optval, right? I
don't see how to eventually save them.

/P

Re: [PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc

[Greg Kroah-Hartman]

On Thu, Apr 23, 2026 at 03:57:55PM +0200, Paolo Abeni wrote:
> On 4/20/26 9:26 PM, Greg Kroah-Hartman wrote:
> > @@ -1486,8 +1491,12 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
> >  	gf.gf_interface = gf32.gf_interface;
> >  	gf.gf_fmode = gf32.gf_fmode;
> >  	num = gf.gf_numsrc = gf32.gf_numsrc;
> > -	gf.gf_group = gf32.gf_group;
> >  
> > +	if (num > (len - size0) / sizeof(struct sockaddr_storage))
> > +		num = (len - size0) / sizeof(struct sockaddr_storage);
> > +	gf.gf_numsrc = num;
> 
> Since this is exactly the same code added above, likely a common helper
> would be useful.

Useful where else?

> I guess we don't care if this would break bad application passing optval
> area properly sized for gf_numsrc sockets and a small optval, right? I
> don't see how to eventually save them.

I couldn't see how to save them either, and if an application sends bad
data we should be rejecting it, right?  Especially as this overflows
things as-is :(

thanks,

greg k-h

Re: [PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc

[Paolo Abeni]

On 4/23/26 4:18 PM, Greg Kroah-Hartman wrote:
> On Thu, Apr 23, 2026 at 03:57:55PM +0200, Paolo Abeni wrote:
>> On 4/20/26 9:26 PM, Greg Kroah-Hartman wrote:
>>> @@ -1486,8 +1491,12 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
>>>  	gf.gf_interface = gf32.gf_interface;
>>>  	gf.gf_fmode = gf32.gf_fmode;
>>>  	num = gf.gf_numsrc = gf32.gf_numsrc;
>>> -	gf.gf_group = gf32.gf_group;
>>>  
>>> +	if (num > (len - size0) / sizeof(struct sockaddr_storage))
>>> +		num = (len - size0) / sizeof(struct sockaddr_storage);
>>> +	gf.gf_numsrc = num;
>>
>> Since this is exactly the same code added above, likely a common helper
>> would be useful.
> 
> Useful where else?

Just in these 2 functions, to avoid duplicating the logic.

Not a big deal, but it would feel nicer. Also the

	gf.gf_group = gf32.gf_group;

statement is moved around but such change is not needed, right?

>> I guess we don't care if this would break bad application passing optval
>> area properly sized for gf_numsrc sockets and a small optval, right? I
>> don't see how to eventually save them.
> 
> I couldn't see how to save them either, and if an application sends bad
> data we should be rejecting it, right?  Especially as this overflows
> things as-is :(

Agreed.

Thanks,

Paolo

Re: [PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc

[Greg Kroah-Hartman]

On Thu, Apr 23, 2026 at 04:29:06PM +0200, Paolo Abeni wrote:
> On 4/23/26 4:18 PM, Greg Kroah-Hartman wrote:
> > On Thu, Apr 23, 2026 at 03:57:55PM +0200, Paolo Abeni wrote:
> >> On 4/20/26 9:26 PM, Greg Kroah-Hartman wrote:
> >>> @@ -1486,8 +1491,12 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
> >>>  	gf.gf_interface = gf32.gf_interface;
> >>>  	gf.gf_fmode = gf32.gf_fmode;
> >>>  	num = gf.gf_numsrc = gf32.gf_numsrc;
> >>> -	gf.gf_group = gf32.gf_group;
> >>>  
> >>> +	if (num > (len - size0) / sizeof(struct sockaddr_storage))
> >>> +		num = (len - size0) / sizeof(struct sockaddr_storage);
> >>> +	gf.gf_numsrc = num;
> >>
> >> Since this is exactly the same code added above, likely a common helper
> >> would be useful.
> > 
> > Useful where else?
> 
> Just in these 2 functions, to avoid duplicating the logic.
> 
> Not a big deal, but it would feel nicer. Also the
> 
> 	gf.gf_group = gf32.gf_group;
> 
> statement is moved around but such change is not needed, right?

It's just moved down, I inserted the check/assignment before that line,
and git diff shows that as remove/add for some reason :(

thansk,

greg k-h