Re: [PATCH net-next 2/2] ipv4: igmp: convert ip_mc_msfget() to sockopt_t
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-12 01:19:59
Also in:
lkml
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 0 - [Medium] In do_ip_getsockopt() (net/ipv4/ip_sockglue.c, case IP_MSFILTER), the output length is now written back to userspace… -- ipv4: igmp: convert ip_mc_msfget() to sockopt_t This converts ip_mc_msfget() from the sockptr_t optval/optlen pair to a sockopt_t built at the do_ip_getsockopt() call site, and replaces the optval copies with sequential copy_to_iter() calls after growing the output iterator with sockopt_expand_out().
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index d56355aca7977..144fca158adcb 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2755,14 +2755,19 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf, len = flex_array_size(psl, sl_addr, copycount); msf->imsf_numsrc = count; msf_size = IP_MSFILTER_SIZE(copycount); - if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) || - copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) { + + /* The source list is sized by the imsf_numsrc the caller left in + * optval, not by optlen, which only has to cover the fixed part. + */ + err = sockopt_expand_out(opt, msf_size); + if (err) + return err; + + opt->optlen = msf_size; + if (copy_to_iter(msf, IP_MSFILTER_SIZE(0), &opt->iter_out) != + IP_MSFILTER_SIZE(0)) return -EFAULT; - } - if (len && - copy_to_sockptr_offset(optval, - offsetof(struct ip_msfilter, imsf_slist_flex), - psl->sl_addr, len)) + if (len && copy_to_iter(psl->sl_addr, len, &opt->iter_out) != len) return -EFAULT; return 0; done:
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index a55ef327ec932..c58e565f2a5aa 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1715,7 +1717,13 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname, err = -EFAULT; goto out; } - err = ip_mc_msfget(sk, &msf, optval, optlen); + err = sockptr_to_sockopt(&opt, optval, optlen, &kvec); + if (err) + goto out; + + err = ip_mc_msfget(sk, &msf, &opt); + if (copy_to_sockptr(optlen, &opt.optlen, sizeof(int))) + err = -EFAULT; goto out;
[Severity: Medium]
Should this write-back be guarded by if (!err)?
Before this patch the *optlen store lived inside ip_mc_msfget() and only
ran once the lookup had succeeded:
msf_size = IP_MSFILTER_SIZE(copycount);
if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) ||
copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) {
return -EFAULT;
}
All of the ip_mc_msfget() failure returns left the user's optlen word
untouched: !ipv4_is_multicast(addr) returning -EINVAL, !in_dev returning
-ENODEV, and the !pmc "must have a prior join" case falling through to
done: with err = -EADDRNOTAVAIL.
Now do_ip_getsockopt() writes opt.optlen back on every return from
ip_mc_msfget(), including those three error paths and the new -EINVAL
from sockopt_expand_out(), since opt->optlen is only assigned on the
success path in ip_mc_msfget(). Is that change of behaviour on failure
intended?
There is a second effect from the missing guard: if the optlen pointer is
readable but not writable, copy_to_sockptr() fails and err is replaced
with -EFAULT, so -EINVAL, -ENODEV and -EADDRNOTAVAIL are no longer
distinguishable by userspace. The earlier copy_from_sockptr(&len, optlen,
...) and the copy_from_sockptr() inside sockptr_to_sockopt() both succeed
on a read-only mapping, so this state is reachable.
Would something like this preserve the old semantics?
err = ip_mc_msfget(sk, &msf, &opt);
if (!err && copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
err = -EFAULT;
The commit message describes the change as a conversion, with "the bytes
land where they did", and does not mention that the optlen store moved to
the caller and now also runs when ip_mc_msfget() fails. Could that be
called out explicitly?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260910-getsockopt_phase6-v1-0-e681e102d5b8%40debian.org