[PATCH net 0/5] ipv6: mcast: RCU and timer fixes

[PATCH net 0/5] ipv6: mcast: RCU and timer fixes

[Eric Dumazet]

This series addresses several RCU synchronization and timer calculation
issues identified in IPv6 multicast (MLD) handling within net/ipv6/mcast.c
while I was working on fixing a syzbot report in net/ipv4/icmp.c.

Patch 1 fixes an RCU reader diversion in ip6_mc_del1_src() where mutating
psf->sf_next to insert an unlinked source node into the tombstone list
diverted concurrent lockless readers (e.g. ipv6_chk_mcast_addr()) into
pmc->mca_tomb, causing them to miss remaining active sources.

Patch 2 converts ip6_mc_source() to use copy-on-write RCU updates.
Previously, source additions and deletions modified the socket's
psl->sl_addr array in-place, causing concurrent lockless readers in
inet6_mc_check() (UDP/RAW receive path) to observe torn 16-byte IPv6
addresses or duplicated/missed sources.

Patch 3 fixes delay calculation in igmp6_join_group() when canceling an
existing delayed work, preventing unsigned jiffies underflows when the
timer has already expired and clamping the delay to the unsolicited report
interval.

Patch 4 ensures rcu_assign_pointer() is consistently used for __rcu list
updates in __ipv6_dev_mc_dec(), ipv6_sock_mc_drop(), __ipv6_sock_mc_close(),
and related helpers.

Patch 5 switches igmp6_mc_seq_show() to use jiffies_delta_to_clock_t()
with a signed long delta, preventing underflows in /proc/net/igmp6 timer
duration reporting.

Eric Dumazet (5):
  ipv6: mcast: fix RCU list diversion in ip6_mc_del1_src()
  ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()
  ipv6: mcast: fix delay calculation in igmp6_join_group()
  ipv6: mcast: use rcu_assign_pointer() for __rcu list updates
  ipv6: mcast: use jiffies_delta_to_clock_t() in igmp6_mc_seq_show()

 include/net/if_inet6.h |   2 -
 net/ipv6/mcast.c       | 151 ++++++++++++++++++++++++-----------------
 2 files changed, 90 insertions(+), 63 deletions(-)

-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH net 1/5] ipv6: mcast: fix RCU list diversion in ip6_mc_del1_src()

[Eric Dumazet]

When removing a source filter whose count reaches zero, ip6_mc_del1_src()
unlinks psf from pmc->mca_sources. If the filter was previously active,
the code moved psf directly into pmc->mca_tomb by updating psf->sf_next.

Because pmc->mca_sources is traversed locklessly under RCU (e.g. by
ipv6_chk_mcast_addr()), mutating psf->sf_next before a grace period
elapses diverts concurrent readers to the tombstone list. Consequently,
readers miss remaining active sources in pmc->mca_sources and improperly
examine deleted tombstone entries.

Fix this by allocating a new tombstone node for pmc->mca_tomb (as done
in sf_setstate()) and retiring the original psf via kfree_rcu().

Fixes: 4b200e398953 ("mld: convert ip6_sf_list to RCU")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Taehee Yoo <ap420073@gmail.com>
---
 net/ipv6/mcast.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index aaba4c2aae23ef378e6328e6eee880ee1e52a76b..ec7fac511c8d50da7125ff1c4cd23af46875791d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -2351,14 +2351,18 @@ static int ip6_mc_del1_src(struct ifmcaddr6 *pmc, int sfmode,
 
 		if (psf->sf_oldin && !(pmc->mca_flags & MAF_NOREPORT) &&
 		    !mld_in_v1_mode(idev)) {
-			psf->sf_crcount = idev->mc_qrv;
-			rcu_assign_pointer(psf->sf_next,
-					   mc_dereference(pmc->mca_tomb, idev));
-			rcu_assign_pointer(pmc->mca_tomb, psf);
-			rv = 1;
-		} else {
-			kfree_rcu(psf, rcu);
+			struct ip6_sf_list *dpsf = kmalloc_obj(*dpsf);
+
+			if (dpsf) {
+				*dpsf = *psf;
+				dpsf->sf_crcount = idev->mc_qrv;
+				rcu_assign_pointer(dpsf->sf_next,
+						   mc_dereference(pmc->mca_tomb, idev));
+				rcu_assign_pointer(pmc->mca_tomb, dpsf);
+				rv = 1;
+			}
 		}
+		kfree_rcu(psf, rcu);
 	}
 	return rv;
 }
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH net 2/5] ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()

[Eric Dumazet]

pmc->sflist is read locklessly under rcu_read_lock() by
inet6_mc_check() during packet reception in the UDP and RAW
multicast receive paths.

ip6_mc_source() mutated psl->sl_addr and psl->sl_count in-place
when adding or removing a source filter. Additionally, when expanding
the filter buffer, newpsl was published via rcu_assign_pointer()
before writing the new source into the array.

Because 16-byte struct in6_addr writes are not atomic and array
shifting is not synchronized with RCU readers, concurrent readers in
inet6_mc_check() could read torn IPv6 addresses or observe
duplicated/missed source entries.

Fix this by switching ip6_mc_source() to copy-on-write RCU updates:
allocate and fully populate newpsl before publishing it via
rcu_assign_pointer(), and reclaim the old filter via kfree_rcu(),
matching ip6_mc_msfilter().

Also remove the now unused IP6_SFBLOCK macro.

Fixes: 882ba1f73c06 ("mld: convert ipv6_mc_socklist->sflist to RCU")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Taehee Yoo <ap420073@gmail.com>
---
 include/net/if_inet6.h |   2 -
 net/ipv6/mcast.c       | 101 +++++++++++++++++++++++------------------
 2 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 238ad3349456a3afeab6c02172a9e2d681d2ec9c..795fb41b45f5d10d903de220df3cd0c3798561a0 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -88,8 +88,6 @@ struct ip6_sf_socklist {
 	struct in6_addr		sl_addr[] __counted_by(sl_max);
 };
 
