* [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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
2026-08-10 12:48 ` Breno Leitao
0 siblings, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread
* Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen
2026-08-07 16:44 ` David Laight
@ 2026-08-10 12:48 ` Breno Leitao
2026-08-10 21:21 ` David Laight
0 siblings, 1 reply; 11+ messages in thread
From: Breno Leitao @ 2026-08-10 12:48 UTC (permalink / raw)
To: David Laight
Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel,
kernel-team, stable
Hello David,
On Fri, Aug 07, 2026 at 05:44:02PM +0100, David Laight wrote:
> 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.
Thanks for the review.
The generic getsockopt(2) contract says otherwise.
For getsockopt(), optlen is a value-result argument, initially
containing the size of the buffer pointed to by optval, and modified
on return to indicate the actual size of the value returned.
You are saying that we have userspace program in the wild that doesn't honour
the contract above, right?
On the set side (setsockopt) of the same option takes the opposite view.
It rejects calls if optlen is not properly set.
err = -EINVAL;
if (GROUP_FILTER_SIZE(gsf->gf_numsrc) > optlen)
goto out_free_gsf;
Back to getsockopt(2), I was trying to look for users it, and glibc
passes the full length for both options, with optlen and numsrc coming
from the same variable:
/* sysdeps/unix/sysv/linux/getsourcefilter.c */
socklen_t needed = GROUP_FILTER_SIZE(*numsrc);
gf->gf_numsrc = *numsrc;
result = __getsockopt (s, sol, MCAST_MSFILTER, gf, &needed);
and
/* sysdeps/unix/sysv/linux/getipv4sourcefilter.c */
socklen_t needed = IP_MSFILTER_SIZE (*numsrc);
imsf->imsf_numsrc = *numsrc;
int result = __getsockopt (s, SOL_IP, IP_MSFILTER, imsf, &needed);
So the clamp is a no-op for every caller that goes through libc.
So my conclusion is that this is a bug: setsockopt and glibc both treat optlen
as the buffer size, and I could not find anything relying on the get side not
doing so.
If you know of a caller that does, I will drop the series - I would rather
look at it than assume it exists and assume that it will break someone that is
leveraging a buggy behavior.
Thanks,
--breno
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen
2026-08-10 12:48 ` Breno Leitao
@ 2026-08-10 21:21 ` David Laight
2026-08-11 12:19 ` Breno Leitao
0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2026-08-10 21:21 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 Mon, 10 Aug 2026 05:48:53 -0700
Breno Leitao <leitao@debian.org> wrote:
> Hello David,
>
> On Fri, Aug 07, 2026 at 05:44:02PM +0100, David Laight wrote:
> > 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.
>
> Thanks for the review.
>
> The generic getsockopt(2) contract says otherwise.
>
> For getsockopt(), optlen is a value-result argument, initially
> containing the size of the buffer pointed to by optval, and modified
> on return to indicate the actual size of the value returned.
>
> You are saying that we have userspace program in the wild that doesn't honour
> the contract above, right?
I've just looked at the old history, since the code was added in 2.4.22 'optlen'
has only needed to be the size of the fixed structure on entry and has been
been updated to be the total size on exit.
The maximum size of the buffer comes from its sl_count field.
Code that doesn't use the glibc wrapper could be relying on it.
There were definitely places where the driver code has traditionally not
checked the length at all - and userspace wouldn't have set it.
The last might have been in the decnet code.
I'm pretty sure there are other places where the length provided to getsockopt()
is only that of the fixed header, variable data then follows the header.
IIRC there is a recently added one for async io.
Can't remember where.
It checks the 'header' size and takes the full length from within the header.
That one definitely requires (and checks) for the short length.
> On the set side (setsockopt) of the same option takes the opposite view.
> It rejects calls if optlen is not properly set.
>
> err = -EINVAL;
> if (GROUP_FILTER_SIZE(gsf->gf_numsrc) > optlen)
> goto out_free_gsf;
It has always been asymmetric.
David
>
> Back to getsockopt(2), I was trying to look for users it, and glibc
> passes the full length for both options, with optlen and numsrc coming
> from the same variable:
>
> /* sysdeps/unix/sysv/linux/getsourcefilter.c */
> socklen_t needed = GROUP_FILTER_SIZE(*numsrc);
> gf->gf_numsrc = *numsrc;
> result = __getsockopt (s, sol, MCAST_MSFILTER, gf, &needed);
>
> and
>
> /* sysdeps/unix/sysv/linux/getipv4sourcefilter.c */
> socklen_t needed = IP_MSFILTER_SIZE (*numsrc);
> imsf->imsf_numsrc = *numsrc;
> int result = __getsockopt (s, SOL_IP, IP_MSFILTER, imsf, &needed);
>
> So the clamp is a no-op for every caller that goes through libc.
>
> So my conclusion is that this is a bug: setsockopt and glibc both treat optlen
> as the buffer size, and I could not find anything relying on the get side not
> doing so.
>
> If you know of a caller that does, I will drop the series - I would rather
> look at it than assume it exists and assume that it will break someone that is
> leveraging a buggy behavior.
>
> Thanks,
> --breno
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen
2026-08-10 21:21 ` David Laight
@ 2026-08-11 12:19 ` Breno Leitao
2026-08-11 15:15 ` Jakub Kicinski
0 siblings, 1 reply; 11+ messages in thread
From: Breno Leitao @ 2026-08-11 12:19 UTC (permalink / raw)
To: David Laight
Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel,
kernel-team, stable
On Mon, Aug 10, 2026 at 10:21:59PM +0100, David Laight wrote:
> > You are saying that we have userspace program in the wild that doesn't honour
> > the contract above, right?
>
> I've just looked at the old history, since the code was added in 2.4.22 'optlen'
> has only needed to be the size of the fixed structure on entry and has been
> been updated to be the total size on exit.
> The maximum size of the buffer comes from its sl_count field.
> Code that doesn't use the glibc wrapper could be relying on it.
>
> There were definitely places where the driver code has traditionally not
> checked the length at all - and userspace wouldn't have set it.
> The last might have been in the decnet code.
> I'm pretty sure there are other places where the length provided to getsockopt()
> is only that of the fixed header, variable data then follows the header.
>
> IIRC there is a recently added one for async io.
> Can't remember where.
> It checks the 'header' size and takes the full length from within the header.
> That one definitely requires (and checks) for the short length.
Right, but I want to back up and figure out whether this is a bug that
should be fixed, or a bug that would break userpsace if we fix it.
I'm saying the kernel shouldn't write past optlen. If userspace
doesn't provide a big enough buffer, it shouldn't assume there is one
behind it to write into.
Looking at the existing users, I haven't found any that pass a smaller
optlen than needed, so I don't see anyone being impacted.
On the other hand, you're saying this is how it has always behaved
and there is userspace out there relying on it, so changing it now
would break things, right?
Depending on the answer, we have two paths ahead:
1) We declare these bugs and fix them, and proceed with sanity
2) We need to implement quirks in the getsockopt, which will resize
"optlen" mid-air by looking at headers fields
So my question to you: can you point me to actual software that
passes a "small" optlen and expects the kernel to write past it? That
would help to decide about the two options above.
Thanks,
--breno
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen
2026-08-11 12:19 ` Breno Leitao
@ 2026-08-11 15:15 ` Jakub Kicinski
2026-08-11 17:51 ` David Laight
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2026-08-11 15:15 UTC (permalink / raw)
To: Breno Leitao
Cc: David Laight, David Ahern, Ido Schimmel, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, netdev, linux-kernel,
kernel-team, stable, Stanislav Fomichev
On Tue, 11 Aug 2026 05:19:17 -0700 Breno Leitao wrote:
> So my question to you: can you point me to actual software that
> passes a "small" optlen and expects the kernel to write past it? That
> would help to decide about the two options above.
If you are very confident that no such SW exists - we can try to queue
this up for -next. (TBH I'm not, mcast specifically may be full of
strange one off manually written user space (as opposed to common libraries)).
If we decide to change the behavior- we will probably have to wait
until this makes it to an LTS release + some time for people to deploy.
It can't be a fix.
So practically speaking it may be more expedient to add some hacks to
cater to this case in the conversion, and then remove the hack. That'd
be easier to revert if someone pipes up later that we broke their SW.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen
2026-08-11 15:15 ` Jakub Kicinski
@ 2026-08-11 17:51 ` David Laight
0 siblings, 0 replies; 11+ messages in thread
From: David Laight @ 2026-08-11 17:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Breno Leitao, David Ahern, Ido Schimmel, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, netdev, linux-kernel,
kernel-team, stable, Stanislav Fomichev
On Tue, 11 Aug 2026 08:15:43 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 11 Aug 2026 05:19:17 -0700 Breno Leitao wrote:
> > So my question to you: can you point me to actual software that
> > passes a "small" optlen and expects the kernel to write past it? That
> > would help to decide about the two options above.
>
> If you are very confident that no such SW exists - we can try to queue
> this up for -next. (TBH I'm not, mcast specifically may be full of
> strange one off manually written user space (as opposed to common libraries)).
>
> If we decide to change the behavior- we will probably have to wait
> until this makes it to an LTS release + some time for people to deploy.
> It can't be a fix.
>
> So practically speaking it may be more expedient to add some hacks to
> cater to this case in the conversion, and then remove the hack. That'd
> be easier to revert if someone pipes up later that we broke their SW.
>
I'm also pretty sure there is another sockopt that uses a count inside
the header to indicate the actual buffer length.
For that option the length supplied has to match the expected header length
and there is code to write back the corrected length with an error code.
I did a search earlier today but failed to find it again.
It would be in the patches/changes I did (locally) that made the getsockopt
protocol functions return either a negative errno or a positive length.
But I think those were done an SDD that pretty much lost all its contents
while powered off for some time.
I never did decide on the best way to handle code that wanted to update
'optlen' and return an error.
It is annoying because there are only a handful of cases in the entire source.
I would suggest (again) that these changes be done starting with the syscall
'glue' and adding an extra getsockopt_new() to the function call table(s).
Then changing the protocols one by one to provide the new function and
finally deleting the old entry.
That way each protocol code only needs changing once.
I'd also wrap the copy_to_iter() in an inline function so that most code
doesn't have to care about the implementation.
David
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-08-11 17:51 UTC | newest]
Thread overview: 11+ 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-10 12:48 ` Breno Leitao
2026-08-10 21:21 ` David Laight
2026-08-11 12:19 ` Breno Leitao
2026-08-11 15:15 ` Jakub Kicinski
2026-08-11 17:51 ` 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