Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen
From: David Laight <hidden>
Date: 2026-08-07 16:44:06
Also in:
lkml, stable
On Thu, 06 Aug 2026 02:42:00 -0700 Breno Leitao [off-list ref] 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
quoted hunk ↗ jump to hunk
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; }