-#define IP6_SFBLOCK	10	/* allocate this many at once */
-
 struct ipv6_mc_socklist {
 	struct in6_addr		addr;
 	int			ifindex;
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index ec7fac511c8d50da7125ff1c4cd23af46875791d..40e996f36c37325adee9165ce9842559da039c04 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -355,12 +355,12 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
 {
 	struct ipv6_pinfo *inet6 = inet6_sk(sk);
 	struct in6_addr *source, *group;
+	struct ip6_sf_socklist *newpsl, *psl;
 	struct net *net = sock_net(sk);
 	struct ipv6_mc_socklist *pmc;
-	struct ip6_sf_socklist *psl;
 	struct inet6_dev *idev;
 	int leavegroup = 0;
-	int i, j, rv;
+	int i, j;
 	int err;
 
 	source = &((struct sockaddr_in6 *)&pgsr->gsr_source)->sin6_addr;
@@ -409,13 +409,11 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
 	if (!add) {
 		if (!psl)
 			goto done;	/* err = -EADDRNOTAVAIL */
-		rv = !0;
 		for (i = 0; i < psl->sl_count; i++) {
-			rv = !ipv6_addr_equal(&psl->sl_addr[i], source);
-			if (rv == 0)
+			if (ipv6_addr_equal(&psl->sl_addr[i], source))
 				break;
 		}
-		if (rv)		/* source not found */
+		if (i == psl->sl_count)		/* source not found */
 			goto done;	/* err = -EADDRNOTAVAIL */
 
 		/* special case - (INCLUDE, empty) == LEAVE_GROUP */
@@ -424,58 +422,75 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
 			goto done;
 		}
 
+		if (psl->sl_count == 1) {
+			newpsl = NULL;
+		} else {
+			newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
+							      psl->sl_count - 1),
+					      GFP_KERNEL);
+			if (!newpsl) {
+				err = -ENOBUFS;
+				goto done;
+			}
+			newpsl->sl_max = psl->sl_count - 1;
+			newpsl->sl_count = psl->sl_count - 1;
+			for (j = 0; j < i; j++)
+				newpsl->sl_addr[j] = psl->sl_addr[j];
+			for (j = i + 1; j < psl->sl_count; j++)
+				newpsl->sl_addr[j - 1] = psl->sl_addr[j];
+		}
+
 		/* update the interface filter */
 		ip6_mc_del_src(idev, group, omode, 1, source, 1);
 
-		for (j = i+1; j < psl->sl_count; j++)
-			psl->sl_addr[j-1] = psl->sl_addr[j];
-		psl->sl_count--;
+		atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
+			   &sk->sk_omem_alloc);
+		rcu_assign_pointer(pmc->sflist, newpsl);
+		kfree_rcu(psl, rcu);
 		err = 0;
 		goto done;
 	}
 	/* else, add a new source to the filter */
 
-	if (psl && psl->sl_count >= sysctl_mld_max_msf) {
+	if (psl && psl->sl_count >= READ_ONCE(sysctl_mld_max_msf)) {
 		err = -ENOBUFS;
 		goto done;
 	}
-	if (!psl || psl->sl_count == psl->sl_max) {
-		struct ip6_sf_socklist *newpsl;
-		int count = IP6_SFBLOCK;
-
-		if (psl)
-			count += psl->sl_max;
-		newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr, count),
-				      GFP_KERNEL);
-		if (!newpsl) {
-			err = -ENOBUFS;
-			goto done;
-		}
-		newpsl->sl_max = count;
-		newpsl->sl_count = count - IP6_SFBLOCK;
-		if (psl) {
-			for (i = 0; i < psl->sl_count; i++)
-				newpsl->sl_addr[i] = psl->sl_addr[i];
-			atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
-				   &sk->sk_omem_alloc);
+	if (psl) {
+		for (i = 0; i < psl->sl_count; i++) {
+			if (ipv6_addr_equal(&psl->sl_addr[i], source))
+				goto done; /* err = -EADDRNOTAVAIL */
 		}
-		rcu_assign_pointer(pmc->sflist, newpsl);
-		kfree_rcu(psl, rcu);
-		psl = newpsl;
 	}
-	rv = 1;	/* > 0 for insert logic below if sl_count is 0 */
-	for (i = 0; i < psl->sl_count; i++) {
-		rv = !ipv6_addr_equal(&psl->sl_addr[i], source);
-		if (rv == 0) /* There is an error in the address. */
-			goto done;
+
+	i = psl ? psl->sl_count + 1 : 1;
+	newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr, i),
+			      GFP_KERNEL);
+	if (!newpsl) {
+		err = -ENOBUFS;
+		goto done;
+	}
+	newpsl->sl_max = i;
+	newpsl->sl_count = i;
+	if (psl) {
+		for (j = 0; j < psl->sl_count; j++)
+			newpsl->sl_addr[j] = psl->sl_addr[j];
 	}
-	for (j = psl->sl_count-1; j >= i; j--)
-		psl->sl_addr[j+1] = psl->sl_addr[j];
-	psl->sl_addr[i] = *source;
-	psl->sl_count++;
+	newpsl->sl_addr[i - 1] = *source;
+
+	err = ip6_mc_add_src(idev, group, omode, 1, source, 1);
+	if (err) {
+		sock_kfree_s(sk, newpsl, struct_size(newpsl, sl_addr,
+						     newpsl->sl_max));
+		goto done;
+	}
+
+	if (psl)
+		atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
+			   &sk->sk_omem_alloc);
+	rcu_assign_pointer(pmc->sflist, newpsl);
+	kfree_rcu(psl, rcu);
 	err = 0;
