Re: [PATCH net-next v2 2/7] net: cap sockopt_expand_out() at MAX_RW_COUNT

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Most of this concern does not hold once the whole series is
  applied.

--

net: cap sockopt_expand_out() at MAX_RW_COUNT

This patch moves the size check in sockopt_expand_out() so it runs before
the optlen early return. It also lowers the limit from INT_MAX to
MAX_RW_COUNT. Any size that iov_iter_revert() could not unroll later is
now rejected up front.

> sockopt_expand_out() grows the output iterator up to INT_MAX, but its
> callers rewind it with iov_iter_revert() once they know the real reply
> size. iov_iter_revert() refuses to unroll past MAX_RW_COUNT:

[ ... ]

> Fixes: 0093f7db9c47 ("net: add sockopt_expand_out()")

[Severity: Low]
Is the present tense accurate at this point in the series?

At this commit, ip_mc_msfget() in net/ipv4/igmp.c is the only caller of
sockopt_expand_out(). It writes forward through opt->iter_out and never
calls iov_iter_revert(). The same holds at 0093f7db9c47, so neither
commit can reach the WARN_ON in iov_iter_revert().

The callers that rewind come from later patches in this series: "ipv6:
mcast: convert ip6_mc_msfget() to sockopt_t" and "ipv4: igmp: convert
ip_mc_gsfget() to sockopt_t". For example:

ip_get_mcast_msfilter()
    iov_iter_revert(&opt->iter_out, gsf_size);

ipv6_get_msfilter()
    iov_iter_revert(&opt->iter_out, len);

The same applies to compat_ip_get_mcast_msfilter() and
compat_ipv6_get_msfilter(). Once those patches land, the new cap does
protect these callers.

Would it be more accurate to say the cap is for callers converted later
in the series? Another option is to move this patch next to those
conversions. The Fixes: tag pointing at 0093f7db9c47 has the same
problem.

Also, the header comment above sockopt_expand_out() in include/linux/net.h
still only describes the kernel-backed -EINVAL case:

 * the head of optval. Only a user buffer can be longer than the optlen the
 * caller declared, so a kernel-backed optval is refused with -EINVAL.

Could it mention the MAX_RW_COUNT rejection as well? The new comment
inside the function already explains it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009-sockopt_expand_out_v2-v2-0-8ac08c469ecb%40debian.org