Re: [PATCH net 1/2] ipv4: mcast: getsockopt: do not overwrite past optlen

On Mon, 10 Aug 2026 05:48:53 -0700
Breno Leitao [off-list ref] wrote:

Hello David,

On Fri, Aug 07, 2026 at 05:44:02PM +0100, David Laight wrote:

quoted

On Thu, 06 Aug 2026 02:42:00 -0700
Breno Leitao [off-list ref] wrote:
  

quoted

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