-	/* update the interface list */
-	ip6_mc_add_src(idev, group, omode, 1, source, 1);
 done:
 	mutex_unlock(&idev->mc_lock);
 	in6_dev_put(idev);
-- 
2.55.0.860.g4b6b3295ed-goog

Re: [PATCH net 2/5] ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()

This converts ip6_mc_source() from in-place mutation of pmc->sflist to
copy-on-write: a new ip6_sf_socklist is fully populated before being
published with rcu_assign_pointer(), and the old one is freed with
kfree_rcu(), matching ip6_mc_msfilter(). The unused IP6_SFBLOCK macro is
also removed.

> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index ec7fac511c8d5..40e996f36c373 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c

[ ... ]

> @@ -424,58 +422,75 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
>  			goto done;
>  		}
>  
> +		if (psl->sl_count == 1) {
> +			newpsl = NULL;
> +		} else {
> +			newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
> +							      psl->sl_count - 1),
> +					      GFP_KERNEL);
> +			if (!newpsl) {
> +				err = -ENOBUFS;
> +				goto done;
> +			}

[Severity: Medium]
Should removing a source from the filter be able to fail with -ENOBUFS
now?

Before this change the !add path only shifted psl->sl_addr[] down and
decremented psl->sl_count, so MCAST_LEAVE_SOURCE_GROUP and
MCAST_UNBLOCK_SOURCE could not fail.  Now a shrinking update needs a
fresh sock_kmalloc(), and the old buffer's charge is released only after
that allocation succeeded:

	newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
					      psl->sl_count - 1), GFP_KERNEL);
	...
	atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
		   &sk->sk_omem_alloc);
	rcu_assign_pointer(pmc->sflist, newpsl);

so the peak sk_omem_alloc charge for the operation is old + (old - 16).

sock_kmalloc() in net/core/sock.c rejects on the sum:

	if ((unsigned int)size <= optmem_max &&
	    atomic_read(&sk->sk_omem_alloc) + size < optmem_max) {

With the default sysctl_mld_max_msf of 64 a full filter is 24 + 16*64 =
1048 bytes, so roughly 19 fully populated groups reach the default
optmem_max of 20480 and any further per-source removal returns -ENOBUFS
instead of releasing memory.

mld_max_msf is also registered with plain proc_dointvec and no extra2
clamp in net/ipv6/sysctl_net_ipv6.c, so a single filter installed via
ip6_mc_msfilter() can exceed half of optmem_max, in which case
per-source removal stays impossible for that filter until userspace
clears the whole list (MCAST_MSFILTER with gf_numsrc == 0 takes the
newpsl == NULL path) or leaves the group.

The IPv4 counterpart ip_mc_source() in net/ipv4/igmp.c still does the
in-place shift:

	for (j = i+1; j < psl->sl_count; j++)
		psl->sl_addr[j-1] = psl->sl_addr[j];
	psl->sl_count--;
	err = 0;

Is the resulting difference in error contracts between the two families
for the same socket option intended?

> +			newpsl->sl_max = psl->sl_count - 1;
> +			newpsl->sl_count = psl->sl_count - 1;
> +			for (j = 0; j < i; j++)
> +				newpsl->sl_addr[j] = psl->sl_addr[j];
> +			for (j = i + 1; j < psl->sl_count; j++)
> +				newpsl->sl_addr[j - 1] = psl->sl_addr[j];
> +		}
> +
>  		/* update the interface filter */
>  		ip6_mc_del_src(idev, group, omode, 1, source, 1);
>  
> -		for (j = i+1; j < psl->sl_count; j++)
> -			psl->sl_addr[j-1] = psl->sl_addr[j];
> -		psl->sl_count--;
> +		atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
> +			   &sk->sk_omem_alloc);
> +		rcu_assign_pointer(pmc->sflist, newpsl);
> +		kfree_rcu(psl, rcu);
>  		err = 0;
>  		goto done;
>  	}
>  	/* else, add a new source to the filter */

[ ... ]

> +	newpsl->sl_max = i;
> +	newpsl->sl_count = i;
> +	if (psl) {
> +		for (j = 0; j < psl->sl_count; j++)
> +			newpsl->sl_addr[j] = psl->sl_addr[j];
>  	}
> -	for (j = psl->sl_count-1; j >= i; j--)
> -		psl->sl_addr[j+1] = psl->sl_addr[j];
> -	psl->sl_addr[i] = *source;
> -	psl->sl_count++;
> +	newpsl->sl_addr[i - 1] = *source;
> +
> +	err = ip6_mc_add_src(idev, group, omode, 1, source, 1);
> +	if (err) {
> +		sock_kfree_s(sk, newpsl, struct_size(newpsl, sl_addr,
> +						     newpsl->sl_max));
> +		goto done;
> +	}

[Severity: Medium]
Is the new error propagation from ip6_mc_add_src() a deliberate part of
this conversion?

The old code was:

	err = 0;
	/* update the interface list */
	ip6_mc_add_src(idev, group, omode, 1, source, 1);

so MCAST_BLOCK_SOURCE and MCAST_JOIN_SOURCE_GROUP always returned 0 and
always installed the socket filter.  Two failures are now returned to
userspace with pmc->sflist left unchanged:

- -ENOBUFS from the interface-level allocation in ip6_mc_add1_src():

	psf = kzalloc_obj(*psf);
	if (!psf)
		return -ENOBUFS;

