* [PATCH net 0/2] net: mcast: do not write past optlen in the source filter getsockopt
@ 2026-08-06 9:41 Breno Leitao
2026-08-06 9:42 ` [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen Breno Leitao
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Breno Leitao @ 2026-08-06 9:41 UTC (permalink / raw)
To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Breno Leitao, kernel-team, stable
getsockopt() on the multicast source filter options writes past the
buffer the caller declared. Only the fixed header is checked against
optlen. The number of sources copied out comes from gf_numsrc/
imsf_numsrc, read back from optval, and nothing bounds that count by
the space left in the buffer.
I hit this while converting the mcast getsockopt paths to sockopt_t.
Fixing it against 'net' first, so the fix is settled on its own before
the conversion goes on top.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (2):
ipv4: mcast: getsockopt: do not overwrite past optlen
ipv6: mcast: do not write past optlen in the source filter getsockopt
net/ipv4/ip_sockglue.c | 16 ++++++++++++++++
net/ipv6/ipv6_sockglue.c | 11 +++++++++++
2 files changed, 27 insertions(+)
---
base-commit: b0057c68df711bf6a62033c072ac61c4f9d3cbc1
change-id: 20260806-mcast_fix-4db13688cbb6
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen 2026-08-06 9:41 [PATCH net 0/2] net: mcast: do not write past optlen in the source filter getsockopt Breno Leitao @ 2026-08-06 9:42 ` Breno Leitao 2026-08-07 16:44 ` David Laight 2026-08-06 9:42 ` [PATCH net 2/2] ipv6: mcast: do not write past optlen in the source filter getsockopt Breno Leitao 2026-08-07 14:38 ` [PATCH net 0/2] net: " Simon Horman 2 siblings, 1 reply; 6+ messages in thread From: Breno Leitao @ 2026-08-06 9:42 UTC (permalink / raw) To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: netdev, linux-kernel, Breno Leitao, kernel-team, stable getsockopt(MCAST_MSFILTER) can write past the end of the buffer the caller declared. This is because the copy_to_user() does not respect the optlen, and can write over the allocated buffer, overwriting userspace undesired memory The amount written comes from the numsrc the caller left in optval, not from optlen. do_ip_getsockopt() reads optlen once, to check that the header fits, and then reuses the variable for the length of the reply, so by the time ip_mc_gsfget() fills the source list nothing remembers how big the buffer was. The copies go through copy_to_user(), so this reaches only the caller's own address space. setsockopt has had the matching check from the start: if (GROUP_FILTER_SIZE(gsf->gf_numsrc) > optlen) return -EINVAL; Clamp numsrc to what optlen holds rather than rejecting. Another option would be to reject (-EINVAL), but, that might break userspace _more_. For reviewing purposes: size0 is the header size, so, the available buffer is len - size0. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Breno Leitao <leitao@debian.org> --- net/ipv4/ip_sockglue.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index a55ef327ec932..2e4e19b90645b 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -1447,6 +1447,7 @@ static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, { const int size0 = offsetof(struct group_filter, gf_slist_flex); struct group_filter gsf; + unsigned int max_numsrc; int num, gsf_size; int err; @@ -1455,6 +1456,10 @@ static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, if (copy_from_sockptr(&gsf, optval, size0)) return -EFAULT; + /* Maximum number of sources that would fit in the userspace buffer*/ + max_numsrc = (len - size0) / sizeof(gsf.gf_slist_flex[0]); + gsf.gf_numsrc = min_t(u32, gsf.gf_numsrc, max_numsrc); + num = gsf.gf_numsrc; err = ip_mc_gsfget(sk, &gsf, optval, offsetof(struct group_filter, gf_slist_flex)); @@ -1474,6 +1479,7 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, { const int size0 = offsetof(struct compat_group_filter, gf_slist_flex); struct compat_group_filter gf32; + unsigned int max_numsrc; struct group_filter gf; int num; int err; @@ -1483,6 +1489,9 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, if (copy_from_sockptr(&gf32, optval, size0)) return -EFAULT; + max_numsrc = (len - size0) / sizeof(gf32.gf_slist_flex[0]); + gf32.gf_numsrc = min_t(u32, gf32.gf_numsrc, max_numsrc); + gf.gf_interface = gf32.gf_interface; gf.gf_fmode = gf32.gf_fmode; num = gf.gf_numsrc = gf32.gf_numsrc; @@ -1705,6 +1714,7 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname, switch (optname) { case IP_MSFILTER: { + unsigned int max_numsrc; struct ip_msfilter msf; if (len < IP_MSFILTER_SIZE(0)) { @@ -1715,6 +1725,12 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname, err = -EFAULT; goto out; } + /* Do not write more sources than the caller said optval can + * hold. + */ + max_numsrc = (len - IP_MSFILTER_SIZE(0)) / + sizeof(msf.imsf_slist_flex[0]); + msf.imsf_numsrc = min_t(u32, msf.imsf_numsrc, max_numsrc); err = ip_mc_msfget(sk, &msf, optval, optlen); goto out; } -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen 2026-08-06 9:42 ` [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen Breno Leitao @ 2026-08-07 16:44 ` David Laight 0 siblings, 0 replies; 6+ messages in thread From: David Laight @ 2026-08-07 16:44 UTC (permalink / raw) To: Breno Leitao Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel, kernel-team, stable On Thu, 06 Aug 2026 02:42:00 -0700 Breno Leitao <leitao@debian.org> wrote: > getsockopt(MCAST_MSFILTER) can write past the end of the buffer the caller > declared. > > This is because the copy_to_user() does not respect the optlen, and can > write over the allocated buffer, overwriting userspace undesired > memory Nak. This is just the way it is defined. The application provides a length that is just the header. As you noted the header contains details of the real buffer. There are quite a few sockopt like it, you have to support them. There may be some where the length isn't checked and is just assumed to be the right size - they have to continue to work as well. David > > The amount written comes from the numsrc the caller left in optval, not > from optlen. do_ip_getsockopt() reads optlen once, to check that the header > fits, and then reuses the variable for the length of the reply, so by the > time ip_mc_gsfget() fills the source list nothing remembers how big the > buffer was. > > The copies go through copy_to_user(), so this reaches only the caller's own > address space. > > setsockopt has had the matching check from the start: > > if (GROUP_FILTER_SIZE(gsf->gf_numsrc) > optlen) > return -EINVAL; > > Clamp numsrc to what optlen holds rather than rejecting. Another option > would be to reject (-EINVAL), but, that might break userspace _more_. > > For reviewing purposes: size0 is the header size, so, the available > buffer is len - size0. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Cc: stable@vger.kernel.org > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > net/ipv4/ip_sockglue.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c > index a55ef327ec932..2e4e19b90645b 100644 > --- a/net/ipv4/ip_sockglue.c > +++ b/net/ipv4/ip_sockglue.c > @@ -1447,6 +1447,7 @@ static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, > { > const int size0 = offsetof(struct group_filter, gf_slist_flex); > struct group_filter gsf; > + unsigned int max_numsrc; > int num, gsf_size; > int err; > > @@ -1455,6 +1456,10 @@ static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, > if (copy_from_sockptr(&gsf, optval, size0)) > return -EFAULT; > > + /* Maximum number of sources that would fit in the userspace buffer*/ > + max_numsrc = (len - size0) / sizeof(gsf.gf_slist_flex[0]); > + gsf.gf_numsrc = min_t(u32, gsf.gf_numsrc, max_numsrc); > + > num = gsf.gf_numsrc; > err = ip_mc_gsfget(sk, &gsf, optval, > offsetof(struct group_filter, gf_slist_flex)); > @@ -1474,6 +1479,7 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, > { > const int size0 = offsetof(struct compat_group_filter, gf_slist_flex); > struct compat_group_filter gf32; > + unsigned int max_numsrc; > struct group_filter gf; > int num; > int err; > @@ -1483,6 +1489,9 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval, > if (copy_from_sockptr(&gf32, optval, size0)) > return -EFAULT; > > + max_numsrc = (len - size0) / sizeof(gf32.gf_slist_flex[0]); > + gf32.gf_numsrc = min_t(u32, gf32.gf_numsrc, max_numsrc); > + > gf.gf_interface = gf32.gf_interface; > gf.gf_fmode = gf32.gf_fmode; > num = gf.gf_numsrc = gf32.gf_numsrc; > @@ -1705,6 +1714,7 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname, > switch (optname) { > case IP_MSFILTER: > { > + unsigned int max_numsrc; > struct ip_msfilter msf; > > if (len < IP_MSFILTER_SIZE(0)) { > @@ -1715,6 +1725,12 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname, > err = -EFAULT; > goto out; > } > + /* Do not write more sources than the caller said optval can > + * hold. > + */ > + max_numsrc = (len - IP_MSFILTER_SIZE(0)) / > + sizeof(msf.imsf_slist_flex[0]); > + msf.imsf_numsrc = min_t(u32, msf.imsf_numsrc, max_numsrc); > err = ip_mc_msfget(sk, &msf, optval, optlen); > goto out; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 2/2] ipv6: mcast: do not write past optlen in the source filter getsockopt 2026-08-06 9:41 [PATCH net 0/2] net: mcast: do not write past optlen in the source filter getsockopt Breno Leitao 2026-08-06 9:42 ` [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen Breno Leitao @ 2026-08-06 9:42 ` Breno Leitao 2026-08-07 16:44 ` David Laight 2026-08-07 14:38 ` [PATCH net 0/2] net: " Simon Horman 2 siblings, 1 reply; 6+ messages in thread From: Breno Leitao @ 2026-08-06 9:42 UTC (permalink / raw) To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: netdev, linux-kernel, Breno Leitao, kernel-team, stable getsockopt(MCAST_MSFILTER) on an IPv6 socket overruns the caller's buffer the same way the IPv4 one does. ip6_mc_msfget() fills the source list from the numsrc left in optval, and nothing compares that against optlen, which ipv6_get_msfilter() has already reused for the length of the reply. Clamp numsrc to what optlen holds, as the IPv4 side now does. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Breno Leitao <leitao@debian.org> --- net/ipv6/ipv6_sockglue.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c index b4c977434c2e0..2c3fbde7cb058 100644 --- a/net/ipv6/ipv6_sockglue.c +++ b/net/ipv6/ipv6_sockglue.c @@ -1012,6 +1012,7 @@ static int ipv6_get_msfilter(struct sock *sk, sockptr_t optval, { const int size0 = offsetof(struct group_filter, gf_slist_flex); struct group_filter gsf; + unsigned int max_numsrc; int num; int err; @@ -1021,6 +1022,11 @@ static int ipv6_get_msfilter(struct sock *sk, sockptr_t optval, return -EFAULT; if (gsf.gf_group.ss_family != AF_INET6) return -EADDRNOTAVAIL; + + /* Number of sources that would fit in the userspace buffer */ + max_numsrc = (len - size0) / sizeof(gsf.gf_slist_flex[0]); + gsf.gf_numsrc = min_t(u32, gsf.gf_numsrc, max_numsrc); + num = gsf.gf_numsrc; sockopt_lock_sock(sk); err = ip6_mc_msfget(sk, &gsf, optval, size0); @@ -1041,6 +1047,7 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval, { const int size0 = offsetof(struct compat_group_filter, gf_slist_flex); struct compat_group_filter gf32; + unsigned int max_numsrc; struct group_filter gf; int err; int num; @@ -1050,6 +1057,10 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval, if (copy_from_sockptr(&gf32, optval, size0)) return -EFAULT; + + max_numsrc = (len - size0) / sizeof(gf32.gf_slist_flex[0]); + gf32.gf_numsrc = min_t(u32, gf32.gf_numsrc, max_numsrc); + gf.gf_interface = gf32.gf_interface; gf.gf_fmode = gf32.gf_fmode; num = gf.gf_numsrc = gf32.gf_numsrc; -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] ipv6: mcast: do not write past optlen in the source filter getsockopt 2026-08-06 9:42 ` [PATCH net 2/2] ipv6: mcast: do not write past optlen in the source filter getsockopt Breno Leitao @ 2026-08-07 16:44 ` David Laight 0 siblings, 0 replies; 6+ messages in thread From: David Laight @ 2026-08-07 16:44 UTC (permalink / raw) To: Breno Leitao Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel, kernel-team, stable On Thu, 06 Aug 2026 02:42:01 -0700 Breno Leitao <leitao@debian.org> wrote: > getsockopt(MCAST_MSFILTER) on an IPv6 socket overruns the caller's buffer > the same way the IPv4 one does. ip6_mc_msfget() fills the source list from > the numsrc left in optval, and nothing compares that against optlen, which > ipv6_get_msfilter() has already reused for the length of the reply. > > Clamp numsrc to what optlen holds, as the IPv4 side now does. Nak, same as IPv4. David > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Cc: stable@vger.kernel.org > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > net/ipv6/ipv6_sockglue.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c > index b4c977434c2e0..2c3fbde7cb058 100644 > --- a/net/ipv6/ipv6_sockglue.c > +++ b/net/ipv6/ipv6_sockglue.c > @@ -1012,6 +1012,7 @@ static int ipv6_get_msfilter(struct sock *sk, sockptr_t optval, > { > const int size0 = offsetof(struct group_filter, gf_slist_flex); > struct group_filter gsf; > + unsigned int max_numsrc; > int num; > int err; > > @@ -1021,6 +1022,11 @@ static int ipv6_get_msfilter(struct sock *sk, sockptr_t optval, > return -EFAULT; > if (gsf.gf_group.ss_family != AF_INET6) > return -EADDRNOTAVAIL; > + > + /* Number of sources that would fit in the userspace buffer */ > + max_numsrc = (len - size0) / sizeof(gsf.gf_slist_flex[0]); > + gsf.gf_numsrc = min_t(u32, gsf.gf_numsrc, max_numsrc); > + > num = gsf.gf_numsrc; > sockopt_lock_sock(sk); > err = ip6_mc_msfget(sk, &gsf, optval, size0); > @@ -1041,6 +1047,7 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval, > { > const int size0 = offsetof(struct compat_group_filter, gf_slist_flex); > struct compat_group_filter gf32; > + unsigned int max_numsrc; > struct group_filter gf; > int err; > int num; > @@ -1050,6 +1057,10 @@ static int compat_ipv6_get_msfilter(struct sock *sk, sockptr_t optval, > > if (copy_from_sockptr(&gf32, optval, size0)) > return -EFAULT; > + > + max_numsrc = (len - size0) / sizeof(gf32.gf_slist_flex[0]); > + gf32.gf_numsrc = min_t(u32, gf32.gf_numsrc, max_numsrc); > + > gf.gf_interface = gf32.gf_interface; > gf.gf_fmode = gf32.gf_fmode; > num = gf.gf_numsrc = gf32.gf_numsrc; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/2] net: mcast: do not write past optlen in the source filter getsockopt 2026-08-06 9:41 [PATCH net 0/2] net: mcast: do not write past optlen in the source filter getsockopt Breno Leitao 2026-08-06 9:42 ` [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen Breno Leitao 2026-08-06 9:42 ` [PATCH net 2/2] ipv6: mcast: do not write past optlen in the source filter getsockopt Breno Leitao @ 2026-08-07 14:38 ` Simon Horman 2 siblings, 0 replies; 6+ messages in thread From: Simon Horman @ 2026-08-07 14:38 UTC (permalink / raw) To: Breno Leitao Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel, kernel-team, stable On Thu, Aug 06, 2026 at 02:41:59AM -0700, Breno Leitao wrote: > getsockopt() on the multicast source filter options writes past the > buffer the caller declared. Only the fixed header is checked against > optlen. The number of sources copied out comes from gf_numsrc/ > imsf_numsrc, read back from optval, and nothing bounds that count by > the space left in the buffer. > > I hit this while converting the mcast getsockopt paths to sockopt_t. > Fixing it against 'net' first, so the fix is settled on its own before > the conversion goes on top. > > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > Breno Leitao (2): > ipv4: mcast: getsockopt: do not overwrite past optlen > ipv6: mcast: do not write past optlen in the source filter getsockopt > > net/ipv4/ip_sockglue.c | 16 ++++++++++++++++ > net/ipv6/ipv6_sockglue.c | 11 +++++++++++ > 2 files changed, 27 insertions(+) For the series: Reviewed-by: Simon Horman <horms@kernel.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-07 16:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-06 9:41 [PATCH net 0/2] net: mcast: do not write past optlen in the source filter getsockopt Breno Leitao 2026-08-06 9:42 ` [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen Breno Leitao 2026-08-07 16:44 ` David Laight 2026-08-06 9:42 ` [PATCH net 2/2] ipv6: mcast: do not write past optlen in the source filter getsockopt Breno Leitao 2026-08-07 16:44 ` David Laight 2026-08-07 14:38 ` [PATCH net 0/2] net: " Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox