net/ipv4/ip_sockglue.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.