- -ESRCH when ip6_mc_add_src() does not find the group on idev.  That
  is reachable with gsr_interface == 0, since the pmc lookup skips the
  ifindex comparison:

	if (pgsr->gsr_interface && pmc->ifindex != pgsr->gsr_interface)
		continue;

  while ip6_mc_find_idev()/ip6_mc_find_dev() resolves the device from
  rt6_lookup(), which need not be the device the socket joined on.

The delete branch above still ignores the identical -ESRCH from
ip6_mc_del_src(), so the two branches disagree.  The commit message
describes only the RCU publication fix and does not mention this change
in setsockopt() return values, which matters for the stable backport
implied by the Fixes: tag.  Would it be worth either splitting this out
or documenting it in the changelog?

> +
> +	if (psl)
> +		atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
> +			   &sk->sk_omem_alloc);
> +	rcu_assign_pointer(pmc->sflist, newpsl);
> +	kfree_rcu(psl, rcu);
>  	err = 0;
> -	/* update the interface list */
> -	ip6_mc_add_src(idev, group, omode, 1, source, 1);
>  done:
>  	mutex_unlock(&idev->mc_lock);
>  	in6_dev_put(idev);

[Severity: Medium]
This isn't a bug introduced by this patch, but the companion field
pmc->sfmode is still a plain store while the list is now published
atomically.  inet6_mc_check() consumes both fields together under
rcu_read_lock() only:

	psl = rcu_dereference(mc->sflist);
	if (!psl) {
		rv = mc->sfmode == MCAST_EXCLUDE;
	} else {
		...
		if (mc->sfmode == MCAST_INCLUDE && i >= psl->sl_count)
			rv = false;
		if (mc->sfmode == MCAST_EXCLUDE && i < psl->sl_count)
			rv = false;
	}

ip6_mc_msfilter() publishes the list first and updates the mode after:

	rcu_assign_pointer(pmc->sflist, newpsl);
	kfree_rcu(psl, rcu);
	pmc->sfmode = gsf->gf_fmode;

and ip6_mc_source() does the same kind of plain store with
pmc->sfmode = omode.

Can a softirq receive path then pair the newly published list with the
stale mode?  For an EXCLUDE{A} to INCLUDE{A} transition a reader that
sees the new list but the old mode takes i < psl->sl_count and sets
rv = false, dropping a datagram the socket now includes; the opposite
interleaving accepts one that should be blocked.  Both sides are also
unannotated, so KCSAN can report the sfmode access as a data race.

Would publishing the mode and the source list as one RCU-protected
object be a reasonable follow-up?

Re: [PATCH net 2/5] ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()

[Eric Dumazet]

On Thu, Aug 27, 2026 at 9:43 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()
>
> This converts ip6_mc_source() from in-place mutation of pmc->sflist to
> copy-on-write: a new ip6_sf_socklist is fully populated before being
> published with rcu_assign_pointer(), and the old one is freed with
> kfree_rcu(), matching ip6_mc_msfilter(). The unused IP6_SFBLOCK macro is
> also removed.
>
> > diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> > index ec7fac511c8d5..40e996f36c373 100644
> > --- a/net/ipv6/mcast.c
> > +++ b/net/ipv6/mcast.c
>
> [ ... ]
>
> > @@ -424,58 +422,75 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
> >                       goto done;
> >               }
> >
> > +             if (psl->sl_count == 1) {
> > +                     newpsl = NULL;
> > +             } else {
> > +                     newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
> > +                                                           psl->sl_count - 1),
> > +                                           GFP_KERNEL);
> > +                     if (!newpsl) {
> > +                             err = -ENOBUFS;
> > +                             goto done;
> > +                     }
>
> [Severity: Medium]
> Should removing a source from the filter be able to fail with -ENOBUFS
> now?
>
> Before this change the !add path only shifted psl->sl_addr[] down and
> decremented psl->sl_count, so MCAST_LEAVE_SOURCE_GROUP and
> MCAST_UNBLOCK_SOURCE could not fail.  Now a shrinking update needs a
> fresh sock_kmalloc(), and the old buffer's charge is released only after
> that allocation succeeded:

Good point, in V2 I adjusted the sk_omem_alloc accounting to deduct the old
charge before calling sock_kmalloc().

Since the new allocation is strictly smaller than the old one, shrinking a
filter will not hit the optmem_max limit.

>
>         newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
>                                               psl->sl_count - 1), GFP_KERNEL);
>         ...
>         atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
>                    &sk->sk_omem_alloc);
>         rcu_assign_pointer(pmc->sflist, newpsl);
>
> so the peak sk_omem_alloc charge for the operation is old + (old - 16).
>
> sock_kmalloc() in net/core/sock.c rejects on the sum:
>
>         if ((unsigned int)size <= optmem_max &&
>             atomic_read(&sk->sk_omem_alloc) + size < optmem_max) {
>
> With the default sysctl_mld_max_msf of 64 a full filter is 24 + 16*64 =
> 1048 bytes, so roughly 19 fully populated groups reach the default
> optmem_max of 20480 and any further per-source removal returns -ENOBUFS
> instead of releasing memory.
>
> mld_max_msf is also registered with plain proc_dointvec and no extra2
> clamp in net/ipv6/sysctl_net_ipv6.c, so a single filter installed via
> ip6_mc_msfilter() can exceed half of optmem_max, in which case
> per-source removal stays impossible for that filter until userspace
> clears the whole list (MCAST_MSFILTER with gf_numsrc == 0 takes the
> newpsl == NULL path) or leaves the group.
>
> The IPv4 counterpart ip_mc_source() in net/ipv4/igmp.c still does the
> in-place shift:
>
>         for (j = i+1; j < psl->sl_count; j++)
>                 psl->sl_addr[j-1] = psl->sl_addr[j];
>         psl->sl_count--;
>         err = 0;
>
> Is the resulting difference in error contracts between the two families
> for the same socket option intended?
>
> > +                     newpsl->sl_max = psl->sl_count - 1;
> > +                     newpsl->sl_count = psl->sl_count - 1;
> > +                     for (j = 0; j < i; j++)
> > +                             newpsl->sl_addr[j] = psl->sl_addr[j];
> > +                     for (j = i + 1; j < psl->sl_count; j++)
> > +                             newpsl->sl_addr[j - 1] = psl->sl_addr[j];
> > +             }
> > +
> >               /* update the interface filter */
> >               ip6_mc_del_src(idev, group, omode, 1, source, 1);
> >
> > -             for (j = i+1; j < psl->sl_count; j++)
> > -                     psl->sl_addr[j-1] = psl->sl_addr[j];
> > -             psl->sl_count--;
> > +             atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
> > +                        &sk->sk_omem_alloc);
> > +             rcu_assign_pointer(pmc->sflist, newpsl);
> > +             kfree_rcu(psl, rcu);
> >               err = 0;
> >               goto done;
> >       }
> >       /* else, add a new source to the filter */
>
> [ ... ]
>
> > +     newpsl->sl_max = i;
> > +     newpsl->sl_count = i;
> > +     if (psl) {
> > +             for (j = 0; j < psl->sl_count; j++)
> > +                     newpsl->sl_addr[j] = psl->sl_addr[j];
> >       }
> > -     for (j = psl->sl_count-1; j >= i; j--)
> > -             psl->sl_addr[j+1] = psl->sl_addr[j];
> > -     psl->sl_addr[i] = *source;
> > -     psl->sl_count++;
> > +     newpsl->sl_addr[i - 1] = *source;
> > +
> > +     err = ip6_mc_add_src(idev, group, omode, 1, source, 1);
> > +     if (err) {
> > +             sock_kfree_s(sk, newpsl, struct_size(newpsl, sl_addr,
> > +                                                  newpsl->sl_max));
> > +             goto done;
> > +     }
>
> [Severity: Medium]
> Is the new error propagation from ip6_mc_add_src() a deliberate part of
> this conversion?
>
> The old code was:
>
>         err = 0;
>         /* update the interface list */
>         ip6_mc_add_src(idev, group, omode, 1, source, 1);
>
> so MCAST_BLOCK_SOURCE and MCAST_JOIN_SOURCE_GROUP always returned 0 and
> always installed the socket filter.

To keep this bugfix patch strictly focused on RCU safety and avoid any
subtle changes to the setsockopt() return contract, I reverted this back to
ignoring ip6_mc_add_src() in ip6_mc_source(), matching both the prior behavior
and IPv4 ip_mc_source().


>Two failures are now returned to
> userspace with pmc->sflist left unchanged:


>
> - -ENOBUFS from the interface-level allocation in ip6_mc_add1_src():
>
>         psf = kzalloc_obj(*psf);
>         if (!psf)
>                 return -ENOBUFS;
>
> - -ESRCH when ip6_mc_add_src() does not find the group on idev.  That
>   is reachable with gsr_interface == 0, since the pmc lookup skips the
>   ifindex comparison:
>
>         if (pgsr->gsr_interface && pmc->ifindex != pgsr->gsr_interface)
>                 continue;
>
>   while ip6_mc_find_idev()/ip6_mc_find_dev() resolves the device from
>   rt6_lookup(), which need not be the device the socket joined on.
>
> The delete branch above still ignores the identical -ESRCH from
> ip6_mc_del_src(), so the two branches disagree.  The commit message
> describes only the RCU publication fix and does not mention this change
> in setsockopt() return values, which matters for the stable backport
> implied by the Fixes: tag.  Would it be worth either splitting this out
> or documenting it in the changelog?
>
> > +
> > +     if (psl)
> > +             atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
> > +                        &sk->sk_omem_alloc);
> > +     rcu_assign_pointer(pmc->sflist, newpsl);
> > +     kfree_rcu(psl, rcu);
> >       err = 0;
> > -     /* update the interface list */
> > -     ip6_mc_add_src(idev, group, omode, 1, source, 1);
> >  done:
> >       mutex_unlock(&idev->mc_lock);
> >       in6_dev_put(idev);
>

> [Severity: Medium]
> This isn't a bug introduced by this patch, but the companion field
> pmc->sfmode is still a plain store while the list is now published
> atomically.  inet6_mc_check() consumes both fields together under
> rcu_read_lock() only:
>
>         psl = rcu_dereference(mc->sflist);
>         if (!psl) {
>                 rv = mc->sfmode == MCAST_EXCLUDE;
>         } else {
>                 ...
>                 if (mc->sfmode == MCAST_INCLUDE && i >= psl->sl_count)
>                         rv = false;
>                 if (mc->sfmode == MCAST_EXCLUDE && i < psl->sl_count)
>                         rv = false;
>         }
>
> ip6_mc_msfilter() publishes the list first and updates the mode after:
>
>         rcu_assign_pointer(pmc->sflist, newpsl);
>         kfree_rcu(psl, rcu);
>         pmc->sfmode = gsf->gf_fmode;
>
> and ip6_mc_source() does the same kind of plain store with
> pmc->sfmode = omode.
>
> Can a softirq receive path then pair the newly published list with the
> stale mode?  For an EXCLUDE{A} to INCLUDE{A} transition a reader that
> sees the new list but the old mode takes i < psl->sl_count and sets
> rv = false, dropping a datagram the socket now includes; the opposite
> interleaving accepts one that should be blocked.  Both sides are also
> unannotated, so KCSAN can report the sfmode access as a data race.
>
> Would publishing the mode and the source list as one RCU-protected
> object be a reasonable follow-up?

Yes, bundling sfmode into the RCU-protected sflist struct makes a lot of sense
and avoids the race between filter array and filter mode. I will submit that as
a follow-up cleanup for net-next once this fix lands in net.

Thanks.

[PATCH net 3/5] ipv6: mcast: fix delay calculation in igmp6_join_group()

[Eric Dumazet]

When joining a multicast group, if a report timer is already running
(e.g. scheduled by a query or a previous join), igmp6_join_group()
cancels the delayed work and recalculates the delay:

	if (cancel_delayed_work(&ma->mca_work)) {
		refcount_dec(&ma->mca_refcnt);
		delay = ma->mca_work.timer.expires - jiffies;
	}

Unlike igmp6_group_queried(), igmp6_join_group() did not check
if delay >= interval. This leads to two issues:

1. If the timer has already expired (timer.expires <= jiffies),
   ma->mca_work.timer.expires - jiffies underflows to a very large
   unsigned long value (~ULONG_MAX), causing mod_delayed_work() to
   schedule the report weeks/months into the future.
2. If the timer was armed by a query with a large maximum response
   delay, delay could exceed unsolicited_report_interval(ma->idev).

Fix this by initializing delay to unsolicited_report_interval(ma->idev)
and clamping delay with get_random_u32_below(interval) when
delay >= interval, mirroring the logic in igmp6_group_queried().

Fixes: 2d9a93b4902b ("mld: convert from timer to delayed work")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Taehee Yoo <ap420073@gmail.com>
---
 net/ipv6/mcast.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 40e996f36c37325adee9165ce9842559da039c04..44fdde940dcdcd16a7642b2be60c6866c574ce92 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -2640,7 +2640,7 @@ static void ip6_mc_clear_src(struct ifmcaddr6 *pmc)
 
 static void igmp6_join_group(struct ifmcaddr6 *ma)
 {
-	unsigned long delay;
+	unsigned long delay, interval;
 
 	mc_assert_locked(ma->idev);
 
@@ -2649,13 +2649,17 @@ static void igmp6_join_group(struct ifmcaddr6 *ma)
 
 	igmp6_send(&ma->mca_addr, ma->idev->dev, ICMPV6_MGM_REPORT);
 
-	delay = get_random_u32_below(unsolicited_report_interval(ma->idev));
+	interval = unsolicited_report_interval(ma->idev);
+	delay = interval;
 
 	if (cancel_delayed_work(&ma->mca_work)) {
 		refcount_dec(&ma->mca_refcnt);
 		delay = ma->mca_work.timer.expires - jiffies;
 	}
 
+	if (delay >= interval)
+		delay = get_random_u32_below(interval);
+
 	if (!mod_delayed_work(mld_wq, &ma->mca_work, delay))
 		refcount_inc(&ma->mca_refcnt);
 	WRITE_ONCE(ma->mca_flags, ma->mca_flags |
-- 
2.55.0.860.g4b6b3295ed-goog

Re: [PATCH net 3/5] ipv6: mcast: fix delay calculation in igmp6_join_group()

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ipv6: mcast: fix delay calculation in igmp6_join_group()

igmp6_join_group() reused the remaining expiry of an already scheduled
report work as the new delay without bounding it. The patch initializes
delay to unsolicited_report_interval(ma->idev) and re-randomizes it with
get_random_u32_below(interval) whenever delay >= interval, matching
igmp6_group_queried().

No Link: tags present in the commit header.

Only the changelog wording looks questionable to me; the code change itself
reads correctly, so the diff hunks are omitted.

> 1. If the timer has already expired (timer.expires <= jiffies),
>    ma->mca_work.timer.expires - jiffies underflows to a very large
>    unsigned long value (~ULONG_MAX), causing mod_delayed_work() to
>    schedule the report weeks/months into the future.

[Severity: Medium]
Can mod_delayed_work() really push the report weeks or months out in this
case?

The delayed work layer converts the delay straight back into an absolute
expiry in __queue_delayed_work():

kernel/workqueue.c:__queue_delayed_work() {
	...
	timer->expires = jiffies + delay;
	...
}

With delay == expires_old - jiffies_old, that modular arithmetic gives back
expires_old, i.e. the same already-past expiry rather than a near-ULONG_MAX
value.

__queue_delayed_work() then calls add_timer_global()/add_timer_on(), which
end in __mod_timer(timer, timer->expires, ...) and calc_wheel_index():

kernel/time/timer.c:calc_wheel_index() {
	...
	} else if ((long) delta < 0) {
		idx = clk & LVL_MASK;
		*bucket_expiry = clk;
	} else {
		/*
		 * Force expire obscene large timeouts to expire at the
		 * capacity limit of the wheel.
		 */
		if (delta >= WHEEL_TIMEOUT_CUTOFF)
			expires = clk + WHEEL_TIMEOUT_MAX;
	...
}

A past expiry takes the (long) delta < 0 branch and lands in the current
bucket, so it fires at the next tick; the WHEEL_TIMEOUT_CUTOFF capping
branch is not reachable for a delta that is negative when read as signed.
add_timer()'s kerneldoc says the same thing:

kernel/time/timer.c:add_timer() {
 * If @timer->expires is already in the past @timer will be queued to
 * expire at the next timer tick.
}

So for case 1, is the actual symptom that the randomized delay is lost and
the second unsolicited report goes out on the next tick, right behind the
first one, rather than a report deferred for weeks or months? The stated
symptom implies dropped membership reports and multicast traffic loss, which
reads differently for anyone triaging this for stable.

Case 2 (a delay inherited from a query with a large maximum response delay
exceeding unsolicited_report_interval()) matches the code, and the clamp
added here covers both cases either way. Could the first paragraph be
reworded to describe the lost randomization and the reuse of the stale
expiry instead?

Re: [PATCH net 3/5] ipv6: mcast: fix delay calculation in igmp6_join_group()

[Eric Dumazet]

On Thu, Aug 27, 2026 at 9:43 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> ipv6: mcast: fix delay calculation in igmp6_join_group()
>
> igmp6_join_group() reused the remaining expiry of an already scheduled
> report work as the new delay without bounding it. The patch initializes
> delay to unsolicited_report_interval(ma->idev) and re-randomizes it with
> get_random_u32_below(interval) whenever delay >= interval, matching
> igmp6_group_queried().
>
> No Link: tags present in the commit header.
>
> Only the changelog wording looks questionable to me; the code change itself
> reads correctly, so the diff hunks are omitted.
>
> > 1. If the timer has already expired (timer.expires <= jiffies),
> >    ma->mca_work.timer.expires - jiffies underflows to a very large
> >    unsigned long value (~ULONG_MAX), causing mod_delayed_work() to
> >    schedule the report weeks/months into the future.
>
> [Severity: Medium]
> Can mod_delayed_work() really push the report weeks or months out in this
> case?
>
> The delayed work layer converts the delay straight back into an absolute
> expiry in __queue_delayed_work():
>
> kernel/workqueue.c:__queue_delayed_work() {
>         ...
>         timer->expires = jiffies + delay;
>         ...
> }
>
> With delay == expires_old - jiffies_old, that modular arithmetic gives back
> expires_old, i.e. the same already-past expiry rather than a near-ULONG_MAX
> value.
>
> __queue_delayed_work() then calls add_timer_global()/add_timer_on(), which
> end in __mod_timer(timer, timer->expires, ...) and calc_wheel_index():
>
> kernel/time/timer.c:calc_wheel_index() {
>         ...
>         } else if ((long) delta < 0) {
>                 idx = clk & LVL_MASK;
>                 *bucket_expiry = clk;
>         } else {
>                 /*
>                  * Force expire obscene large timeouts to expire at the
>                  * capacity limit of the wheel.
>                  */
>                 if (delta >= WHEEL_TIMEOUT_CUTOFF)
>                         expires = clk + WHEEL_TIMEOUT_MAX;
>         ...
> }
>
> A past expiry takes the (long) delta < 0 branch and lands in the current
> bucket, so it fires at the next tick; the WHEEL_TIMEOUT_CUTOFF capping
> branch is not reachable for a delta that is negative when read as signed.
> add_timer()'s kerneldoc says the same thing:
>
> kernel/time/timer.c:add_timer() {
>  * If @timer->expires is already in the past @timer will be queued to
>  * expire at the next timer tick.
> }
>
> So for case 1, is the actual symptom that the randomized delay is lost and
> the second unsolicited report goes out on the next tick, right behind the
> first one, rather than a report deferred for weeks or months? The stated
> symptom implies dropped membership reports and multicast traffic loss, which
> reads differently for anyone triaging this for stable.
>
> Case 2 (a delay inherited from a query with a large maximum response delay
> exceeding unsolicited_report_interval()) matches the code, and the clamp
> added here covers both cases either way. Could the first paragraph be
> reworded to describe the lost randomization and the reuse of the stale
> expiry instead?

OK I can rephrase the changelog to:

1. If the timer has already expired (timer.expires <= jiffies), the
   stale expiry is reused by mod_delayed_work(), causing the second
   unsolicited report to fire on the very next tick without a
   randomized delay.

2. If the timer was originally armed by a query with a large maximum
   response delay, delay could exceed unsolicited_report_interval(ma->idev).


Hopefully, all the AI agents will be happy this time.

Re: [PATCH net 3/5] ipv6: mcast: fix delay calculation in igmp6_join_group()

[Jakub Kicinski]

On Thu, 27 Aug 2026 22:02:27 +0200 Eric Dumazet wrote:
> > So for case 1, is the actual symptom that the randomized delay is lost and
> > the second unsolicited report goes out on the next tick, right behind the
> > first one, rather than a report deferred for weeks or months? The stated
> > symptom implies dropped membership reports and multicast traffic loss, which
> > reads differently for anyone triaging this for stable.
> >
> > Case 2 (a delay inherited from a query with a large maximum response delay
> > exceeding unsolicited_report_interval()) matches the code, and the clamp
> > added here covers both cases either way. Could the first paragraph be
> > reworded to describe the lost randomization and the reuse of the stale
> > expiry instead?  
> 
> OK I can rephrase the changelog to:
> 
> 1. If the timer has already expired (timer.expires <= jiffies), the
>    stale expiry is reused by mod_delayed_work(), causing the second
>    unsolicited report to fire on the very next tick without a
>    randomized delay.
> 
> 2. If the timer was originally armed by a query with a large maximum
>    response delay, delay could exceed unsolicited_report_interval(ma->idev).
> 
> 
> Hopefully, all the AI agents will be happy this time.

Feel free to ignore, I hit send because of the complaint on patch 2.
I may have misunderstood but I read that one as "delete requires 
optmem headroom".

[PATCH net 4/5] ipv6: mcast: use rcu_assign_pointer() for __rcu list updates

[Eric Dumazet]

Several places in net/ipv6/mcast.c update RCU-protected lists
(np->ipv6_mc_list, idev->mc_list, idev->mc_tomb) using direct pointer
assignments instead of rcu_assign_pointer():

1. In __ipv6_dev_mc_dec(), unlinking a group from idev->mc_list did:
       *map = ma->next;
   without rcu_assign_pointer() while concurrent readers traverse
   idev->mc_list locklessly under rcu_read_lock().
2. In ipv6_sock_mc_drop() and __ipv6_sock_mc_close(), unlinking a group
   from np->ipv6_mc_list directly assigned *lnk = mc_lst->next and
   np->ipv6_mc_list = mc_lst->next without rcu_assign_pointer(), racing
   with lockless readers in inet6_mc_check().
3. In __ipv6_sock_mc_join(), mc_lst->next was initialized to
   np->ipv6_mc_list via raw assignment before publishing mc_lst.
4. In mld_del_delrec() and __ipv6_dev_mc_inc(), __rcu source pointers
   passed into rcu_assign_pointer() lacked explicit dereference helpers.

Fix these by consistently using rcu_assign_pointer() along with
mc_dereference() / sock_dereference().

Fixes: 456b61bca8ee ("ipv6: mcast: RCU conversion")
Fixes: 88e2ca308094 ("mld: convert ifmcaddr6 to RCU")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Taehee Yoo <ap420073@gmail.com>
---
 net/ipv6/mcast.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 44fdde940dcdcd16a7642b2be60c6866c574ce92..a90dddba719726f36e0d5176c5e0574d1a488d5e 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -240,7 +240,8 @@ static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
 		return err;
 	}
 
-	mc_lst->next = np->ipv6_mc_list;
+	rcu_assign_pointer(mc_lst->next,
+			   sock_dereference(np->ipv6_mc_list, sk));
 	rcu_assign_pointer(np->ipv6_mc_list, mc_lst);
 
 	return 0;
@@ -300,7 +301,8 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	      lnk = &mc_lst->next) {
 		if ((ifindex == 0 || mc_lst->ifindex == ifindex) &&
 		    ipv6_addr_equal(&mc_lst->addr, addr)) {
-			*lnk = mc_lst->next;
+			rcu_assign_pointer(*lnk,
+					   sock_dereference(mc_lst->next, sk));
 			__ipv6_sock_mc_drop(sk, mc_lst);
 			return 0;
 		}
@@ -333,7 +335,8 @@ void __ipv6_sock_mc_close(struct sock *sk)
 	struct ipv6_mc_socklist *mc_lst;
 
 	while ((mc_lst = sock_dereference(np->ipv6_mc_list, sk)) != NULL) {
-		np->ipv6_mc_list = mc_lst->next;
+		rcu_assign_pointer(np->ipv6_mc_list,
+				   sock_dereference(mc_lst->next, sk));
 		__ipv6_sock_mc_drop(sk, mc_lst);
 	}
 }
@@ -799,9 +802,11 @@ static void mld_del_delrec(struct inet6_dev *idev, struct ifmcaddr6 *im)
 	if (!pmc)
 		return;
 	if (pmc_prev)
-		rcu_assign_pointer(pmc_prev->next, pmc->next);
+		rcu_assign_pointer(pmc_prev->next,
+				   mc_dereference(pmc->next, idev));
 	else
-		rcu_assign_pointer(idev->mc_tomb, pmc->next);
+		rcu_assign_pointer(idev->mc_tomb,
+				   mc_dereference(pmc->next, idev));
 
 	im->idev = pmc->idev;
 	if (im->mca_sfmode == MCAST_INCLUDE) {
@@ -981,7 +986,7 @@ static int __ipv6_dev_mc_inc(struct net_device *dev,
 		return -ENOMEM;
 	}
 
-	rcu_assign_pointer(mc->next, idev->mc_list);
+	rcu_assign_pointer(mc->next, mc_dereference(idev->mc_list, idev));
 	rcu_assign_pointer(idev->mc_list, mc);
 
 	mld_del_delrec(idev, mc);
@@ -1015,7 +1020,8 @@ int __ipv6_dev_mc_dec(struct inet6_dev *idev, const struct in6_addr *addr)
 
 			WRITE_ONCE(ma->mca_users, new_users);
 			if (new_users == 0) {
-				*map = ma->next;
+				rcu_assign_pointer(*map,
+						   mc_dereference(ma->next, idev));
 
 				igmp6_group_dropped(ma);
 				inet6_ifmcaddr_notify(idev->dev, ma,
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH net 5/5] ipv6: mcast: use jiffies_delta_to_clock_t() in igmp6_mc_seq_show()

[Eric Dumazet]

If a multicast group timer has expired but the delayed work has
not yet run to clear MAF_TIMER_RUNNING, expires - jiffies produces
a negative value.

Because unsigned arithmetic was used with jiffies_to_clock_t(),
expires - jiffies underflows to a huge value and reports invalid
timer durations in /proc/net/igmp6.

Use jiffies_delta_to_clock_t() with a signed long delta to properly
cap expired deltas to 0, matching IPv4 igmp_mc_seq_show() and commit
a399a8053164 ("time: jiffies_delta_to_clock_t() helper to the rescue").

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/mcast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index a90dddba719726f36e0d5176c5e0574d1a488d5e..65916dc3a3146ccb7edba2984042b093ae1cd4f8 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -3030,7 +3030,7 @@ static int igmp6_mc_seq_show(struct seq_file *seq, void *v)
 	struct ifmcaddr6 *im = (struct ifmcaddr6 *)v;
 	struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
 	unsigned int mca_flags = READ_ONCE(im->mca_flags);
-	unsigned long expires = READ_ONCE(im->mca_work.timer.expires);
+	long delta = READ_ONCE(im->mca_work.timer.expires) - jiffies;
 
 	seq_printf(seq,
 		   "%-4d %-15s %pi6 %5d %08X %ld\n",
@@ -3038,7 +3038,7 @@ static int igmp6_mc_seq_show(struct seq_file *seq, void *v)
 		   &im->mca_addr,
 		   READ_ONCE(im->mca_users), mca_flags,
 		   (mca_flags & MAF_TIMER_RUNNING) ?
-		   jiffies_to_clock_t(expires - jiffies) : 0);
+		   jiffies_delta_to_clock_t(delta) : 0);
 	return 0;
 }
 
-- 
2.55.0.860.g4b6b3295ed-goog